On Thu, Aug 31, 2023 at 3:17 AM 'Jinjie Ruan' via KUnit Development
<kunit-dev@xxxxxxxxxxxxxxxx> wrote:
>
> Inject fault while probing kunit-example-test.ko, if kzalloc fails
> in kunit_parse_glob_filter(), strcpy() or strncpy() to NULL will
> cause below null-ptr-deref bug. So check NULL for kzalloc() and
> return int instead of void for kunit_parse_glob_filter().
>
> Unable to handle kernel paging request at virtual address dfff800000000000
> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> Mem abort info:
> ESR = 0x0000000096000005
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x05: level 1 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [dfff800000000000] address between user and kernel address ranges
> Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
> Modules linked in: kunit_example_test cfg80211 rfkill 8021q garp mrp stp llc ipv6 [last unloaded: kunit_example_test]
> CPU: 4 PID: 6047 Comm: modprobe Tainted: G W N 6.5.0-next-20230829+ #141
> Hardware name: linux,dummy-virt (DT)
> pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : strncpy+0x58/0xc0
> lr : kunit_filter_suites+0x15c/0xa84
> sp : ffff800082a17420
> x29: ffff800082a17420 x28: 0000000000000000 x27: 0000000000000004
> x26: 0000000000000000 x25: ffffa847e40a5320 x24: 0000000000000001
> x23: 0000000000000000 x22: 0000000000000001 x21: dfff800000000000
> x20: 000000000000002a x19: 0000000000000000 x18: 00000000750b3b54
> x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> x14: 0000000000000000 x13: 34393178302f3039 x12: ffff7508fcea4ec1
> x11: 1ffff508fcea4ec0 x10: ffff7508fcea4ec0 x9 : dfff800000000000
> x8 : ffff6051b1a7f86a x7 : ffff800082a17270 x6 : 0000000000000002
> x5 : 0000000000000098 x4 : ffff028d9817b250 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : ffffa847e40a5320 x0 : 0000000000000000
> Call trace:
> strncpy+0x58/0xc0
> kunit_filter_suites+0x15c/0xa84
> kunit_module_notify+0x1b0/0x3ac
> blocking_notifier_call_chain+0xc4/0x128
> do_init_module+0x250/0x594
> load_module+0x37b0/0x44b4
> init_module_from_file+0xd4/0x128
> idempotent_init_module+0x2c8/0x524
> __arm64_sys_finit_module+0xac/0x100
> invoke_syscall+0x6c/0x258
> el0_svc_common.constprop.0+0x160/0x22c
> do_el0_svc+0x44/0x5c
> el0_svc+0x38/0x78
> el0t_64_sync_handler+0x13c/0x158
> el0t_64_sync+0x190/0x194
> Code: 5400028a d343fe63 12000a62 39400034 (38f56863)
> ---[ end trace 0000000000000000 ]---
> Kernel panic - not syncing: Oops: Fatal exception
> SMP: stopping secondary CPUs
> Kernel Offset: 0x284761400000 from 0xffff800080000000
> PHYS_OFFSET: 0xfffffd7380000000
> CPU features: 0x88000203,3c020000,1000421b
> Memory Limit: none
> Rebooting in 1 seconds..
>
> Fixes: a127b154a8f2 ("kunit: tool: allow filtering test cases via glob")
> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
Hello!
This also looks good to me. Thanks for catching these!
Reviewed-by: Rae Moar <rmoar@xxxxxxxxxx>
Thanks!
-Rae
> ---
> lib/kunit/executor.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 5181aa2e760b..25ce8d6e5fe7 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -65,7 +65,7 @@ struct kunit_glob_filter {
> };
>
> /* Split "suite_glob.test_glob" into two. Assumes filter_glob is not empty. */
> -static void kunit_parse_glob_filter(struct kunit_glob_filter *parsed,
> +static int kunit_parse_glob_filter(struct kunit_glob_filter *parsed,
> const char *filter_glob)
> {
> const int len = strlen(filter_glob);
> @@ -73,16 +73,28 @@ static void kunit_parse_glob_filter(struct kunit_glob_filter *parsed,
>
> if (!period) {
> parsed->suite_glob = kzalloc(len + 1, GFP_KERNEL);
> + if (!parsed->suite_glob)
> + return -ENOMEM;
> +
> parsed->test_glob = NULL;
> strcpy(parsed->suite_glob, filter_glob);
> - return;
> + return 0;
> }
>
> parsed->suite_glob = kzalloc(period - filter_glob + 1, GFP_KERNEL);
> + if (!parsed->suite_glob)
> + return -ENOMEM;
> +
> parsed->test_glob = kzalloc(len - (period - filter_glob) + 1, GFP_KERNEL);
> + if (!parsed->test_glob) {
> + kfree(parsed->suite_glob);
> + return -ENOMEM;
> + }
>
> strncpy(parsed->suite_glob, filter_glob, period - filter_glob);
> strncpy(parsed->test_glob, period + 1, len - (period - filter_glob));
> +
> + return 0;
> }
>
> /* Create a copy of suite with only tests that match test_glob. */
> @@ -152,8 +164,13 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set,
> }
> copy_start = copy;
>
> - if (filter_glob)
> - kunit_parse_glob_filter(&parsed_glob, filter_glob);
> + if (filter_glob) {
> + *err = kunit_parse_glob_filter(&parsed_glob, filter_glob);
> + if (*err) {
> + kfree(copy);
> + return filtered;
> + }
> + }
>
> /* Parse attribute filters */
> if (filters) {
> --
> 2.34.1
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230831071655.2907683-3-ruanjinjie%40huawei.com.
