| Message ID | 20231208074629.1656356-1-linan666@huaweicloud.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] ubi: block: fix memleak in ubiblock_create() | expand |
friendly ping... 在 2023/12/8 15:46, linan666@huaweicloud.com 写道: > From: Li Nan <linan122@huawei.com> > > If idr_alloc() fails, dev->gd will be put after goto out_cleanup_disk in > ubiblock_create(), but dev->gd has not been assigned yet at this time, and > 'gd' will not be put anymore. Fix it by putting 'gd' directly. > > Signed-off-by: Li Nan <linan122@huawei.com> > Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> > --- > Changes in v2: > - modify the description of the problem in log > > drivers/mtd/ubi/block.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c > index 309a42aeaa4c..654bd7372cd8 100644 > --- a/drivers/mtd/ubi/block.c > +++ b/drivers/mtd/ubi/block.c > @@ -434,7 +434,7 @@ int ubiblock_create(struct ubi_volume_info *vi) > list_del(&dev->list); > idr_remove(&ubiblock_minor_idr, gd->first_minor); > out_cleanup_disk: > - put_disk(dev->gd); > + put_disk(gd); > out_free_tags: > blk_mq_free_tag_set(&dev->tag_set); > out_free_dev:
friendly ping ... 在 2023/12/8 15:46, linan666@huaweicloud.com 写道: > From: Li Nan <linan122@huawei.com> > > If idr_alloc() fails, dev->gd will be put after goto out_cleanup_disk in > ubiblock_create(), but dev->gd has not been assigned yet at this time, and > 'gd' will not be put anymore. Fix it by putting 'gd' directly. > > Signed-off-by: Li Nan <linan122@huawei.com> > Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com> > --- > Changes in v2: > - modify the description of the problem in log > > drivers/mtd/ubi/block.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c > index 309a42aeaa4c..654bd7372cd8 100644 > --- a/drivers/mtd/ubi/block.c > +++ b/drivers/mtd/ubi/block.c > @@ -434,7 +434,7 @@ int ubiblock_create(struct ubi_volume_info *vi) > list_del(&dev->list); > idr_remove(&ubiblock_minor_idr, gd->first_minor); > out_cleanup_disk: > - put_disk(dev->gd); > + put_disk(gd); > out_free_tags: > blk_mq_free_tag_set(&dev->tag_set); > out_free_dev:
Can it be nicer to use the term “memory leak” instead of “memleak”
in the patch subject?
…
> 'gd' will not be put anymore. Fix it by putting 'gd' directly.
Would the following wording variant be more desirable?
… anymore.
Thus pass the variable “gd” to a put_disk() call directly.
Would you like to add the tag “Fixes” accordingly?
Regards,
Markus
在 2024/4/14 17:16, Markus Elfring 写道: > Can it be nicer to use the term “memory leak” instead of “memleak” > in the patch subject? > > > … >> 'gd' will not be put anymore. Fix it by putting 'gd' directly. > > Would the following wording variant be more desirable? > > … anymore. > Thus pass the variable “gd” to a put_disk() call directly. > > > Would you like to add the tag “Fixes” accordingly? > The patch has already been applied: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=adbf4c4954e33e623897058a617c583d65a177f6 Thank you very much for your suggestion. I will pay attention to them in future patches. > Regards, > Markus > .
On Fri, Dec 08, 2023 at 03:46:29PM +0800, linan666@huaweicloud.com wrote: > From: Li Nan <linan122@huawei.com> > > If idr_alloc() fails, dev->gd will be put after goto out_cleanup_disk in > ubiblock_create(), but dev->gd has not been assigned yet at this time, and > 'gd' will not be put anymore. Fix it by putting 'gd' directly. > There is another invalid reference to dev->gd if blk_mq_alloc_tag_set() fails. dev_err(disk_to_dev(dev->gd), "blk_mq_alloc_tag_set failed"); ^^^^^^^ regards, dan carpenter
Hi, dan, 在 2024/4/15 14:33, Dan Carpenter 写道: > On Fri, Dec 08, 2023 at 03:46:29PM +0800, linan666@huaweicloud.com wrote: >> From: Li Nan <linan122@huawei.com> >> >> If idr_alloc() fails, dev->gd will be put after goto out_cleanup_disk in >> ubiblock_create(), but dev->gd has not been assigned yet at this time, and >> 'gd' will not be put anymore. Fix it by putting 'gd' directly. >> > > There is another invalid reference to dev->gd if blk_mq_alloc_tag_set() > fails. > > dev_err(disk_to_dev(dev->gd), "blk_mq_alloc_tag_set failed"); > ^^^^^^^ > You are really very careful! I will fix it later. > regards, > dan carpenter
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c index 309a42aeaa4c..654bd7372cd8 100644 --- a/drivers/mtd/ubi/block.c +++ b/drivers/mtd/ubi/block.c @@ -434,7 +434,7 @@ int ubiblock_create(struct ubi_volume_info *vi) list_del(&dev->list); idr_remove(&ubiblock_minor_idr, gd->first_minor); out_cleanup_disk: - put_disk(dev->gd); + put_disk(gd); out_free_tags: blk_mq_free_tag_set(&dev->tag_set); out_free_dev: