Received-SPF: pass (google.com: domain of linux-kernel+bounces-3351-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20231218024403.1076134-1-chunyan.zhang@unisoc.com> <2023121830-proxy-washed-ae4d@gregkh>
In-Reply-To: <2023121830-proxy-washed-ae4d@gregkh>
From: jing xia <jing.xia.mail@gmail.com>
Date: Mon, 18 Dec 2023 18:41:09 +0800
Message-ID: <CAN=25QNcd3y+9VmRk_L7JYZTYD++bgZ=YizpdWncmNnfoDD1cA@mail.gmail.com>
Subject: Re: [PATCH] class: fix use-after-free in class_register()
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Chunyan Zhang <chunyan.zhang@unisoc.com>, "Rafael J . Wysocki" <rafael@kernel.org>, 
	LKML <linux-kernel@vger.kernel.org>, Jing Xia <jing.xia@unisoc.com>, 
	Xuewen Yan <xuewen.yan@unisoc.com>, Ke Wang <ke.wang@unisoc.com>, 
	Chunyan Zhang <zhang.lyra@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Dec 18, 2023 at 2:52=E2=80=AFPM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Dec 18, 2023 at 10:44:03AM +0800, Chunyan Zhang wrote:
> > From: Jing Xia <jing.xia@unisoc.com>
> >
> > 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 =3D 0x0000000096000021
   EC =3D 0x25: DABT (current EL), IL =3D 32 bits
   SET =3D 0, FnV =3D 0
   EA =3D 0, S1PTW =3D 0
   FSC =3D 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=3D1|kasantag=3D0x0)
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 <jing.xia@unisoc.com>
> > Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
>
> 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