[PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()

Kevin Wolf posted 2 patches 4 years, 9 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
[PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()
Posted by Kevin Wolf 4 years, 9 months ago
Like other error paths, this one needs to call tran_finalize() and clean
up the BlockReopenQueue, too.

Fixes: CID 1452772
Fixes: 72373e40fbc7e4218061a8211384db362d3e7348
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 5c0ced6238..69615fabd1 100644
--- a/block.c
+++ b/block.c
@@ -4052,7 +4052,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
         ret = bdrv_flush(bs_entry->state.bs);
         if (ret < 0) {
             error_setg_errno(errp, -ret, "Error flushing drive");
-            goto cleanup;
+            goto abort;
         }
     }
 
-- 
2.30.2


Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()
Posted by Vladimir Sementsov-Ogievskiy 4 years, 9 months ago
03.05.2021 14:05, Kevin Wolf wrote:
> Like other error paths, this one needs to call tran_finalize() and clean
> up the BlockReopenQueue, too.

We don't need the "abort" loop on that path. And clean-up of BlockReopenQueue is at "cleanup:" label.

So I'd prefer Peter's suggestion (my "[PATCH 2/6] block: bdrv_reopen_multiple(): fix leak of tran object")

> 
> Fixes: CID 1452772
> Fixes: 72373e40fbc7e4218061a8211384db362d3e7348
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 5c0ced6238..69615fabd1 100644
> --- a/block.c
> +++ b/block.c
> @@ -4052,7 +4052,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
>           ret = bdrv_flush(bs_entry->state.bs);
>           if (ret < 0) {
>               error_setg_errno(errp, -ret, "Error flushing drive");
> -            goto cleanup;
> +            goto abort;
>           }
>       }
>   
> 




-- 
Best regards,
Vladimir

Re: [PATCH 2/2] block: Fix Transaction leak in bdrv_reopen_multiple()
Posted by Vladimir Sementsov-Ogievskiy 4 years, 9 months ago
03.05.2021 14:05, Kevin Wolf wrote:
> Like other error paths, this one needs to call tran_finalize() and clean
> up the BlockReopenQueue, too.
> 
> Fixes: CID 1452772
> Fixes: 72373e40fbc7e4218061a8211384db362d3e7348
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 5c0ced6238..69615fabd1 100644
> --- a/block.c
> +++ b/block.c
> @@ -4052,7 +4052,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
>           ret = bdrv_flush(bs_entry->state.bs);
>           if (ret < 0) {
>               error_setg_errno(errp, -ret, "Error flushing drive");
> -            goto cleanup;
> +            goto abort;
>           }
>       }
>   
> 


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir