Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 6 May 2024 21:35:22 +0800 | Subject | Re: [PATCH] ubi: block: fix null-pointer-dereference in ubiblock_create() | From | Li Nan <> |
| |
在 2024/4/28 15:51, Zhihao Cheng 写道: > 在 2024/4/28 15:19, linan666@huaweicloud.com 写道: >> From: Li Nan <linan122@huawei.com> >> >> Similar to commit adbf4c4954e3 ("ubi: block: fix memleak in >> ubiblock_create()"), 'dev->gd' is not assigned but dereferenced if >> blk_mq_alloc_tag_set() fails, and leading to a null-pointer-dereference. >> >> Using 'gd' directly here is not a good idea, too. 'gd->part0.bd_device' >> is not initialized at this point. The error log will be: >> block (null): block: dynamic minor allocation failed >> >> Fix it by using pr_err() and print ubi id. >> >> Fixes: 77567b25ab9f ("ubi: use blk_mq_alloc_disk and blk_cleanup_disk") >> Signed-off-by: Li Nan <linan122@huawei.com> >> --- >> drivers/mtd/ubi/block.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c >> index f82e3423acb9..bf7308e8ec2f 100644 >> --- a/drivers/mtd/ubi/block.c >> +++ b/drivers/mtd/ubi/block.c >> @@ -390,7 +390,8 @@ int ubiblock_create(struct ubi_volume_info *vi) >> ret = blk_mq_alloc_tag_set(&dev->tag_set); >> if (ret) { >> - dev_err(disk_to_dev(dev->gd), "blk_mq_alloc_tag_set failed"); >> + pr_err("ubiblock%d_%d: blk_mq_alloc_tag_set failed\n", >> + dev->ubi_num, dev->vol_id); >> goto out_free_dev; >> } >> @@ -407,8 +408,8 @@ int ubiblock_create(struct ubi_volume_info *vi) >> gd->minors = 1; >> gd->first_minor = idr_alloc(&ubiblock_minor_idr, dev, 0, 0, >> GFP_KERNEL); > > There is no need to modify this place. The device of 'gd' is initialized in > blk_mq_alloc_disk. Refer to nbd_dev_add.
The print of this dev_err() is: block (null): block: dynamic minor allocation failed ~~~~~~ There is no information about which device failed. dev_err() use 'kobject_name(&dev->kobj)' here, which is init in device_add_disk() later. So use pr_err() here can print more detail.
I will split the patch into two in v2 to make it clearer.
>> if (gd->first_minor < 0) { >> - dev_err(disk_to_dev(gd), >> - "block: dynamic minor allocation failed"); >> + pr_err("ubiblock%d_%d: block: dynamic minor allocation failed\n", >> + dev->ubi_num, dev->vol_id); >> ret = -ENODEV; >> goto out_cleanup_disk; >> } >>
-- Thanks, Nan
| ![\](/images/icornerr.gif) |