From 97bbf9e312c3fbaf0baa56120238825d2eb23b8a Mon Sep 17 00:00:00 2001 From: Denis Arefev Date: Mon, 2 Dec 2024 12:36:52 +0300 Subject: [PATCH 1/8] ubi: Add a check for ubi_num Added a check for ubi_num for negative numbers If the variable ubi_num takes negative values then we get: qemu-system-arm ... -append "ubi.mtd=0,0,0,-22222345" ... [ 0.745065] ubi_attach_mtd_dev from ubi_init+0x178/0x218 [ 0.745230] ubi_init from do_one_initcall+0x70/0x1ac [ 0.745344] do_one_initcall from kernel_init_freeable+0x198/0x224 [ 0.745474] kernel_init_freeable from kernel_init+0x18/0x134 [ 0.745600] kernel_init from ret_from_fork+0x14/0x28 [ 0.745727] Exception stack(0x90015fb0 to 0x90015ff8) Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 83ff59a06663 ("UBI: support ubi_num on mtd.ubi command line") Cc: stable@vger.kernel.org Signed-off-by: Denis Arefev Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- drivers/mtd/ubi/build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c index 30be4ed68fad..ef6a22f372f9 100644 --- a/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c @@ -1537,7 +1537,7 @@ static int ubi_mtd_param_parse(const char *val, const struct kernel_param *kp) if (token) { int err = kstrtoint(token, 10, &p->ubi_num); - if (err) { + if (err || p->ubi_num < UBI_DEV_NUM_AUTO) { pr_err("UBI error: bad value for ubi_num parameter: %s\n", token); return -EINVAL; From 923d3583ead133da742b42d9debbb7d5c5a56587 Mon Sep 17 00:00:00 2001 From: Pintu Kumar Date: Mon, 9 Dec 2024 19:29:36 +0530 Subject: [PATCH 2/8] ubifs: dump_lpt_leb: remove return at end of void function Noticed that there is a useless return statement at the end of void function dump_lpt_leb(). Just removing it. Signed-off-by: Pintu Kumar Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/lpt_commit.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ubifs/lpt_commit.c b/fs/ubifs/lpt_commit.c index aa8837e6247c..f2cb214581fd 100644 --- a/fs/ubifs/lpt_commit.c +++ b/fs/ubifs/lpt_commit.c @@ -1932,7 +1932,6 @@ static void dump_lpt_leb(const struct ubifs_info *c, int lnum) pr_err("(pid %d) finish dumping LEB %d\n", current->pid, lnum); out: vfree(buf); - return; } /** From 404de7abc05758254ad57d9501d30427d9c57417 Mon Sep 17 00:00:00 2001 From: Pintu Kumar Date: Mon, 9 Dec 2024 21:51:04 +0530 Subject: [PATCH 3/8] ubifs: ubifs_dump_leb: remove return from end of void function Noticed that there is a useless return statement at the end of void function ubifs_dump_leb(). Just removed it. Signed-off-by: Pintu Kumar Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/debug.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c index 5cc69beaa62e..987eb5b6782a 100644 --- a/fs/ubifs/debug.c +++ b/fs/ubifs/debug.c @@ -863,7 +863,6 @@ void ubifs_dump_leb(const struct ubifs_info *c, int lnum) out: vfree(buf); - return; } void ubifs_dump_znode(const struct ubifs_info *c, From 844c6fdc13cf3d9d251533631988a58f8356a8c8 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Sat, 14 Dec 2024 19:01:53 +0800 Subject: [PATCH 4/8] ubi: Revert "ubi: wl: Close down wear-leveling before nand is suspended" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 5580cdae05ae ("ubi: wl: Close down wear-leveling before nand is suspended") added a reboot notification in UBI layer to shutdown the wear-leveling subsystem, which imported an UAF problem[1]. Besides that, the method also brings other potential UAF problems, for example: reboot kworker ubi_wl_reboot_notifier ubi_wl_close ubi_fastmap_close kfree(ubi->fm) update_fastmap_work_fn ubi_update_fastmap old_fm = ubi->fm if (old_fm && old_fm->e[i]) // UAF! Actually, the problem fixed by commit 5580cdae05ae ("ubi: wl: Close down wear-leveling before nand is suspended") has been solved by commit 8cba323437a4 ("mtd: rawnand: protect access to rawnand devices while in suspend"), which was discussed in [2]. So we can revert the commit 5580cdae05ae ("ubi: wl: Close down wear-leveling before nand is suspended") directly. [1] https://lore.kernel.org/linux-mtd/20241208175211.9406-2-dennis.lamerice@gmail.com/ [2] https://lore.kernel.org/all/9bf76f5d-12a4-46ff-90d4-4a7f0f47c381@axis.com/ Fixes: 5580cdae05ae ("ubi: wl: Close down wear-leveling before nand is suspended") Reported-by: Dennis Lam Closes: https://lore.kernel.org/linux-mtd/20241208175211.9406-2-dennis.lamerice@gmail.com/ Signed-off-by: Zhihao Cheng Acked-by: Mårten Lindahl Signed-off-by: Richard Weinberger --- drivers/mtd/ubi/ubi.h | 2 -- drivers/mtd/ubi/wl.c | 21 --------------------- 2 files changed, 23 deletions(-) diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 26cc53ad34ec..c792b9bcab9b 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -549,7 +549,6 @@ struct ubi_debug_info { * @peb_buf: a buffer of PEB size used for different purposes * @buf_mutex: protects @peb_buf * @ckvol_mutex: serializes static volume checking when opening - * @wl_reboot_notifier: close all wear-leveling work before reboot * * @dbg: debugging information for this UBI device */ @@ -652,7 +651,6 @@ struct ubi_device { void *peb_buf; struct mutex buf_mutex; struct mutex ckvol_mutex; - struct notifier_block wl_reboot_notifier; struct ubi_debug_info dbg; }; diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index 4f6f339d8fb8..fbd399cf6503 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -89,7 +89,6 @@ #include #include #include -#include #include "ubi.h" #include "wl.h" @@ -128,8 +127,6 @@ static int self_check_in_wl_tree(const struct ubi_device *ubi, struct ubi_wl_entry *e, struct rb_root *root); static int self_check_in_pq(const struct ubi_device *ubi, struct ubi_wl_entry *e); -static int ubi_wl_reboot_notifier(struct notifier_block *n, - unsigned long state, void *cmd); /** * wl_tree_add - add a wear-leveling entry to a WL RB-tree. @@ -1953,13 +1950,6 @@ int ubi_wl_init(struct ubi_device *ubi, struct ubi_attach_info *ai) if (!ubi->ro_mode && !ubi->fm_disabled) ubi_ensure_anchor_pebs(ubi); #endif - - if (!ubi->wl_reboot_notifier.notifier_call) { - ubi->wl_reboot_notifier.notifier_call = ubi_wl_reboot_notifier; - ubi->wl_reboot_notifier.priority = 1; /* Higher than MTD */ - register_reboot_notifier(&ubi->wl_reboot_notifier); - } - return 0; out_free: @@ -2005,17 +1995,6 @@ void ubi_wl_close(struct ubi_device *ubi) kfree(ubi->lookuptbl); } -static int ubi_wl_reboot_notifier(struct notifier_block *n, - unsigned long state, void *cmd) -{ - struct ubi_device *ubi; - - ubi = container_of(n, struct ubi_device, wl_reboot_notifier); - ubi_wl_close(ubi); - - return NOTIFY_DONE; -} - /** * self_check_ec - make sure that the erase counter of a PEB is correct. * @ubi: UBI device description object From bdb0ca39e0acccf6771db49c3f94ed787d05f2d7 Mon Sep 17 00:00:00 2001 From: pangliyuan Date: Tue, 24 Dec 2024 16:18:23 +0800 Subject: [PATCH 5/8] ubifs: skip dumping tnc tree when zroot is null Clearing slab cache will free all znode in memory and make c->zroot.znode = NULL, then dumping tnc tree will access c->zroot.znode which cause null pointer dereference. Link: https://bugzilla.kernel.org/show_bug.cgi?id=219624#c0 Fixes: 1e51764a3c2a ("UBIFS: add new flash file system") Signed-off-by: pangliyuan Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/debug.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c index 987eb5b6782a..b01f382ce8db 100644 --- a/fs/ubifs/debug.c +++ b/fs/ubifs/debug.c @@ -945,16 +945,20 @@ void ubifs_dump_tnc(struct ubifs_info *c) pr_err("\n"); pr_err("(pid %d) start dumping TNC tree\n", current->pid); - znode = ubifs_tnc_levelorder_next(c, c->zroot.znode, NULL); - level = znode->level; - pr_err("== Level %d ==\n", level); - while (znode) { - if (level != znode->level) { - level = znode->level; - pr_err("== Level %d ==\n", level); + if (c->zroot.znode) { + znode = ubifs_tnc_levelorder_next(c, c->zroot.znode, NULL); + level = znode->level; + pr_err("== Level %d ==\n", level); + while (znode) { + if (level != znode->level) { + level = znode->level; + pr_err("== Level %d ==\n", level); + } + ubifs_dump_znode(c, znode); + znode = ubifs_tnc_levelorder_next(c, c->zroot.znode, znode); } - ubifs_dump_znode(c, znode); - znode = ubifs_tnc_levelorder_next(c, c->zroot.znode, znode); + } else { + pr_err("empty TNC tree in memory\n"); } pr_err("(pid %d) finish dumping TNC tree\n", current->pid); } From 3156ceb222414456084d964f43ada071206039b8 Mon Sep 17 00:00:00 2001 From: Rickard Andersson Date: Mon, 16 Dec 2024 09:54:19 +0100 Subject: [PATCH 6/8] ubi: Expose interface for detailed erase counters Using the ioctl command 'UBI_IOCECNFO' user space can obtain detailed erase counter information of all blocks of a device. Signed-off-by: Rickard Andersson Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- include/uapi/mtd/ubi-user.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/include/uapi/mtd/ubi-user.h b/include/uapi/mtd/ubi-user.h index e1571603175e..aa872a41ffb9 100644 --- a/include/uapi/mtd/ubi-user.h +++ b/include/uapi/mtd/ubi-user.h @@ -175,6 +175,8 @@ #define UBI_IOCRPEB _IOW(UBI_IOC_MAGIC, 4, __s32) /* Force scrubbing on the specified PEB */ #define UBI_IOCSPEB _IOW(UBI_IOC_MAGIC, 5, __s32) +/* Read detailed device erase counter information */ +#define UBI_IOCECNFO _IOWR(UBI_IOC_MAGIC, 6, struct ubi_ecinfo_req) /* ioctl commands of the UBI control character device */ @@ -412,6 +414,37 @@ struct ubi_rnvol_req { } ents[UBI_MAX_RNVOL]; } __packed; +/** + * struct ubi_ecinfo_req - a data structure used for requesting and receiving + * erase block counter information from a UBI device. + * + * @start: index of first physical erase block to read (in) + * @length: number of erase counters to read (in) + * @read_length: number of erase counters that was actually read (out) + * @padding: reserved for future, not used, has to be zeroed + * @erase_counters: array of erase counter values (out) + * + * This structure is used to retrieve erase counter information for a specified + * range of PEBs on a UBI device. + * Erase counters are read from @start and attempts to read @length number of + * erase counters. + * The retrieved values are stored in the @erase_counters array. It is the + * responsibility of the caller to allocate enough memory for storing @length + * elements in the @erase_counters array. + * If a block is bad or if the erase counter is unknown the corresponding value + * in the array will be set to -1. + * The @read_length field will indicate the number of erase counters actually + * read. Typically @read_length will be limited due to memory or the number of + * PEBs on the UBI device. + */ +struct ubi_ecinfo_req { + __s32 start; + __s32 length; + __s32 read_length; + __s8 padding[16]; + __s32 erase_counters[]; +} __packed; + /** * struct ubi_leb_change_req - a data structure used in atomic LEB change * requests. From 01099f635a4c68b8574d350a972ba062dd5142e9 Mon Sep 17 00:00:00 2001 From: Rickard Andersson Date: Mon, 16 Dec 2024 09:54:20 +0100 Subject: [PATCH 7/8] ubi: Implement ioctl for detailed erase counters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, "max_ec" can be read from sysfs, which provides a limited view of the flash device’s wear. In certain cases, such as bugs in the wear-leveling algorithm, specific blocks can be worn down more than others, resulting in uneven wear distribution. Also some use cases can wear the erase blocks of the fastmap area more heavily than other parts of flash. Providing detailed erase counter values give a better understanding of the overall flash wear and is needed to be able to calculate for example expected life time. There exists more detailed info in debugfs, but this information is only available for debug builds. Signed-off-by: Rickard Andersson Tested-by: Zhihao Cheng Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- drivers/mtd/ubi/cdev.c | 69 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c index 6bb80d7714bc..4c3e4edb6853 100644 --- a/drivers/mtd/ubi/cdev.c +++ b/drivers/mtd/ubi/cdev.c @@ -828,6 +828,69 @@ out_free: return err; } +static int ubi_get_ec_info(struct ubi_device *ubi, struct ubi_ecinfo_req __user *ureq) +{ + struct ubi_ecinfo_req req; + struct ubi_wl_entry *wl; + int read_cnt; + int peb; + int end_peb; + + /* Copy the input arguments */ + if (copy_from_user(&req, ureq, sizeof(struct ubi_ecinfo_req))) + return -EFAULT; + + /* Check input arguments */ + if (req.length <= 0 || req.start < 0 || req.start >= ubi->peb_count) + return -EINVAL; + + if (check_add_overflow(req.start, req.length, &end_peb)) + return -EINVAL; + + if (end_peb > ubi->peb_count) + end_peb = ubi->peb_count; + + /* Check access rights before filling erase_counters array */ + if (!access_ok(ureq->erase_counters, (end_peb-req.start) * sizeof(int32_t))) + return -EFAULT; + + /* Fill erase counter array */ + read_cnt = 0; + for (peb = req.start; peb < end_peb; read_cnt++, peb++) { + int ec; + + if (ubi_io_is_bad(ubi, peb)) { + if (__put_user(UBI_UNKNOWN, ureq->erase_counters+read_cnt)) + return -EFAULT; + + continue; + } + + spin_lock(&ubi->wl_lock); + + wl = ubi->lookuptbl[peb]; + if (wl) + ec = wl->ec; + else + ec = UBI_UNKNOWN; + + spin_unlock(&ubi->wl_lock); + + if (__put_user(ec, ureq->erase_counters+read_cnt)) + return -EFAULT; + + } + + /* Return actual read length */ + req.read_length = read_cnt; + + /* Copy everything except erase counter array */ + if (copy_to_user(ureq, &req, sizeof(struct ubi_ecinfo_req))) + return -EFAULT; + + return 0; +} + static long ubi_cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -991,6 +1054,12 @@ static long ubi_cdev_ioctl(struct file *file, unsigned int cmd, break; } + case UBI_IOCECNFO: + { + err = ubi_get_ec_info(ubi, argp); + break; + } + default: err = -ENOTTY; break; From 69146a8c893f734cefaac0af6f917f894f29077e Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Mon, 20 Jan 2025 12:38:24 +0800 Subject: [PATCH 8/8] ubi: ubi_get_ec_info: Fix compiling error 'cast specifies array type' On risc V platform, there is a type conversion for the return value (unsigned long type) of __untagged_addr_remote() in function untagged_addr(). The compiler will complain when the parameter 'addr' is an array type: arch/riscv/include/asm/uaccess.h:33:9: error: cast specifies array type (__force __typeof__(addr))__untagged_addr_remote(current->mm, __addr) Fix it by converting the input parameter as a pointer. Fixes: 01099f635a4c ("ubi: Implement ioctl for detailed erase counters") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202501191405.WYnmdL0U-lkp@intel.com/ Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- drivers/mtd/ubi/cdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c index 4c3e4edb6853..b700a0efaa93 100644 --- a/drivers/mtd/ubi/cdev.c +++ b/drivers/mtd/ubi/cdev.c @@ -851,7 +851,8 @@ static int ubi_get_ec_info(struct ubi_device *ubi, struct ubi_ecinfo_req __user end_peb = ubi->peb_count; /* Check access rights before filling erase_counters array */ - if (!access_ok(ureq->erase_counters, (end_peb-req.start) * sizeof(int32_t))) + if (!access_ok((void __user *)ureq->erase_counters, + (end_peb-req.start) * sizeof(int32_t))) return -EFAULT; /* Fill erase counter array */