| Message ID | 20230729010003.4058582-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5a175d369c702ce08c9feb630125c9fc7a9e1370 |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | [-next,v2] kunit: fix wild-memory-access bug in kunit_filter_suites() | expand |
On Sat, 29 Jul 2023 at 09:00, Ruan Jinjie <ruanjinjie@huawei.com> wrote: > > As for kunit_filter_suites(), When the filters arg = NULL, such as > the call of kunit_filter_suites(&suite_set, "suite2", NULL, NULL, &err) > in filter_suites_test() tese case in kunit, both filter_count and > parsed_filters will not be initialized. > > So it's possible to enter kunit_filter_attr_tests(), and the use of > uninitialized parsed_filters will cause below wild-memory-access. > > RIP: 0010:kunit_filter_suites+0x780/0xa40 > Code: fe ff ff e8 42 87 4d ff 41 83 c6 01 49 83 c5 10 49 89 dc 44 39 74 24 50 0f 8e 81 fe ff ff e8 27 87 4d ff 4c 89 e8 48 c1 e8 03 <66> 42 83 3c 38 00 0f 85 af 01 00 00 49 8b 75 00 49 8b 55 08 4c 89 > RSP: 0000:ff1100010743fc38 EFLAGS: 00010203 > RAX: 03fc4400041d0ff1 RBX: ff1100010389a900 RCX: ffffffff9f940ad9 > RDX: ff11000107429740 RSI: 0000000000000000 RDI: ff110001037ec920 > RBP: ff1100010743fd50 R08: 0000000000000000 R09: ffe21c0020e87f1e > R10: 0000000000000003 R11: 0000000000032001 R12: ff110001037ec800 > R13: 1fe2200020e87f8c R14: 0000000000000000 R15: dffffc0000000000 > FS: 0000000000000000(0000) GS:ff1100011b000000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ff11000115201000 CR3: 0000000113066001 CR4: 0000000000771ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > PKRU: 55555554 > Call Trace: > <TASK> > ? die_addr+0x3c/0xa0 > ? exc_general_protection+0x148/0x220 > ? asm_exc_general_protection+0x26/0x30 > ? kunit_filter_suites+0x779/0xa40 > ? kunit_filter_suites+0x780/0xa40 > ? kunit_filter_suites+0x779/0xa40 > ? __pfx_kunit_filter_suites+0x10/0x10 > ? __pfx_kfree+0x10/0x10 > ? kunit_add_action_or_reset+0x3d/0x50 > filter_suites_test+0x1b7/0x440 > ? __pfx_filter_suites_test+0x10/0x10 > ? __pfx___schedule+0x10/0x10 > ? try_to_wake_up+0xa8e/0x1210 > ? _raw_spin_lock_irqsave+0x86/0xe0 > ? __pfx__raw_spin_lock_irqsave+0x10/0x10 > ? set_cpus_allowed_ptr+0x7c/0xb0 > kunit_try_run_case+0x119/0x270 > ? __kthread_parkme+0xdc/0x160 > ? __pfx_kunit_try_run_case+0x10/0x10 > kunit_generic_run_threadfn_adapter+0x4e/0xa0 > ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10 > kthread+0x2c7/0x3c0 > ? __pfx_kthread+0x10/0x10 > ret_from_fork+0x2c/0x70 > ? __pfx_kthread+0x10/0x10 > ret_from_fork_asm+0x1b/0x30 > </TASK> > Modules linked in: > Dumping ftrace buffer: > (ftrace buffer empty) > ---[ end trace 0000000000000000 ]--- > RIP: 0010:kunit_filter_suites+0x780/0xa40 > Code: fe ff ff e8 42 87 4d ff 41 83 c6 01 49 83 c5 10 49 89 dc 44 39 74 24 50 0f 8e 81 fe ff ff e8 27 87 4d ff 4c 89 e8 48 c1 e8 03 <66> 42 83 3c 38 00 0f 85 af 01 00 00 49 8b 75 00 49 8b 55 08 4c 89 > RSP: 0000:ff1100010743fc38 EFLAGS: 00010203 > RAX: 03fc4400041d0ff1 RBX: ff1100010389a900 RCX: ffffffff9f940ad9 > RDX: ff11000107429740 RSI: 0000000000000000 RDI: ff110001037ec920 > RBP: ff1100010743fd50 R08: 0000000000000000 R09: ffe21c0020e87f1e > R10: 0000000000000003 R11: 0000000000032001 R12: ff110001037ec800 > R13: 1fe2200020e87f8c R14: 0000000000000000 R15: dffffc0000000000 > FS: 0000000000000000(0000) GS:ff1100011b000000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ff11000115201000 CR3: 0000000113066001 CR4: 0000000000771ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > PKRU: 55555554 > Kernel panic - not syncing: Fatal exception > Dumping ftrace buffer: > (ftrace buffer empty) > Kernel Offset: 0x1da00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff) > Rebooting in 1 seconds.. > > Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > --- > v2: > - add fixes tag > --- Thanks very much. For some reason, I'm not getting the same crash here (even under KASAN), but this is definitely a real issue. I know Rae has some smatch warning fixes she was looking at, too, which may or may not be related. Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > lib/kunit/executor.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c > index 483f7b7873a7..5b5bed1efb93 100644 > --- a/lib/kunit/executor.c > +++ b/lib/kunit/executor.c > @@ -125,7 +125,8 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, > char *filter_action, > int *err) > { > - int i, j, k, filter_count; > + int i, j, k; > + int filter_count = 0; > struct kunit_suite **copy, *filtered_suite, *new_filtered_suite; > struct suite_set filtered; > struct kunit_glob_filter parsed_glob; > -- > 2.34.1 >
On Sat, Jul 29, 2023 at 09:00:03AM +0800, Ruan Jinjie wrote: > As for kunit_filter_suites(), When the filters arg = NULL, such as > the call of kunit_filter_suites(&suite_set, "suite2", NULL, NULL, &err) > in filter_suites_test() tese case in kunit, both filter_count and > parsed_filters will not be initialized. > > So it's possible to enter kunit_filter_attr_tests(), and the use of > uninitialized parsed_filters will cause below wild-memory-access. > > RIP: 0010:kunit_filter_suites+0x780/0xa40 > Code: fe ff ff e8 42 87 4d ff 41 83 c6 01 49 83 c5 10 49 89 dc 44 39 74 24 50 0f 8e 81 fe ff ff e8 27 87 4d ff 4c 89 e8 48 c1 e8 03 <66> 42 83 3c 38 00 0f 85 af 01 00 00 49 8b 75 00 49 8b 55 08 4c 89 > RSP: 0000:ff1100010743fc38 EFLAGS: 00010203 > RAX: 03fc4400041d0ff1 RBX: ff1100010389a900 RCX: ffffffff9f940ad9 > RDX: ff11000107429740 RSI: 0000000000000000 RDI: ff110001037ec920 > RBP: ff1100010743fd50 R08: 0000000000000000 R09: ffe21c0020e87f1e > R10: 0000000000000003 R11: 0000000000032001 R12: ff110001037ec800 > R13: 1fe2200020e87f8c R14: 0000000000000000 R15: dffffc0000000000 > FS: 0000000000000000(0000) GS:ff1100011b000000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ff11000115201000 CR3: 0000000113066001 CR4: 0000000000771ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > PKRU: 55555554 > Call Trace: > <TASK> > ? die_addr+0x3c/0xa0 > ? exc_general_protection+0x148/0x220 > ? asm_exc_general_protection+0x26/0x30 > ? kunit_filter_suites+0x779/0xa40 > ? kunit_filter_suites+0x780/0xa40 > ? kunit_filter_suites+0x779/0xa40 > ? __pfx_kunit_filter_suites+0x10/0x10 > ? __pfx_kfree+0x10/0x10 > ? kunit_add_action_or_reset+0x3d/0x50 > filter_suites_test+0x1b7/0x440 > ? __pfx_filter_suites_test+0x10/0x10 > ? __pfx___schedule+0x10/0x10 > ? try_to_wake_up+0xa8e/0x1210 > ? _raw_spin_lock_irqsave+0x86/0xe0 > ? __pfx__raw_spin_lock_irqsave+0x10/0x10 > ? set_cpus_allowed_ptr+0x7c/0xb0 > kunit_try_run_case+0x119/0x270 > ? __kthread_parkme+0xdc/0x160 > ? __pfx_kunit_try_run_case+0x10/0x10 > kunit_generic_run_threadfn_adapter+0x4e/0xa0 > ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10 > kthread+0x2c7/0x3c0 > ? __pfx_kthread+0x10/0x10 > ret_from_fork+0x2c/0x70 > ? __pfx_kthread+0x10/0x10 > ret_from_fork_asm+0x1b/0x30 > </TASK> > Modules linked in: > Dumping ftrace buffer: > (ftrace buffer empty) > ---[ end trace 0000000000000000 ]--- > RIP: 0010:kunit_filter_suites+0x780/0xa40 > Code: fe ff ff e8 42 87 4d ff 41 83 c6 01 49 83 c5 10 49 89 dc 44 39 74 24 50 0f 8e 81 fe ff ff e8 27 87 4d ff 4c 89 e8 48 c1 e8 03 <66> 42 83 3c 38 00 0f 85 af 01 00 00 49 8b 75 00 49 8b 55 08 4c 89 > RSP: 0000:ff1100010743fc38 EFLAGS: 00010203 > RAX: 03fc4400041d0ff1 RBX: ff1100010389a900 RCX: ffffffff9f940ad9 > RDX: ff11000107429740 RSI: 0000000000000000 RDI: ff110001037ec920 > RBP: ff1100010743fd50 R08: 0000000000000000 R09: ffe21c0020e87f1e > R10: 0000000000000003 R11: 0000000000032001 R12: ff110001037ec800 > R13: 1fe2200020e87f8c R14: 0000000000000000 R15: dffffc0000000000 > FS: 0000000000000000(0000) GS:ff1100011b000000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: ff11000115201000 CR3: 0000000113066001 CR4: 0000000000771ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > PKRU: 55555554 > Kernel panic - not syncing: Fatal exception > Dumping ftrace buffer: > (ftrace buffer empty) > Kernel Offset: 0x1da00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff) > Rebooting in 1 seconds.. > > Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> Tested-by: Guenter Roeck <linux@roeck-us.net> Guenter > --- > v2: > - add fixes tag > --- > lib/kunit/executor.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c > index 483f7b7873a7..5b5bed1efb93 100644 > --- a/lib/kunit/executor.c > +++ b/lib/kunit/executor.c > @@ -125,7 +125,8 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, > char *filter_action, > int *err) > { > - int i, j, k, filter_count; > + int i, j, k; > + int filter_count = 0; > struct kunit_suite **copy, *filtered_suite, *new_filtered_suite; > struct suite_set filtered; > struct kunit_glob_filter parsed_glob; > -- > 2.34.1 >
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 483f7b7873a7..5b5bed1efb93 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -125,7 +125,8 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, char *filter_action, int *err) { - int i, j, k, filter_count; + int i, j, k; + int filter_count = 0; struct kunit_suite **copy, *filtered_suite, *new_filtered_suite; struct suite_set filtered; struct kunit_glob_filter parsed_glob;
As for kunit_filter_suites(), When the filters arg = NULL, such as the call of kunit_filter_suites(&suite_set, "suite2", NULL, NULL, &err) in filter_suites_test() tese case in kunit, both filter_count and parsed_filters will not be initialized. So it's possible to enter kunit_filter_attr_tests(), and the use of uninitialized parsed_filters will cause below wild-memory-access. RIP: 0010:kunit_filter_suites+0x780/0xa40 Code: fe ff ff e8 42 87 4d ff 41 83 c6 01 49 83 c5 10 49 89 dc 44 39 74 24 50 0f 8e 81 fe ff ff e8 27 87 4d ff 4c 89 e8 48 c1 e8 03 <66> 42 83 3c 38 00 0f 85 af 01 00 00 49 8b 75 00 49 8b 55 08 4c 89 RSP: 0000:ff1100010743fc38 EFLAGS: 00010203 RAX: 03fc4400041d0ff1 RBX: ff1100010389a900 RCX: ffffffff9f940ad9 RDX: ff11000107429740 RSI: 0000000000000000 RDI: ff110001037ec920 RBP: ff1100010743fd50 R08: 0000000000000000 R09: ffe21c0020e87f1e R10: 0000000000000003 R11: 0000000000032001 R12: ff110001037ec800 R13: 1fe2200020e87f8c R14: 0000000000000000 R15: dffffc0000000000 FS: 0000000000000000(0000) GS:ff1100011b000000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ff11000115201000 CR3: 0000000113066001 CR4: 0000000000771ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: <TASK> ? die_addr+0x3c/0xa0 ? exc_general_protection+0x148/0x220 ? asm_exc_general_protection+0x26/0x30 ? kunit_filter_suites+0x779/0xa40 ? kunit_filter_suites+0x780/0xa40 ? kunit_filter_suites+0x779/0xa40 ? __pfx_kunit_filter_suites+0x10/0x10 ? __pfx_kfree+0x10/0x10 ? kunit_add_action_or_reset+0x3d/0x50 filter_suites_test+0x1b7/0x440 ? __pfx_filter_suites_test+0x10/0x10 ? __pfx___schedule+0x10/0x10 ? try_to_wake_up+0xa8e/0x1210 ? _raw_spin_lock_irqsave+0x86/0xe0 ? __pfx__raw_spin_lock_irqsave+0x10/0x10 ? set_cpus_allowed_ptr+0x7c/0xb0 kunit_try_run_case+0x119/0x270 ? __kthread_parkme+0xdc/0x160 ? __pfx_kunit_try_run_case+0x10/0x10 kunit_generic_run_threadfn_adapter+0x4e/0xa0 ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10 kthread+0x2c7/0x3c0 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x2c/0x70 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1b/0x30 </TASK> Modules linked in: Dumping ftrace buffer: (ftrace buffer empty) ---[ end trace 0000000000000000 ]--- RIP: 0010:kunit_filter_suites+0x780/0xa40 Code: fe ff ff e8 42 87 4d ff 41 83 c6 01 49 83 c5 10 49 89 dc 44 39 74 24 50 0f 8e 81 fe ff ff e8 27 87 4d ff 4c 89 e8 48 c1 e8 03 <66> 42 83 3c 38 00 0f 85 af 01 00 00 49 8b 75 00 49 8b 55 08 4c 89 RSP: 0000:ff1100010743fc38 EFLAGS: 00010203 RAX: 03fc4400041d0ff1 RBX: ff1100010389a900 RCX: ffffffff9f940ad9 RDX: ff11000107429740 RSI: 0000000000000000 RDI: ff110001037ec920 RBP: ff1100010743fd50 R08: 0000000000000000 R09: ffe21c0020e87f1e R10: 0000000000000003 R11: 0000000000032001 R12: ff110001037ec800 R13: 1fe2200020e87f8c R14: 0000000000000000 R15: dffffc0000000000 FS: 0000000000000000(0000) GS:ff1100011b000000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ff11000115201000 CR3: 0000000113066001 CR4: 0000000000771ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Kernel panic - not syncing: Fatal exception Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: 0x1da00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff) Rebooting in 1 seconds.. Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> --- v2: - add fixes tag --- lib/kunit/executor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)