Messages in this thread Patch in this message | | | From | ZhaoLong Wang <> | Subject | [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier | Date | Wed, 18 Oct 2023 20:16:18 +0800 |
| |
If both flt.ko and gluebi.ko are loaded, the notiier of ftl triggers NULL pointer dereference when trying to access ‘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.
The value of gluebi->desc may also be a negative error code, which triggers the page fault error.
This patch has the following modifications:
1. Do not assign gluebi->desc to the error code. Use the NULL instead.
2. Always check the validity of gluebi->desc in gluebi_read() If the gluebi->desc is NULL, try to get MTD device.
Such a modification currently works because the mutex "mtd_table_mutex" is held on all necessary paths, including the ftl_add_mtd() call path, open and close paths. Therefore, many race condition can be avoided.
Fixes: 2ba3d76a1e29 ("UBI: make gluebi a separate module") Link: https://bugzilla.kernel.org/show_bug.cgi?id=217992 [1] Signed-off-by: ZhaoLong Wang <wangzhaolong1@huawei.com> --- drivers/mtd/ubi/gluebi.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c index 1b980d15d9fb..0ca7f104adbf 100644 --- a/drivers/mtd/ubi/gluebi.c +++ b/drivers/mtd/ubi/gluebi.c @@ -85,6 +85,7 @@ static int gluebi_get_device(struct mtd_info *mtd) { struct gluebi_device *gluebi; int ubi_mode = UBI_READONLY; + struct ubi_volume_desc *vdesc; if (mtd->flags & MTD_WRITEABLE) ubi_mode = UBI_READWRITE; @@ -109,12 +110,14 @@ static int gluebi_get_device(struct mtd_info *mtd) * This is the first reference to this UBI volume via the MTD device * interface. Open the corresponding volume in read-write mode. */ - gluebi->desc = ubi_open_volume(gluebi->ubi_num, gluebi->vol_id, + vdesc = ubi_open_volume(gluebi->ubi_num, gluebi->vol_id, ubi_mode); - if (IS_ERR(gluebi->desc)) { + if (IS_ERR(vdesc)) { + gluebi->desc = NULL; mutex_unlock(&devices_mutex); - return PTR_ERR(gluebi->desc); + return PTR_ERR(vdesc); } + gluebi->desc = vdesc; gluebi->refcnt += 1; mutex_unlock(&devices_mutex); return 0; @@ -134,8 +137,10 @@ static void gluebi_put_device(struct mtd_info *mtd) gluebi = container_of(mtd, struct gluebi_device, mtd); mutex_lock(&devices_mutex); gluebi->refcnt -= 1; - if (gluebi->refcnt == 0) + if (gluebi->refcnt == 0) { ubi_close_volume(gluebi->desc); + gluebi->desc = NULL; + } mutex_unlock(&devices_mutex); } @@ -154,9 +159,26 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, unsigned char *buf) { int err = 0, lnum, offs, bytes_left; - struct gluebi_device *gluebi; + struct gluebi_device *gluebi = container_of(mtd, struct gluebi_device, + mtd); + int no_desc = gluebi->desc == NULL ? 1 : 0; + + /** + * In normal case, the UBI volume desc has been initialized by + * ->_get_device(). However, in the ftl notifier process, the + * ->_get_device() is not executed in advance and the MTD device + * is directly scanned which cause NULL pointer dereference. + * Therefore, try to get the MTD device here. + */ + if (unlikely(no_desc)) { + err = __get_mtd_device(mtd); + if (err) { + err_msg("cannot get MTD device %d, UBI device %d, volume %d, error %d", + mtd->index, gluebi->ubi_num, gluebi->vol_id, err); + return err; + } + } - gluebi = container_of(mtd, struct gluebi_device, mtd); lnum = div_u64_rem(from, mtd->erasesize, &offs); bytes_left = len; while (bytes_left) { @@ -176,6 +198,9 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len, } *retlen = len - bytes_left; + + if (unlikely(no_desc)) + __put_mtd_device(mtd); return err; } -- 2.31.1
| |