Re: [PATCH] MTD: fix slab-use-after-free due to dangling pointer in notifier chain

From: Zhihao Cheng
Date: Mon Dec 09 2024 - 00:03:13 EST


在 2024/12/9 1:52, Dennis Lam 写道:
When attaching MTDs, a reboot notifier is added to the blocking
notification chain. However, when detaching a MTD and freeing its
related objects, the notifier is never unregistered from the
notification chain and is freed during the detaching process. This
results in the slab-use-after-free.

To fix this, I added an unregister notifier function in the ubi_wl_close
function to ensure that the notifier is removed from the chain after the
device has been detached.

Signed-off-by: Dennis Lam <dennis.lamerice@xxxxxxxxx>
Reported-by: syzbot+0988a383ae7c57b99dd9@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://lore.kernel.org/all/67409091.050a0220.363a1b.013d.GAE@xxxxxxxxxx/T/
---
drivers/mtd/ubi/wl.c | 1 +
1 file changed, 1 insertion(+)

Hi Dennis, thanks for reporting it! It is an actual bug and I can reproduce it easily:
[ 232.811608] ==================================================================
[ 232.812392] BUG: KASAN: slab-use-after-free in notifier_call_chain+0x2b0/0x300
[ 232.813195] Read of size 8 at addr ffff888109fd92e0 by task systemd-shutdow/1
[ 232.813979]
[ 232.814162] CPU: 1 UID: 0 PID: 1 Comm: systemd-shutdow Not tainted 6.13.0-rc2-00019-g580644ec5adf-dirty #54
[ 232.815224] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014
[ 232.816184] Call Trace:
[ 232.816466] <TASK>
[ 232.816713] dump_stack_lvl+0x9f/0xf0
[ 232.817122] print_report+0x21e/0x76f
[ 232.819952] __asan_report_load8_noabort+0x24/0x40
[ 232.820487] notifier_call_chain+0x2b0/0x300
[ 232.820982] blocking_notifier_call_chain+0x6a/0xc0
[ 232.821531] kernel_restart_prepare+0x25/0xa0
[ 232.822022] kernel_restart+0x1a/0x60
[ 232.822435] __do_sys_reboot+0x21b/0x3c0
[ 232.825632] __x64_sys_reboot+0x9e/0x110
[ 232.826616] x64_sys_call+0x1315/0x4150
[ 232.827047] do_syscall_64+0xa7/0x210
[ 232.827462] entry_SYSCALL_64_after_hwframe+0x76/0x7e

And I think this patch could fix the single problem. Before that, I find the commit 5580cdae05aefa96deebd7f5ade9d70c92adabd7("ubi: wl: Close down wear-leveling before nand is suspended") is applied by a mistake. Hi, Richard, the patch has been discussed in https://lore.kernel.org/all/9bf76f5d-12a4-46ff-90d4-4a7f0f47c381@xxxxxxxx/, and the patch is not needed now, otherwise, there will be other UAF problems(in fm worker). If you agree, I will send a patch to revert it.

diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 4f6f339d8fb8..31a1e5515d98 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -1995,6 +1995,7 @@ static void protection_queue_destroy(struct ubi_device *ubi)
void ubi_wl_close(struct ubi_device *ubi)
{
dbg_wl("close the WL sub-system");
+ unregister_reboot_notifier(&ubi->wl_reboot_notifier);
ubi_fastmap_close(ubi);
shutdown_work(ubi);
protection_queue_destroy(ubi);