[PATCH V2] class: fix use-after-free in class_register()

[PATCH V2] class: fix use-after-free in class_register()

[Jing Xia]

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).

On our platform, a driver fails to kset_register because of
creating duplicate filename '/class/xxx'.With Kasan enabled,
it prints a invalid-access bug report.

KASAN 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
...
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
                                     ^
ffffff808b8c0400: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03

As CONFIG_KASAN_GENERIC is not set, Kasan reports invalid-access
not use-after-free here.In this case, modprobe is manipulating
the corrupted lock_keys_hash hlish where lock_class_key is already
freed before.

It's worth noting that this only can happen if lockdep is enabled,
which is not true for normal system.

Fixes: dcfbb67e48a2 ("driver core: class: use lock_class_key already present in struct subsys_private")
Signed-off-by: Jing Xia <jing.xia@unisoc.com>
Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
---
 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

Re: [PATCH V2] class: fix use-after-free in class_register()

[Greg Kroah-Hartman]

On Wed, Dec 20, 2023 at 10:46:03AM +0800, Jing Xia wrote:
> 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).
> 
> On our platform, a driver fails to kset_register because of
> creating duplicate filename '/class/xxx'.With Kasan enabled,
> it prints a invalid-access bug report.
> 
> KASAN 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
> ...
> 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
>                                      ^
> ffffff808b8c0400: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03
> 
> As CONFIG_KASAN_GENERIC is not set, Kasan reports invalid-access
> not use-after-free here.In this case, modprobe is manipulating
> the corrupted lock_keys_hash hlish where lock_class_key is already
> freed before.
> 
> It's worth noting that this only can happen if lockdep is enabled,
> which is not true for normal system.
> 
> Fixes: dcfbb67e48a2 ("driver core: class: use lock_class_key already present in struct subsys_private")
> Signed-off-by: Jing Xia <jing.xia@unisoc.com>
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> ---
>  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
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot