[PATCH] rbd: plug rbd_dev->header.object_prefix memory leak

[PATCH] rbd: plug rbd_dev->header.object_prefix memory leak

[Ilya Dryomov]

Need to free object_prefix when rbd_dev_v2_snap_context() fails, but
only if this is the first time we are reading in the header.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 drivers/block/rbd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 324bf35ec4dd..69d03aa46d0d 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4720,7 +4720,10 @@ static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
 	}
 
 	ret = rbd_dev_v2_snap_context(rbd_dev);
-	dout("rbd_dev_v2_snap_context returned %d\n", ret);
+	if (ret && first_time) {
+		kfree(rbd_dev->header.object_prefix);
+		rbd_dev->header.object_prefix = NULL;
+	}
 
 	return ret;
 }
-- 
1.9.3

Re: [PATCH] rbd: plug rbd_dev->header.object_prefix memory leak

[Alex Elder]

On 08/31/2015 12:02 PM, Ilya Dryomov wrote:
> Need to free object_prefix when rbd_dev_v2_snap_context() fails, but
> only if this is the first time we are reading in the header.
> 
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

Yes this makes sense.  It might be nice to encapsulate
some of this, something like rbd_dev_header_info_release().
As things stand, freeing of the object prefix appears
sort of random.  It wasn't easy to follow the life
cycle of that field doing a quick scan of the code for
when it's set and cleared.

Anyway, looks good.

Reviewed-by: Alex Elder <elder@linaro.org>

> ---
>  drivers/block/rbd.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 324bf35ec4dd..69d03aa46d0d 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -4720,7 +4720,10 @@ static int rbd_dev_v2_header_info(struct rbd_device *rbd_dev)
>  	}
>  
>  	ret = rbd_dev_v2_snap_context(rbd_dev);
> -	dout("rbd_dev_v2_snap_context returned %d\n", ret);
> +	if (ret && first_time) {
> +		kfree(rbd_dev->header.object_prefix);
> +		rbd_dev->header.object_prefix = NULL;
> +	}
>  
>  	return ret;
>  }
>