Re: [PATCH] bcache: Fix potential memory leak in register_bcache()

[Coly Li]

On 2020/11/9 17:28, Tiezhu Yang wrote:
> Call kfree() in the error path to free the memory allocated by kzalloc().
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>

Hi Tiezhu,

NACK, dc is freed in bch_cached_dev_release() and ca is freed in
bch_cache_release().

Indeed you are not the first or second who tried to fix here. The error
handling code path to release the memory objects are implicit.

Thanks.

Coly Li


> ---
>  drivers/md/bcache/super.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index 46a0013..af51574 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2593,8 +2593,10 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  		ret = register_bdev(sb, sb_disk, bdev, dc);
>  		mutex_unlock(&bch_register_lock);
>  		/* blkdev_put() will be called in cached_dev_free() */
> -		if (ret < 0)
> +		if (ret < 0) {
> +			kfree(dc);
>  			goto out_free_sb;
> +		}
>  	} else {
>  		struct cache *ca = kzalloc(sizeof(*ca), GFP_KERNEL);
>  
> @@ -2602,8 +2604,10 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>  			goto out_put_sb_page;
>  
>  		/* blkdev_put() will be called in bch_cache_release() */
> -		if (register_cache(sb, sb_disk, bdev, ca) != 0)
> +		if (register_cache(sb, sb_disk, bdev, ca) != 0) {
> +			kfree(ca);
>  			goto out_free_sb;
> +		}
>  	}
>  
>  done:
>