Messages in this thread | | | Subject | Re: [PATCH RFC] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier | From | Zhihao Cheng <> | Date | Thu, 12 Oct 2023 10:17:37 +0800 |
| |
在 2023/10/10 22:29, ZhaoLong Wang 写道: > If both flt.ko and gluebi.ko are loaded, the notiier of ftl > triggers NULL pointer dereference when trying to visit > ‘gluebi->desc’ in gluebi_read(). > > ubi_gluebi_init > ubi_register_volume_notifier > ubi_enumerate_volumes > ubi_notify_all > gluebi_notify nb->notifier_call() > gluebi_create > mtd_device_register > mtd_device_parse_register > add_mtd_device > blktrans_notify_add not->add() > ftl_add_mtd tr->add_mtd() > scan_header > mtd_read > mtd_read > mtd_read_oob > gluebi_read mtd->read() > gluebi->desc - NULL > > Detailed reproduction information available at the link[1], > > In the normal case, obtain gluebi->desc in the gluebi_get_device(), > and accesses gluebi->desc in the gluebi_read(). However, > gluebi_get_device() is not executed in advance in the > ftl_add_mtd() process, which leads to null pointer dereference. > > This patch assumes that the gluebi module is not designed to work with > the ftl module. In this case, the patch only needs to prevent the ftl > notifier operation.
We can't refuse ftl when gluebi is loaded, it looks weird: mtd0(nand) -> ftl0 mtd1(gluebi) has no ftl1?
When FTL is loaded, it should make sure each mtd device correspond to a block device, no matter which type the mtd device is(nand or gluebi).
So I prefer the root cause is gluebi->desc is not initialized in gluebi_create->mtd_device_register.
> > Add some correctness check for gluebi->desc in gluebi_read/write/erase(), > If the pointer is invalid, the -EINVAL is returned. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217992 [1] > Signed-off-by: ZhaoLong Wang <wangzhaolong1@huawei.com> > --- > drivers/mtd/ubi/gluebi.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c > index 1b980d15d9fb..189ecc0eacd1 100644 > --- a/drivers/mtd/ubi/gluebi.c > +++ b/drivers/mtd/ubi/gluebi.c > @@ -157,6 +157,9 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len, > struct gluebi_device *gluebi; > > gluebi = container_of(mtd, struct gluebi_device, mtd); > + if (IS_ERR_OR_NULL(gluebi->desc)) > + return -EINVAL; > + > lnum = div_u64_rem(from, mtd->erasesize, &offs); > bytes_left = len; > while (bytes_left) { > @@ -197,6 +200,9 @@ static int gluebi_write(struct mtd_info *mtd, loff_t to, size_t len, > struct gluebi_device *gluebi; > > gluebi = container_of(mtd, struct gluebi_device, mtd); > + if (IS_ERR_OR_NULL(gluebi->desc)) > + return -EINVAL; > + > lnum = div_u64_rem(to, mtd->erasesize, &offs); > > if (len % mtd->writesize || offs % mtd->writesize) > @@ -242,6 +248,8 @@ static int gluebi_erase(struct mtd_info *mtd, struct erase_info *instr) > lnum = mtd_div_by_eb(instr->addr, mtd); > count = mtd_div_by_eb(instr->len, mtd); > gluebi = container_of(mtd, struct gluebi_device, mtd); > + if (IS_ERR_OR_NULL(gluebi->desc)) > + return -EINVAL; > > for (i = 0; i < count - 1; i++) { > err = ubi_leb_unmap(gluebi->desc, lnum + i); >
| |