Potential Memory Leak Bug in register_bcache() in Linux 5.6.0

Potential Memory Leak Bug in register_bcache() in Linux 5.6.0

[Dongyang Zhan]

Hi,

My name is Dongyang Zhan, I am a security researcher.
Currently, I found a potential memory leak bug in register_bcache() of
drivers/md/bcache/super.c.
The allocated memory regions dc and ca will not be released when
register_cache() or register_bdev() fails.
I hope you can help me to confirm this bug.

The source code and comments are as follows.
https://elixir.bootlin.com/linux/v5.6/source/drivers/md/bcache/super.c#L2253

static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
      const char *buffer, size_t size)
{
...
struct cached_dev *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
if (!dc)
    goto out_put_sb_page;

mutex_lock(&bch_register_lock);
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)
    goto out_free_sb; // If ret <0, dc will not be released.
} else {
struct cache *ca = kzalloc(sizeof(*ca), GFP_KERNEL);

if (!ca)
    goto out_put_sb_page;

/* blkdev_put() will be called in bch_cache_release() */
if (register_cache(sb, sb_disk, bdev, ca) != 0)
    goto out_free_sb; // If it fails , ca will not be released.

out_put_sb_page:
put_page(virt_to_page(sb_disk));
out_blkdev_put:
blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
out_free_sb:
kfree(sb);
out_free_path:
kfree(path);
path = NULL;
out_module_put:
module_put(THIS_MODULE);
out:
pr_info("error %s: %s", path?path:"", err);
return ret;
}

Re: Potential Memory Leak Bug in register_bcache() in Linux 5.6.0

[Coly Li]

On 2020/5/28 10:13, Dongyang Zhan wrote:
> Hi,
> 
> My name is Dongyang Zhan, I am a security researcher.
> Currently, I found a potential memory leak bug in register_bcache() of
> drivers/md/bcache/super.c.
> The allocated memory regions dc and ca will not be released when
> register_cache() or register_bdev() fails.
> I hope you can help me to confirm this bug.
> 

By a quick glance, I feel dc is OK, but ca is a bit suspicious. Normally
ca is freed when kobject_put(&ca->kobj) makes the refcount reach 0. But
if cache_alloc() fails in register_cache(), it seems ca is forgotten to
release.

Do you have any suggestion to fix ?

Thanks for the information.

Coly Li



> The source code and comments are as follows.
> https://elixir.bootlin.com/linux/v5.6/source/drivers/md/bcache/super.c#L2253
> 
> static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
>       const char *buffer, size_t size)
> {
> ...
> struct cached_dev *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
> if (!dc)
>     goto out_put_sb_page;
> 
> mutex_lock(&bch_register_lock);
> 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)
>     goto out_free_sb; // If ret <0, dc will not be released.
> } else {
> struct cache *ca = kzalloc(sizeof(*ca), GFP_KERNEL);
> 
> if (!ca)
>     goto out_put_sb_page;
> 
> /* blkdev_put() will be called in bch_cache_release() */
> if (register_cache(sb, sb_disk, bdev, ca) != 0)
>     goto out_free_sb; // If it fails , ca will not be released.
> 
> out_put_sb_page:
> put_page(virt_to_page(sb_disk));
> out_blkdev_put:
> blkdev_put(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
> out_free_sb:
> kfree(sb);
> out_free_path:
> kfree(path);
> path = NULL;
> out_module_put:
> module_put(THIS_MODULE);
> out:
> pr_info("error %s: %s", path?path:"", err);
> return ret;
> }
>