From: Jing Xia <[email protected]>
The lock_class_key is still registered and can be found in
lock_keys_hash hlist after subsys_private is freed in error
handler path.A task who iterate over the lock_keys_hash
later may cause use-after-free.So fix that up and unregister
the lock_class_key before kfree(cp).
Signed-off-by: Jing Xia <[email protected]>
Signed-off-by: Xuewen Yan <[email protected]>
---
drivers/base/class.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/base/class.c b/drivers/base/class.c
index 7e78aee0fd6c..7b38fdf8e1d7 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -213,6 +213,7 @@ int class_register(const struct class *cls)
return 0;
err_out:
+ lockdep_unregister_key(key);
kfree(cp);
return error;
}
--
2.25.1
On Mon, Dec 18, 2023 at 10:44:03AM +0800, Chunyan Zhang wrote:
> From: Jing Xia <[email protected]>
>
> The lock_class_key is still registered and can be found in
> lock_keys_hash hlist after subsys_private is freed in error
> handler path.A task who iterate over the lock_keys_hash
> later may cause use-after-free.So fix that up and unregister
> the lock_class_key before kfree(cp).
What task iterates over all hashes?
And can you put ' ' after your '.'?
And how was this found?
>
> Signed-off-by: Jing Xia <[email protected]>
> Signed-off-by: Xuewen Yan <[email protected]>
What commit id does this fix?
Also note in the changelog that this only can happen if lockdep is
enabled, which is not true for normal systems.
thanks,
greg k-h
On Mon, Dec 18, 2023 at 07:52:18AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Dec 18, 2023 at 10:44:03AM +0800, Chunyan Zhang wrote:
> > From: Jing Xia <[email protected]>
> >
> > The lock_class_key is still registered and can be found in
> > lock_keys_hash hlist after subsys_private is freed in error
> > handler path.A task who iterate over the lock_keys_hash
> > later may cause use-after-free.So fix that up and unregister
> > the lock_class_key before kfree(cp).
>
> What task iterates over all hashes?
>
> And can you put ' ' after your '.'?
>
> And how was this found?
And more importantly, how was this tested?
thanks,
greg k-h
On Mon, Dec 18, 2023 at 2:52 PM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Mon, Dec 18, 2023 at 10:44:03AM +0800, Chunyan Zhang wrote:
> > From: Jing Xia <[email protected]>
> >
> > The lock_class_key is still registered and can be found in
> > lock_keys_hash hlist after subsys_private is freed in error
> > handler path.A task who iterate over the lock_keys_hash
> > later may cause use-after-free.So fix that up and unregister
> > the lock_class_key before kfree(cp).
>
> What task iterates over all hashes?
>
> And can you put ' ' after your '.'?
>
> And how was this found?
>
Thanks for your comments. I'll add more information in the changelog.
On our platform, a driver fails to kset_register because of creating
duplicate filename '/class/xxx'.And we got serval kernel panic issues
about alignment fault on stability tests.All backtraces show that it is
manipulating the corrupted lock_keys_hash hlish when a workqueue is
created or released.
Here is one backtrace:
Unable to handle kernel paging request at virtual address ffffffc081480bae
Mem abort info:
ESR = 0x0000000096000021
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x21: alignment fault
...
Call trace:
lockdep_register_key+0x128/0x22c
alloc_workqueue+0x190/0x640
loop_configure+0x2f0/0x560
lo_ioctl+0x70c/0xf60
blkdev_ioctl+0x290/0x88c
__arm64_sys_ioctl+0xa8/0xe4
And we are aware of the fact that it might be a use-after-free issue.
So we enable Kasan and it gives a invalid-access bug report.
BUG: KASAN: invalid-access in lockdep_register_key+0x19c/0x1bc
Write of size 8 at addr 15ffff808b8c0368 by task modprobe/252
Pointer tag: [15], memory tag: [fe]
CPU: 7 PID: 252 Comm: modprobe Tainted: G W
6.6.0-mainline-maybe-dirty #1
Call trace:
dump_backtrace+0x1b0/0x1e4
show_stack+0x2c/0x40
dump_stack_lvl+0xac/0xe0
print_report+0x18c/0x4d8
kasan_report+0xe8/0x148
__hwasan_store8_noabort+0x88/0x98
lockdep_register_key+0x19c/0x1bc
class_register+0x94/0x1ec
init_module+0xbc/0xf48 [rfkill]
do_one_initcall+0x17c/0x72c
do_init_module+0x19c/0x3f8
load_module+0x2300/0x2394
__arm64_sys_finit_module+0x350/0x4d8
invoke_syscall+0x88/0x1f0
el0_svc_common+0xe8/0x1c0
do_el0_svc+0x34/0x44
el0_svc+0x50/0xb8
el0t_64_sync_handler+0x68/0xbc
el0t_64_sync+0x19c/0x1a0
The buggy address belongs to the object at ffffff808b8c0000
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 232 bytes to the right of
640-byte region [ffffff808b8c0000, ffffff808b8c0280)
The buggy address belongs to the physical page:
page:00000000918c4834 refcount:1 mapcount:0 mapping:0000000000000000
index:0x0 pfn:0x10b8c0
head:00000000918c4834 order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
anon flags: 0x4000000000000840(slab|head|zone=1|kasantag=0x0)
page_type: 0xffffffff()
raw: 4000000000000840 80ffff8080002a00 0000000000000000 0000000000000001
raw: 0000000000000000 0000000000200020 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffffff808b8c0100: 8a 8a 8a 8a 8a 8a 8a 8a 8a 8a 8a 8a 8a 8a 8a 8a
ffffff808b8c0200: 8a 8a 8a 8a 8a 8a 8a 8a fe fe fe fe fe fe fe fe
>ffffff808b8c0300: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
^
> >
> > Signed-off-by: Jing Xia <[email protected]>
> > Signed-off-by: Xuewen Yan <[email protected]>
>
> What commit id does this fix?
>
> Also note in the changelog that this only can happen if lockdep is
> enabled, which is not true for normal systems.
>
I'll fix it on v1.Thanks.
> thanks,
>
> greg k-h