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

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

[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@gmail.com>
Reported-by: syzbot+0988a383ae7c57b99dd9@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/67409091.050a0220.363a1b.013d.GAE@google.com/T/
---
 drivers/mtd/ubi/wl.c | 1 +
 1 file changed, 1 insertion(+)

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);
-- 
2.47.0

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

[Zhihao Cheng]

在 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@gmail.com>
> Reported-by: syzbot+0988a383ae7c57b99dd9@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/67409091.050a0220.363a1b.013d.GAE@google.com/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@axis.com/, 
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);
>