From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.2 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 484F8C4332E for ; Tue, 2 Mar 2021 12:15:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 226E464F59 for ; Tue, 2 Mar 2021 12:15:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1446575AbhCBML6 (ORCPT ); Tue, 2 Mar 2021 07:11:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349617AbhCBLv0 (ORCPT ); Tue, 2 Mar 2021 06:51:26 -0500 Received: from mail-ot1-x329.google.com (mail-ot1-x329.google.com [IPv6:2607:f8b0:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 766ADC061756 for ; Tue, 2 Mar 2021 03:39:24 -0800 (PST) Received: by mail-ot1-x329.google.com with SMTP id h22so19650880otr.6 for ; Tue, 02 Mar 2021 03:39:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Awszz/knFkSmYzdRRUyEmKjIRZ+f0H/e4tpJCA3RU4E=; b=iBhA66Cxg23wOsfcdwMy15/pGeHKmWxWBtnrognin+Gbjcw/rO49bKoczbvs4vQ6Ch C/PT/JvTva0UPczgQ30s9iGCznwtsFb9JmKlLXcBer9NHDCneium9raiqwARd+v1lvN5 8ZQpIWwM8O0sQKitNXBXU5hX+KdBYqVNLpRp6P1YTUDunswnQIUWDTgo99gEsQacmi0V xdwjcZF9oBW46YetIHjuLScFtHq+WQ22S1/nH9cA5ufZqJFmFtTWkOKyKdJecE4r85VH M9mNJheRK4341KjdpWsVSZzQJ0zU80h8WiPlc/o9EY46SAheF4wnI6J6NI4VShho44Rx aqCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Awszz/knFkSmYzdRRUyEmKjIRZ+f0H/e4tpJCA3RU4E=; b=m3ARRS0hhCrBmG2Axi4J0vfsOVpddIm8sioRQJM5wNyx6VBHMPDS8f5fhDJVB6g62O lbtFWsB6u7GWNlNzoUyXPyjgW0gXc+/DLeEZ/2Sv9iOJRG9eCLUFi2TZ/qQsyS+9dq9z nkekZIx8nw0PSTg7bIU8OtwfuoSfZZqstNwHeCyGLzNYD+/epGBQQqMvjAvEcjgDcqML 6rZzVUCkOTriMcKF9t5teb/Z8NXq9LsHwc8UQWdHryodtxhk/HCXiClzlTjCBcTWc1S+ C0XagOg/YkSmAil2/RLYBg7iTbRAQqobLCPHmHzZWAZQ1WGMQYJiZSvoOtvgCWIAHKbu mOBg== X-Gm-Message-State: AOAM531zPGzE9YyNPKBIt2PyQzP1y9cHk/HyOFaSugiTKKFLTtTNCbG2 hyhUSJUtJ8D+guCG/BjVssKHV9bJdl5YmCz+fC1ZZg== X-Google-Smtp-Source: ABdhPJz2v7sceHM/XgV8W6hy42SjEr7jDNzCwy5D2aAzX26SvjSwaX5eE8U45FAcQiVnegNgzv9NQi6tHIv9d8GDwK0= X-Received: by 2002:a9d:644a:: with SMTP id m10mr17823761otl.233.1614685163647; Tue, 02 Mar 2021 03:39:23 -0800 (PST) MIME-Version: 1.0 References: <51c397a23631d8bb2e2a6515c63440d88bf74afd.1614674144.git.christophe.leroy@csgroup.eu> <08a96c5d-4ae7-03b4-208f-956226dee6bb@csgroup.eu> In-Reply-To: <08a96c5d-4ae7-03b4-208f-956226dee6bb@csgroup.eu> From: Marco Elver Date: Tue, 2 Mar 2021 12:39:12 +0100 Message-ID: Subject: Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32 To: Christophe Leroy Cc: Alexander Potapenko , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Dmitry Vyukov , LKML , linuxppc-dev@lists.ozlabs.org, kasan-dev Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2 Mar 2021 at 12:21, Christophe Leroy wrote: [...] > >> Booting with 'no_hash_pointers" I get the following. Does it helps ? > >> > >> [ 16.837198] ================================================================== > >> [ 16.848521] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c > >> [ 16.848521] > >> [ 16.857158] Invalid read at 0xdf98800a: > >> [ 16.861004] finish_task_switch.isra.0+0x54/0x23c > >> [ 16.865731] kunit_try_run_case+0x5c/0xd0 > >> [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30 > >> [ 16.875199] kthread+0x15c/0x174 > >> [ 16.878460] ret_from_kernel_thread+0x14/0x1c > >> [ 16.882847] > >> [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: G B > >> 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674 > >> [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38 > >> [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: G B > >> (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty) > >> [ 16.911386] MSR: 00009032 CR: 22000004 XER: 00000000 > >> [ 16.918153] DAR: df98800a DSISR: 20000000 > >> [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c 00000008 c084b32b c016eb38 > >> [ 16.918153] GPR08: c0850000 df988000 c0d10000 e2449eb0 22000288 > >> [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108 > >> [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0 > >> [ 16.947292] Call Trace: > >> [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c (unreliable) > > > > The "(unreliable)" might be a clue that it's related to ppc32 stack > > unwinding. Any ppc expert know what this is about? > > > >> [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0 > >> [ 16.963319] [e2449ed0] [c02f63ec] kunit_generic_run_threadfn_adapter+0x24/0x30 > >> [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174 > >> [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c > >> [ 16.981896] Instruction dump: > >> [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 39000000 995f0024 907f0028 90ff001c > >> [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f > >> [ 17.000711] ================================================================== > >> [ 17.008223] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636 > >> [ 17.008223] Expected report_matches(&expect) to be true, but is false > >> [ 17.023243] not ok 21 - test_invalid_access > > > > On a fault in test_invalid_access, KFENCE prints the stack trace based > > on the information in pt_regs. So we do not think there's anything we > > can do to improve stack printing pe-se. > > stack printing, probably not. Would be good anyway to mark the last level [unreliable] as the ppc does. We use stack_trace_save_regs() + stack_trace_print(). > IIUC, on ppc the address in the stack frame of the caller is written by the caller. In most tests, > there is some function call being done before the fault, for instance > test_kmalloc_aligned_oob_read() does a call to kunit_do_assertion which populates the address of the > call in the stack. However this is fragile. Interesting, this might explain it. > This works for function calls because in order to call a subfunction, a function has to set up a > stack frame in order to same the value in the Link Register, which contains the address of the > function's parent and that will be clobbered by the sub-function call. > > However, it cannot be done by exceptions, because exceptions can happen in a function that has no > stack frame (because that function has no need to call a subfunction and doesn't need to same > anything on the stack). If the exception handler was writting the caller's address in the stack > frame, it would in fact write it in the parent's frame, leading to a mess. > > But in fact the information is in pt_regs, it is in regs->nip so KFENCE should be able to use that > instead of the stack. Perhaps stack_trace_save_regs() needs fixing for ppc32? Although that seems to use arch_stack_walk(). > > What's confusing is that it's only this test, and none of the others. > > Given that, it might be code-gen related, which results in some subtle > > issue with stack unwinding. There are a few things to try, if you feel > > like it: > > > > -- Change the unwinder, if it's possible for ppc32. > > I don't think it is possible. > > > > > -- Add code to test_invalid_access(), to get the compiler to emit > > different code. E.g. add a bunch (unnecessary) function calls, or add > > barriers, etc. > > The following does the trick > > diff --git a/mm/kfence/kfence_test.c b/mm/kfence/kfence_test.c > index 4acf4251ee04..22550676cd1f 100644 > --- a/mm/kfence/kfence_test.c > +++ b/mm/kfence/kfence_test.c > @@ -631,8 +631,11 @@ static void test_invalid_access(struct kunit *test) > .addr = &__kfence_pool[10], > .is_write = false, > }; > + char *buf; > > + buf = test_alloc(test, 4, GFP_KERNEL, ALLOCATE_RIGHT); > READ_ONCE(__kfence_pool[10]); > + test_free(buf); > KUNIT_EXPECT_TRUE(test, report_matches(&expect)); > } > > > But as I said above, this is fragile. If for some reason one day test_alloc() gets inlined, it may > not work anymore. Yeah, obviously that's hack, but interesting nevertheless. Based on what you say above, however, it seems that stack_trace_save_regs()/arch_stack_walk() don't exactly do what they should? Can they be fixed for ppc32? Thanks, -- Marco