* [PATCH 0/2] HID: uclogic: Fix two bugs in uclogic
@ 2023-09-21 13:38 Jinjie Ruan
2023-09-21 13:38 ` [PATCH 1/2] HID: uclogic: Fix user-memory-access bug in uclogic_params_ugee_v2_init_event_hooks() Jinjie Ruan
2023-09-21 13:38 ` [PATCH 2/2] HID: uclogic: Fix a work->entry not empty bug in __queue_work() Jinjie Ruan
0 siblings, 2 replies; 6+ messages in thread
From: Jinjie Ruan @ 2023-09-21 13:38 UTC (permalink / raw)
To: jikos, benjamin.tissoires, jose.exposito89, linux-input; +Cc: ruanjinjie
When CONFIG_HID_UCLOGIC=y and CONFIG_KUNIT_ALL_TESTS=y, launch
kernel and then there are a user-memory-access bug and a work->entry
not empty bug. This patchset fix these issues.
Jinjie Ruan (2):
HID: uclogic: Fix user-memory-access bug in
uclogic_params_ugee_v2_init_event_hooks()
HID: uclogic: Fix a work->entry not empty bug in __queue_work()
drivers/hid/hid-uclogic-core-test.c | 7 +++++++
drivers/hid/hid-uclogic-params-test.c | 15 ++++++++++++++-
2 files changed, 21 insertions(+), 1 deletion(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] HID: uclogic: Fix user-memory-access bug in uclogic_params_ugee_v2_init_event_hooks() 2023-09-21 13:38 [PATCH 0/2] HID: uclogic: Fix two bugs in uclogic Jinjie Ruan @ 2023-09-21 13:38 ` Jinjie Ruan 2023-10-08 17:21 ` José Expósito 2023-09-21 13:38 ` [PATCH 2/2] HID: uclogic: Fix a work->entry not empty bug in __queue_work() Jinjie Ruan 1 sibling, 1 reply; 6+ messages in thread From: Jinjie Ruan @ 2023-09-21 13:38 UTC (permalink / raw) To: jikos, benjamin.tissoires, jose.exposito89, linux-input; +Cc: ruanjinjie When CONFIG_HID_UCLOGIC=y and CONFIG_KUNIT_ALL_TESTS=y, launch kernel and then the below user-memory-access bug occurs. In hid_test_uclogic_params_cleanup_event_hooks(),it call uclogic_params_ugee_v2_init_event_hooks() with the first arg=NULL, so when it calls uclogic_params_ugee_v2_has_battery(), the hid_get_drvdata() will access hdev->dev with hdev=NULL, which will cause below user-memory-access. So add a fake_device with quirks member and call hid_set_drvdata() to assign hdev->dev->driver_data which avoids the null-ptr-def bug for drvdata->quirks in uclogic_params_ugee_v2_has_battery(). After applying this patch, the below user-memory-access bug never occurs. general protection fault, probably for non-canonical address 0xdffffc0000000329: 0000 [#1] PREEMPT SMP KASAN KASAN: probably user-memory-access in range [0x0000000000001948-0x000000000000194f] CPU: 5 PID: 2189 Comm: kunit_try_catch Tainted: G B W N 6.6.0-rc2+ #30 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 RIP: 0010:uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 Code: f3 f3 65 48 8b 14 25 28 00 00 00 48 89 54 24 60 31 d2 48 89 fa c7 44 24 30 00 00 00 00 48 c7 44 24 28 02 f8 02 01 48 c1 ea 03 <80> 3c 02 00 0f 85 2c 04 00 00 48 8b 9d 48 19 00 00 48 b8 00 00 00 RSP: 0000:ffff88810679fc88 EFLAGS: 00010202 RAX: dffffc0000000000 RBX: 0000000000000004 RCX: 0000000000000000 RDX: 0000000000000329 RSI: ffff88810679fd88 RDI: 0000000000001948 RBP: 0000000000000000 R08: 0000000000000000 R09: ffffed1020f639f0 R10: ffff888107b1cf87 R11: 0000000000000400 R12: 1ffff11020cf3f92 R13: ffff88810679fd88 R14: ffff888100b97b08 R15: ffff8881030bb080 FS: 0000000000000000(0000) GS:ffff888119e80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000005286001 CR4: 0000000000770ee0 DR0: ffffffff8fdd6cf4 DR1: ffffffff8fdd6cf5 DR2: ffffffff8fdd6cf6 DR3: ffffffff8fdd6cf7 DR6: 00000000fffe0ff0 DR7: 0000000000000600 PKRU: 55555554 Call Trace: <TASK> ? die_addr+0x3d/0xa0 ? exc_general_protection+0x144/0x220 ? asm_exc_general_protection+0x22/0x30 ? uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 ? sched_clock_cpu+0x69/0x550 ? uclogic_parse_ugee_v2_desc_gen_params+0x70/0x70 ? load_balance+0x2950/0x2950 ? rcu_trc_cmpxchg_need_qs+0x67/0xa0 hid_test_uclogic_params_cleanup_event_hooks+0x9e/0x1a0 ? uclogic_params_ugee_v2_init_event_hooks+0x600/0x600 ? __switch_to+0x5cf/0xe60 ? migrate_enable+0x260/0x260 ? __kthread_parkme+0x83/0x150 ? kunit_try_run_case_cleanup+0xe0/0xe0 kunit_generic_run_threadfn_adapter+0x4a/0x90 ? kunit_try_catch_throw+0x80/0x80 kthread+0x2b5/0x380 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork+0x2d/0x70 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork_asm+0x11/0x20 </TASK> Modules linked in: Dumping ftrace buffer: (ftrace buffer empty) ---[ end trace 0000000000000000 ]--- RIP: 0010:uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 Code: f3 f3 65 48 8b 14 25 28 00 00 00 48 89 54 24 60 31 d2 48 89 fa c7 44 24 30 00 00 00 00 48 c7 44 24 28 02 f8 02 01 48 c1 ea 03 <80> 3c 02 00 0f 85 2c 04 00 00 48 8b 9d 48 19 00 00 48 b8 00 00 00 RSP: 0000:ffff88810679fc88 EFLAGS: 00010202 RAX: dffffc0000000000 RBX: 0000000000000004 RCX: 0000000000000000 RDX: 0000000000000329 RSI: ffff88810679fd88 RDI: 0000000000001948 RBP: 0000000000000000 R08: 0000000000000000 R09: ffffed1020f639f0 R10: ffff888107b1cf87 R11: 0000000000000400 R12: 1ffff11020cf3f92 R13: ffff88810679fd88 R14: ffff888100b97b08 R15: ffff8881030bb080 FS: 0000000000000000(0000) GS:ffff888119e80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000005286001 CR4: 0000000000770ee0 DR0: ffffffff8fdd6cf4 DR1: ffffffff8fdd6cf5 DR2: ffffffff8fdd6cf6 DR3: ffffffff8fdd6cf7 DR6: 00000000fffe0ff0 DR7: 0000000000000600 PKRU: 55555554 Kernel panic - not syncing: Fatal exception Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: disabled Rebooting in 1 seconds.. Fixes: a251d6576d2a ("HID: uclogic: Handle wireless device reconnection") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- drivers/hid/hid-uclogic-params-test.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-uclogic-params-test.c b/drivers/hid/hid-uclogic-params-test.c index 678f50cbb160..3938bae25982 100644 --- a/drivers/hid/hid-uclogic-params-test.c +++ b/drivers/hid/hid-uclogic-params-test.c @@ -174,12 +174,22 @@ static void hid_test_uclogic_parse_ugee_v2_desc(struct kunit *test) KUNIT_EXPECT_EQ(test, params->frame_type, frame_type); } +struct fake_device { + unsigned long quirks; +}; + static void hid_test_uclogic_params_cleanup_event_hooks(struct kunit *test) { int res, n; + struct hid_device *hdev; + struct fake_device *fake_dev; struct uclogic_params p = {0, }; - res = uclogic_params_ugee_v2_init_event_hooks(NULL, &p); + hdev = kzalloc(sizeof(struct hid_device), GFP_KERNEL); + fake_dev = kzalloc(sizeof(struct fake_device), GFP_KERNEL); + hid_set_drvdata(hdev, fake_dev); + + res = uclogic_params_ugee_v2_init_event_hooks(hdev, &p); KUNIT_ASSERT_EQ(test, res, 0); /* Check that the function can be called repeatedly */ @@ -187,6 +197,9 @@ static void hid_test_uclogic_params_cleanup_event_hooks(struct kunit *test) uclogic_params_cleanup_event_hooks(&p); KUNIT_EXPECT_PTR_EQ(test, p.event_hooks, NULL); } + + kfree(fake_dev); + kfree(hdev); } static struct kunit_case hid_uclogic_params_test_cases[] = { -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] HID: uclogic: Fix user-memory-access bug in uclogic_params_ugee_v2_init_event_hooks() 2023-09-21 13:38 ` [PATCH 1/2] HID: uclogic: Fix user-memory-access bug in uclogic_params_ugee_v2_init_event_hooks() Jinjie Ruan @ 2023-10-08 17:21 ` José Expósito 2023-10-09 2:19 ` Jinjie Ruan 0 siblings, 1 reply; 6+ messages in thread From: José Expósito @ 2023-10-08 17:21 UTC (permalink / raw) To: Jinjie Ruan; +Cc: jikos, benjamin.tissoires, linux-input Hi Jinjie Ruan, Thanks a lot for finding and fixing this bug. On Thu, Sep 21, 2023 at 09:38:23PM +0800, Jinjie Ruan wrote: > When CONFIG_HID_UCLOGIC=y and CONFIG_KUNIT_ALL_TESTS=y, launch kernel and > then the below user-memory-access bug occurs. > > In hid_test_uclogic_params_cleanup_event_hooks(),it call > uclogic_params_ugee_v2_init_event_hooks() with the first arg=NULL, so > when it calls uclogic_params_ugee_v2_has_battery(), the hid_get_drvdata() > will access hdev->dev with hdev=NULL, which will cause below > user-memory-access. > > So add a fake_device with quirks member and call hid_set_drvdata() > to assign hdev->dev->driver_data which avoids the null-ptr-def bug > for drvdata->quirks in uclogic_params_ugee_v2_has_battery(). After applying > this patch, the below user-memory-access bug never occurs. > > general protection fault, probably for non-canonical address 0xdffffc0000000329: 0000 [#1] PREEMPT SMP KASAN > KASAN: probably user-memory-access in range [0x0000000000001948-0x000000000000194f] > CPU: 5 PID: 2189 Comm: kunit_try_catch Tainted: G B W N 6.6.0-rc2+ #30 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > RIP: 0010:uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 > Code: f3 f3 65 48 8b 14 25 28 00 00 00 48 89 54 24 60 31 d2 48 89 fa c7 44 24 30 00 00 00 00 48 c7 44 24 28 02 f8 02 01 48 c1 ea 03 <80> 3c 02 00 0f 85 2c 04 00 00 48 8b 9d 48 19 00 00 48 b8 00 00 00 > RSP: 0000:ffff88810679fc88 EFLAGS: 00010202 > RAX: dffffc0000000000 RBX: 0000000000000004 RCX: 0000000000000000 > RDX: 0000000000000329 RSI: ffff88810679fd88 RDI: 0000000000001948 > RBP: 0000000000000000 R08: 0000000000000000 R09: ffffed1020f639f0 > R10: ffff888107b1cf87 R11: 0000000000000400 R12: 1ffff11020cf3f92 > R13: ffff88810679fd88 R14: ffff888100b97b08 R15: ffff8881030bb080 > FS: 0000000000000000(0000) GS:ffff888119e80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 0000000005286001 CR4: 0000000000770ee0 > DR0: ffffffff8fdd6cf4 DR1: ffffffff8fdd6cf5 DR2: ffffffff8fdd6cf6 > DR3: ffffffff8fdd6cf7 DR6: 00000000fffe0ff0 DR7: 0000000000000600 > PKRU: 55555554 > Call Trace: > <TASK> > ? die_addr+0x3d/0xa0 > ? exc_general_protection+0x144/0x220 > ? asm_exc_general_protection+0x22/0x30 > ? uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 > ? sched_clock_cpu+0x69/0x550 > ? uclogic_parse_ugee_v2_desc_gen_params+0x70/0x70 > ? load_balance+0x2950/0x2950 > ? rcu_trc_cmpxchg_need_qs+0x67/0xa0 > hid_test_uclogic_params_cleanup_event_hooks+0x9e/0x1a0 > ? uclogic_params_ugee_v2_init_event_hooks+0x600/0x600 > ? __switch_to+0x5cf/0xe60 > ? migrate_enable+0x260/0x260 > ? __kthread_parkme+0x83/0x150 > ? kunit_try_run_case_cleanup+0xe0/0xe0 > kunit_generic_run_threadfn_adapter+0x4a/0x90 > ? kunit_try_catch_throw+0x80/0x80 > kthread+0x2b5/0x380 > ? kthread_complete_and_exit+0x20/0x20 > ret_from_fork+0x2d/0x70 > ? kthread_complete_and_exit+0x20/0x20 > ret_from_fork_asm+0x11/0x20 > </TASK> > Modules linked in: > Dumping ftrace buffer: > (ftrace buffer empty) > ---[ end trace 0000000000000000 ]--- > RIP: 0010:uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 > Code: f3 f3 65 48 8b 14 25 28 00 00 00 48 89 54 24 60 31 d2 48 89 fa c7 44 24 30 00 00 00 00 48 c7 44 24 28 02 f8 02 01 48 c1 ea 03 <80> 3c 02 00 0f 85 2c 04 00 00 48 8b 9d 48 19 00 00 48 b8 00 00 00 > RSP: 0000:ffff88810679fc88 EFLAGS: 00010202 > RAX: dffffc0000000000 RBX: 0000000000000004 RCX: 0000000000000000 > RDX: 0000000000000329 RSI: ffff88810679fd88 RDI: 0000000000001948 > RBP: 0000000000000000 R08: 0000000000000000 R09: ffffed1020f639f0 > R10: ffff888107b1cf87 R11: 0000000000000400 R12: 1ffff11020cf3f92 > R13: ffff88810679fd88 R14: ffff888100b97b08 R15: ffff8881030bb080 > FS: 0000000000000000(0000) GS:ffff888119e80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 0000000005286001 CR4: 0000000000770ee0 > DR0: ffffffff8fdd6cf4 DR1: ffffffff8fdd6cf5 DR2: ffffffff8fdd6cf6 > DR3: ffffffff8fdd6cf7 DR6: 00000000fffe0ff0 DR7: 0000000000000600 > PKRU: 55555554 > Kernel panic - not syncing: Fatal exception > Dumping ftrace buffer: > (ftrace buffer empty) > Kernel Offset: disabled > Rebooting in 1 seconds.. > > Fixes: a251d6576d2a ("HID: uclogic: Handle wireless device reconnection") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > drivers/hid/hid-uclogic-params-test.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-uclogic-params-test.c b/drivers/hid/hid-uclogic-params-test.c > index 678f50cbb160..3938bae25982 100644 > --- a/drivers/hid/hid-uclogic-params-test.c > +++ b/drivers/hid/hid-uclogic-params-test.c > @@ -174,12 +174,22 @@ static void hid_test_uclogic_parse_ugee_v2_desc(struct kunit *test) > KUNIT_EXPECT_EQ(test, params->frame_type, frame_type); > } > > +struct fake_device { > + unsigned long quirks; > +}; > + > static void hid_test_uclogic_params_cleanup_event_hooks(struct kunit *test) > { > int res, n; > + struct hid_device *hdev; > + struct fake_device *fake_dev; > struct uclogic_params p = {0, }; > > - res = uclogic_params_ugee_v2_init_event_hooks(NULL, &p); > + hdev = kzalloc(sizeof(struct hid_device), GFP_KERNEL); > + fake_dev = kzalloc(sizeof(struct fake_device), GFP_KERNEL); Intead of using `kzalloc()` to allocate memory for `hdev` and `fake_dev` we should use `kunit_kzalloc()`. It has 2 main advatages: - If an assertion fails, the memory is freed - No need for `kfree()` > + hid_set_drvdata(hdev, fake_dev); > + > + res = uclogic_params_ugee_v2_init_event_hooks(hdev, &p); > KUNIT_ASSERT_EQ(test, res, 0); > > /* Check that the function can be called repeatedly */ > @@ -187,6 +197,9 @@ static void hid_test_uclogic_params_cleanup_event_hooks(struct kunit *test) > uclogic_params_cleanup_event_hooks(&p); > KUNIT_EXPECT_PTR_EQ(test, p.event_hooks, NULL); > } > + > + kfree(fake_dev); > + kfree(hdev); This 2 lines can be removed if `kunit_kzalloc()` is used. > } > > static struct kunit_case hid_uclogic_params_test_cases[] = { > -- > 2.34.1 > Once the `kunit_kzalloc()` change is appliyed: Reviewed-by: José Expósito <jose.exposito89@gmail.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] HID: uclogic: Fix user-memory-access bug in uclogic_params_ugee_v2_init_event_hooks() 2023-10-08 17:21 ` José Expósito @ 2023-10-09 2:19 ` Jinjie Ruan 0 siblings, 0 replies; 6+ messages in thread From: Jinjie Ruan @ 2023-10-09 2:19 UTC (permalink / raw) To: José Expósito; +Cc: jikos, benjamin.tissoires, linux-input On 2023/10/9 1:21, José Expósito wrote: > Hi Jinjie Ruan, > > Thanks a lot for finding and fixing this bug. > > On Thu, Sep 21, 2023 at 09:38:23PM +0800, Jinjie Ruan wrote: >> When CONFIG_HID_UCLOGIC=y and CONFIG_KUNIT_ALL_TESTS=y, launch kernel and >> then the below user-memory-access bug occurs. >> >> In hid_test_uclogic_params_cleanup_event_hooks(),it call >> uclogic_params_ugee_v2_init_event_hooks() with the first arg=NULL, so >> when it calls uclogic_params_ugee_v2_has_battery(), the hid_get_drvdata() >> will access hdev->dev with hdev=NULL, which will cause below >> user-memory-access. >> >> So add a fake_device with quirks member and call hid_set_drvdata() >> to assign hdev->dev->driver_data which avoids the null-ptr-def bug >> for drvdata->quirks in uclogic_params_ugee_v2_has_battery(). After applying >> this patch, the below user-memory-access bug never occurs. >> >> general protection fault, probably for non-canonical address 0xdffffc0000000329: 0000 [#1] PREEMPT SMP KASAN >> KASAN: probably user-memory-access in range [0x0000000000001948-0x000000000000194f] >> CPU: 5 PID: 2189 Comm: kunit_try_catch Tainted: G B W N 6.6.0-rc2+ #30 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 >> RIP: 0010:uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 >> Code: f3 f3 65 48 8b 14 25 28 00 00 00 48 89 54 24 60 31 d2 48 89 fa c7 44 24 30 00 00 00 00 48 c7 44 24 28 02 f8 02 01 48 c1 ea 03 <80> 3c 02 00 0f 85 2c 04 00 00 48 8b 9d 48 19 00 00 48 b8 00 00 00 >> RSP: 0000:ffff88810679fc88 EFLAGS: 00010202 >> RAX: dffffc0000000000 RBX: 0000000000000004 RCX: 0000000000000000 >> RDX: 0000000000000329 RSI: ffff88810679fd88 RDI: 0000000000001948 >> RBP: 0000000000000000 R08: 0000000000000000 R09: ffffed1020f639f0 >> R10: ffff888107b1cf87 R11: 0000000000000400 R12: 1ffff11020cf3f92 >> R13: ffff88810679fd88 R14: ffff888100b97b08 R15: ffff8881030bb080 >> FS: 0000000000000000(0000) GS:ffff888119e80000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 0000000000000000 CR3: 0000000005286001 CR4: 0000000000770ee0 >> DR0: ffffffff8fdd6cf4 DR1: ffffffff8fdd6cf5 DR2: ffffffff8fdd6cf6 >> DR3: ffffffff8fdd6cf7 DR6: 00000000fffe0ff0 DR7: 0000000000000600 >> PKRU: 55555554 >> Call Trace: >> <TASK> >> ? die_addr+0x3d/0xa0 >> ? exc_general_protection+0x144/0x220 >> ? asm_exc_general_protection+0x22/0x30 >> ? uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 >> ? sched_clock_cpu+0x69/0x550 >> ? uclogic_parse_ugee_v2_desc_gen_params+0x70/0x70 >> ? load_balance+0x2950/0x2950 >> ? rcu_trc_cmpxchg_need_qs+0x67/0xa0 >> hid_test_uclogic_params_cleanup_event_hooks+0x9e/0x1a0 >> ? uclogic_params_ugee_v2_init_event_hooks+0x600/0x600 >> ? __switch_to+0x5cf/0xe60 >> ? migrate_enable+0x260/0x260 >> ? __kthread_parkme+0x83/0x150 >> ? kunit_try_run_case_cleanup+0xe0/0xe0 >> kunit_generic_run_threadfn_adapter+0x4a/0x90 >> ? kunit_try_catch_throw+0x80/0x80 >> kthread+0x2b5/0x380 >> ? kthread_complete_and_exit+0x20/0x20 >> ret_from_fork+0x2d/0x70 >> ? kthread_complete_and_exit+0x20/0x20 >> ret_from_fork_asm+0x11/0x20 >> </TASK> >> Modules linked in: >> Dumping ftrace buffer: >> (ftrace buffer empty) >> ---[ end trace 0000000000000000 ]--- >> RIP: 0010:uclogic_params_ugee_v2_init_event_hooks+0x87/0x600 >> Code: f3 f3 65 48 8b 14 25 28 00 00 00 48 89 54 24 60 31 d2 48 89 fa c7 44 24 30 00 00 00 00 48 c7 44 24 28 02 f8 02 01 48 c1 ea 03 <80> 3c 02 00 0f 85 2c 04 00 00 48 8b 9d 48 19 00 00 48 b8 00 00 00 >> RSP: 0000:ffff88810679fc88 EFLAGS: 00010202 >> RAX: dffffc0000000000 RBX: 0000000000000004 RCX: 0000000000000000 >> RDX: 0000000000000329 RSI: ffff88810679fd88 RDI: 0000000000001948 >> RBP: 0000000000000000 R08: 0000000000000000 R09: ffffed1020f639f0 >> R10: ffff888107b1cf87 R11: 0000000000000400 R12: 1ffff11020cf3f92 >> R13: ffff88810679fd88 R14: ffff888100b97b08 R15: ffff8881030bb080 >> FS: 0000000000000000(0000) GS:ffff888119e80000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 0000000000000000 CR3: 0000000005286001 CR4: 0000000000770ee0 >> DR0: ffffffff8fdd6cf4 DR1: ffffffff8fdd6cf5 DR2: ffffffff8fdd6cf6 >> DR3: ffffffff8fdd6cf7 DR6: 00000000fffe0ff0 DR7: 0000000000000600 >> PKRU: 55555554 >> Kernel panic - not syncing: Fatal exception >> Dumping ftrace buffer: >> (ftrace buffer empty) >> Kernel Offset: disabled >> Rebooting in 1 seconds.. >> >> Fixes: a251d6576d2a ("HID: uclogic: Handle wireless device reconnection") >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >> --- >> drivers/hid/hid-uclogic-params-test.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/hid/hid-uclogic-params-test.c b/drivers/hid/hid-uclogic-params-test.c >> index 678f50cbb160..3938bae25982 100644 >> --- a/drivers/hid/hid-uclogic-params-test.c >> +++ b/drivers/hid/hid-uclogic-params-test.c >> @@ -174,12 +174,22 @@ static void hid_test_uclogic_parse_ugee_v2_desc(struct kunit *test) >> KUNIT_EXPECT_EQ(test, params->frame_type, frame_type); >> } >> >> +struct fake_device { >> + unsigned long quirks; >> +}; >> + >> static void hid_test_uclogic_params_cleanup_event_hooks(struct kunit *test) >> { >> int res, n; >> + struct hid_device *hdev; >> + struct fake_device *fake_dev; >> struct uclogic_params p = {0, }; >> >> - res = uclogic_params_ugee_v2_init_event_hooks(NULL, &p); >> + hdev = kzalloc(sizeof(struct hid_device), GFP_KERNEL); >> + fake_dev = kzalloc(sizeof(struct fake_device), GFP_KERNEL); > > Intead of using `kzalloc()` to allocate memory for `hdev` and `fake_dev` > we should use `kunit_kzalloc()`. > > It has 2 main advatages: > - If an assertion fails, the memory is freed > - No need for `kfree()` Thank you! I'll fix it sooner. > >> + hid_set_drvdata(hdev, fake_dev); >> + >> + res = uclogic_params_ugee_v2_init_event_hooks(hdev, &p); >> KUNIT_ASSERT_EQ(test, res, 0); >> >> /* Check that the function can be called repeatedly */ >> @@ -187,6 +197,9 @@ static void hid_test_uclogic_params_cleanup_event_hooks(struct kunit *test) >> uclogic_params_cleanup_event_hooks(&p); >> KUNIT_EXPECT_PTR_EQ(test, p.event_hooks, NULL); >> } >> + >> + kfree(fake_dev); >> + kfree(hdev); > > This 2 lines can be removed if `kunit_kzalloc()` is used. > >> } >> >> static struct kunit_case hid_uclogic_params_test_cases[] = { >> -- >> 2.34.1 >> > > Once the `kunit_kzalloc()` change is appliyed: > Reviewed-by: José Expósito <jose.exposito89@gmail.com> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] HID: uclogic: Fix a work->entry not empty bug in __queue_work() 2023-09-21 13:38 [PATCH 0/2] HID: uclogic: Fix two bugs in uclogic Jinjie Ruan 2023-09-21 13:38 ` [PATCH 1/2] HID: uclogic: Fix user-memory-access bug in uclogic_params_ugee_v2_init_event_hooks() Jinjie Ruan @ 2023-09-21 13:38 ` Jinjie Ruan 2023-10-08 17:21 ` José Expósito 1 sibling, 1 reply; 6+ messages in thread From: Jinjie Ruan @ 2023-09-21 13:38 UTC (permalink / raw) To: jikos, benjamin.tissoires, jose.exposito89, linux-input; +Cc: ruanjinjie When CONFIG_HID_UCLOGIC=y and CONFIG_KUNIT_ALL_TESTS=y, launch kernel and then the below work->entry not empty bug occurs. In hid_test_uclogic_exec_event_hook_test(), the filter->work is not initialized to be added to p.event_hooks->list, and then the schedule_work() in uclogic_exec_event_hook() will call __queue_work(), which check whether the work->entry is empty and cause the below warning call trace. So call INIT_WORK() with a fake work to solve the issue. After applying this patch, the below work->entry not empty bug never occurs. WARNING: CPU: 0 PID: 2177 at kernel/workqueue.c:1787 __queue_work.part.0+0x780/0xad0 Modules linked in: CPU: 0 PID: 2177 Comm: kunit_try_catch Tainted: G B W N 6.6.0-rc2+ #30 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 RIP: 0010:__queue_work.part.0+0x780/0xad0 Code: 44 24 20 0f b6 00 84 c0 74 08 3c 03 0f 8e 52 03 00 00 f6 83 00 01 00 00 02 74 6f 4c 89 ef e8 c7 d8 f1 02 f3 90 e9 e5 f8 ff ff <0f> 0b e9 63 fc ff ff 89 e9 49 8d 57 68 4c 89 e6 4c 89 ff 83 c9 02 RSP: 0000:ffff888102bb7ce8 EFLAGS: 00010086 RAX: 0000000000000000 RBX: ffff888106b8e460 RCX: ffffffff84141cc7 RDX: 1ffff11020d71c8c RSI: 0000000000000004 RDI: ffff8881001d0118 RBP: dffffc0000000000 R08: 0000000000000001 R09: ffffed1020576f92 R10: 0000000000000003 R11: ffff888102bb7980 R12: ffff888106b8e458 R13: ffff888119c38800 R14: 0000000000000000 R15: ffff8881001d0100 FS: 0000000000000000(0000) GS:ffff888119c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff888119506000 CR3: 0000000005286001 CR4: 0000000000770ef0 DR0: ffffffff8fdd6ce0 DR1: ffffffff8fdd6ce1 DR2: ffffffff8fdd6ce3 DR3: ffffffff8fdd6ce5 DR6: 00000000fffe0ff0 DR7: 0000000000000600 PKRU: 55555554 Call Trace: <TASK> ? __warn+0xc9/0x260 ? __queue_work.part.0+0x780/0xad0 ? report_bug+0x345/0x400 ? handle_bug+0x3c/0x70 ? exc_invalid_op+0x14/0x40 ? asm_exc_invalid_op+0x16/0x20 ? _raw_spin_lock+0x87/0xe0 ? __queue_work.part.0+0x780/0xad0 ? __queue_work.part.0+0x249/0xad0 queue_work_on+0x48/0x50 uclogic_exec_event_hook.isra.0+0xf7/0x160 hid_test_uclogic_exec_event_hook_test+0x2f1/0x5d0 ? try_to_wake_up+0x151/0x13e0 ? uclogic_exec_event_hook.isra.0+0x160/0x160 ? _raw_spin_lock_irqsave+0x8d/0xe0 ? __sched_text_end+0xa/0xa ? __sched_text_end+0xa/0xa ? migrate_enable+0x260/0x260 ? kunit_try_run_case_cleanup+0xe0/0xe0 kunit_generic_run_threadfn_adapter+0x4a/0x90 ? kunit_try_catch_throw+0x80/0x80 kthread+0x2b5/0x380 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork+0x2d/0x70 ? kthread_complete_and_exit+0x20/0x20 ret_from_fork_asm+0x11/0x20 </TASK> Fixes: a251d6576d2a ("HID: uclogic: Handle wireless device reconnection") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- drivers/hid/hid-uclogic-core-test.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/hid/hid-uclogic-core-test.c b/drivers/hid/hid-uclogic-core-test.c index 2bb916226a38..cb274cde3ad2 100644 --- a/drivers/hid/hid-uclogic-core-test.c +++ b/drivers/hid/hid-uclogic-core-test.c @@ -56,6 +56,11 @@ static struct uclogic_raw_event_hook_test test_events[] = { }, }; +static void fake_work(struct work_struct *work) +{ + +} + static void hid_test_uclogic_exec_event_hook_test(struct kunit *test) { struct uclogic_params p = {0, }; @@ -77,6 +82,8 @@ static void hid_test_uclogic_exec_event_hook_test(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filter->event); memcpy(filter->event, &hook_events[n].event[0], filter->size); + INIT_WORK(&filter->work, fake_work); + list_add_tail(&filter->list, &p.event_hooks->list); } -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] HID: uclogic: Fix a work->entry not empty bug in __queue_work() 2023-09-21 13:38 ` [PATCH 2/2] HID: uclogic: Fix a work->entry not empty bug in __queue_work() Jinjie Ruan @ 2023-10-08 17:21 ` José Expósito 0 siblings, 0 replies; 6+ messages in thread From: José Expósito @ 2023-10-08 17:21 UTC (permalink / raw) To: Jinjie Ruan; +Cc: jikos, benjamin.tissoires, linux-input On Thu, Sep 21, 2023 at 09:38:24PM +0800, Jinjie Ruan wrote: > When CONFIG_HID_UCLOGIC=y and CONFIG_KUNIT_ALL_TESTS=y, launch > kernel and then the below work->entry not empty bug occurs. > > In hid_test_uclogic_exec_event_hook_test(), the filter->work is not > initialized to be added to p.event_hooks->list, and then the > schedule_work() in uclogic_exec_event_hook() will call __queue_work(), > which check whether the work->entry is empty and cause the below > warning call trace. > > So call INIT_WORK() with a fake work to solve the issue. After applying > this patch, the below work->entry not empty bug never occurs. > > WARNING: CPU: 0 PID: 2177 at kernel/workqueue.c:1787 __queue_work.part.0+0x780/0xad0 > Modules linked in: > CPU: 0 PID: 2177 Comm: kunit_try_catch Tainted: G B W N 6.6.0-rc2+ #30 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > RIP: 0010:__queue_work.part.0+0x780/0xad0 > Code: 44 24 20 0f b6 00 84 c0 74 08 3c 03 0f 8e 52 03 00 00 f6 83 00 01 00 00 02 74 6f 4c 89 ef e8 c7 d8 f1 02 f3 90 e9 e5 f8 ff ff <0f> 0b e9 63 fc ff ff 89 e9 49 8d 57 68 4c 89 e6 4c 89 ff 83 c9 02 > RSP: 0000:ffff888102bb7ce8 EFLAGS: 00010086 > RAX: 0000000000000000 RBX: ffff888106b8e460 RCX: ffffffff84141cc7 > RDX: 1ffff11020d71c8c RSI: 0000000000000004 RDI: ffff8881001d0118 > RBP: dffffc0000000000 R08: 0000000000000001 R09: ffffed1020576f92 > R10: 0000000000000003 R11: ffff888102bb7980 R12: ffff888106b8e458 > R13: ffff888119c38800 R14: 0000000000000000 R15: ffff8881001d0100 > FS: 0000000000000000(0000) GS:ffff888119c00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ffff888119506000 CR3: 0000000005286001 CR4: 0000000000770ef0 > DR0: ffffffff8fdd6ce0 DR1: ffffffff8fdd6ce1 DR2: ffffffff8fdd6ce3 > DR3: ffffffff8fdd6ce5 DR6: 00000000fffe0ff0 DR7: 0000000000000600 > PKRU: 55555554 > Call Trace: > <TASK> > ? __warn+0xc9/0x260 > ? __queue_work.part.0+0x780/0xad0 > ? report_bug+0x345/0x400 > ? handle_bug+0x3c/0x70 > ? exc_invalid_op+0x14/0x40 > ? asm_exc_invalid_op+0x16/0x20 > ? _raw_spin_lock+0x87/0xe0 > ? __queue_work.part.0+0x780/0xad0 > ? __queue_work.part.0+0x249/0xad0 > queue_work_on+0x48/0x50 > uclogic_exec_event_hook.isra.0+0xf7/0x160 > hid_test_uclogic_exec_event_hook_test+0x2f1/0x5d0 > ? try_to_wake_up+0x151/0x13e0 > ? uclogic_exec_event_hook.isra.0+0x160/0x160 > ? _raw_spin_lock_irqsave+0x8d/0xe0 > ? __sched_text_end+0xa/0xa > ? __sched_text_end+0xa/0xa > ? migrate_enable+0x260/0x260 > ? kunit_try_run_case_cleanup+0xe0/0xe0 > kunit_generic_run_threadfn_adapter+0x4a/0x90 > ? kunit_try_catch_throw+0x80/0x80 > kthread+0x2b5/0x380 > ? kthread_complete_and_exit+0x20/0x20 > ret_from_fork+0x2d/0x70 > ? kthread_complete_and_exit+0x20/0x20 > ret_from_fork_asm+0x11/0x20 > </TASK> > > Fixes: a251d6576d2a ("HID: uclogic: Handle wireless device reconnection") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Reviewed-by: José Expósito <jose.exposito89@gmail.com> > --- > drivers/hid/hid-uclogic-core-test.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/hid/hid-uclogic-core-test.c b/drivers/hid/hid-uclogic-core-test.c > index 2bb916226a38..cb274cde3ad2 100644 > --- a/drivers/hid/hid-uclogic-core-test.c > +++ b/drivers/hid/hid-uclogic-core-test.c > @@ -56,6 +56,11 @@ static struct uclogic_raw_event_hook_test test_events[] = { > }, > }; > > +static void fake_work(struct work_struct *work) > +{ > + > +} > + > static void hid_test_uclogic_exec_event_hook_test(struct kunit *test) > { > struct uclogic_params p = {0, }; > @@ -77,6 +82,8 @@ static void hid_test_uclogic_exec_event_hook_test(struct kunit *test) > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filter->event); > memcpy(filter->event, &hook_events[n].event[0], filter->size); > > + INIT_WORK(&filter->work, fake_work); > + > list_add_tail(&filter->list, &p.event_hooks->list); > } > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-09 2:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-21 13:38 [PATCH 0/2] HID: uclogic: Fix two bugs in uclogic Jinjie Ruan 2023-09-21 13:38 ` [PATCH 1/2] HID: uclogic: Fix user-memory-access bug in uclogic_params_ugee_v2_init_event_hooks() Jinjie Ruan 2023-10-08 17:21 ` José Expósito 2023-10-09 2:19 ` Jinjie Ruan 2023-09-21 13:38 ` [PATCH 2/2] HID: uclogic: Fix a work->entry not empty bug in __queue_work() Jinjie Ruan 2023-10-08 17:21 ` José Expósito
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).