| Message ID | 20221027091918.2294132-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rbd: fix possible memory leak in rbd_sysfs_init() | expand |
On 10/27/22 4:19 AM, Yang Yingliang wrote: > If device_register() returns error in rbd_sysfs_init(), name of kobject > which is allocated in dev_set_name() called in device_add() is leaked. > > As comment of device_add() says, it should call put_device() to drop > the reference count that was set in device_initialize() when it fails, > so the name can be freed in kobject_cleanup(). > > Fault injection test can trigger this problem: > > unreferenced object 0xffff88810173aa78 (size 8): > comm "modprobe", pid 247, jiffies 4294714278 (age 31.789s) > hex dump (first 8 bytes): > 72 62 64 00 81 88 ff ff rbd..... > backtrace: > [<00000000f58fae56>] __kmalloc_node_track_caller+0x44/0x1b0 > [<00000000bdd44fe7>] kstrdup+0x3a/0x70 > [<00000000f7844d0b>] kstrdup_const+0x63/0x80 > [<000000001b0a0eeb>] kvasprintf_const+0x10b/0x190 > [<00000000a47bd894>] kobject_set_name_vargs+0x56/0x150 > [<00000000d5edbf18>] dev_set_name+0xab/0xe0 > [<00000000f5153e80>] device_add+0x106/0x1f20 > > Fixes: dfc5606dc513 ("rbd: replace the rbd sysfs interface") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> This looks right to me. Reviewed-by: Alex Elder <elder@linaro.org> > --- > drivers/block/rbd.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index f9e39301c4af..04453f4a319c 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -7222,8 +7222,10 @@ static int __init rbd_sysfs_init(void) > int ret; > > ret = device_register(&rbd_root_dev); > - if (ret < 0) > + if (ret < 0) { > + put_device(&rbd_root_dev); > return ret; > + } > > ret = bus_register(&rbd_bus_type); > if (ret < 0)
On Thu, 27 Oct 2022 17:19:18 +0800, Yang Yingliang wrote: > If device_register() returns error in rbd_sysfs_init(), name of kobject > which is allocated in dev_set_name() called in device_add() is leaked. > > As comment of device_add() says, it should call put_device() to drop > the reference count that was set in device_initialize() when it fails, > so the name can be freed in kobject_cleanup(). > > [...] Applied, thanks! [1/1] rbd: fix possible memory leak in rbd_sysfs_init() (no commit info) Best regards,
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index f9e39301c4af..04453f4a319c 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -7222,8 +7222,10 @@ static int __init rbd_sysfs_init(void) int ret; ret = device_register(&rbd_root_dev); - if (ret < 0) + if (ret < 0) { + put_device(&rbd_root_dev); return ret; + } ret = bus_register(&rbd_bus_type); if (ret < 0)
If device_register() returns error in rbd_sysfs_init(), name of kobject which is allocated in dev_set_name() called in device_add() is leaked. As comment of device_add() says, it should call put_device() to drop the reference count that was set in device_initialize() when it fails, so the name can be freed in kobject_cleanup(). Fault injection test can trigger this problem: unreferenced object 0xffff88810173aa78 (size 8): comm "modprobe", pid 247, jiffies 4294714278 (age 31.789s) hex dump (first 8 bytes): 72 62 64 00 81 88 ff ff rbd..... backtrace: [<00000000f58fae56>] __kmalloc_node_track_caller+0x44/0x1b0 [<00000000bdd44fe7>] kstrdup+0x3a/0x70 [<00000000f7844d0b>] kstrdup_const+0x63/0x80 [<000000001b0a0eeb>] kvasprintf_const+0x10b/0x190 [<00000000a47bd894>] kobject_set_name_vargs+0x56/0x150 [<00000000d5edbf18>] dev_set_name+0xab/0xe0 [<00000000f5153e80>] device_add+0x106/0x1f20 Fixes: dfc5606dc513 ("rbd: replace the rbd sysfs interface") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/block/rbd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)