From 97bbf9e312c3fbaf0baa56120238825d2eb23b8a Mon Sep 17 00:00:00 2001 From: Denis Arefev Date: Mon, 2 Dec 2024 12:36:52 +0300 Subject: 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(-) (limited to 'drivers/mtd') 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; -- cgit v1.2.3 From 844c6fdc13cf3d9d251533631988a58f8356a8c8 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Sat, 14 Dec 2024 19:01:53 +0800 Subject: 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(-) (limited to 'drivers/mtd') 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 -- cgit v1.2.3 From 01099f635a4c68b8574d350a972ba062dd5142e9 Mon Sep 17 00:00:00 2001 From: Rickard Andersson Date: Mon, 16 Dec 2024 09:54:20 +0100 Subject: 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(+) (limited to 'drivers/mtd') 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; -- cgit v1.2.3 From 69146a8c893f734cefaac0af6f917f894f29077e Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Mon, 20 Jan 2025 12:38:24 +0800 Subject: 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(-) (limited to 'drivers/mtd') 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 */ -- cgit v1.2.3