linux-kernel - Re: [PATCH v6 37/41] selftests/x86: Add shadow stack test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5fe0874655a7190a6ea5a070584d2603522f4395.camel@intel.com>
Date:   Thu, 23 Feb 2023 17:54:55 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "bp@...en8.de" <bp@...en8.de>
CC:     "david@...hat.com" <david@...hat.com>,
        "bsingharora@...il.com" <bsingharora@...il.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "Syromiatnikov, Eugene" <esyr@...hat.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "Yu, Yu-cheng" <yu-cheng.yu@...el.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "Eranian, Stephane" <eranian@...gle.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "fweimer@...hat.com" <fweimer@...hat.com>,
        "nadav.amit@...il.com" <nadav.amit@...il.com>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "dethoma@...rosoft.com" <dethoma@...rosoft.com>,
        "kcc@...gle.com" <kcc@...gle.com>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "pavel@....cz" <pavel@....cz>, "oleg@...hat.com" <oleg@...hat.com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "Yang, Weijiang" <weijiang.yang@...el.com>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "arnd@...db.de" <arnd@...db.de>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Schimpe, Christina" <christina.schimpe@...el.com>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "debug@...osinc.com" <debug@...osinc.com>,
        "jamorris@...ux.microsoft.com" <jamorris@...ux.microsoft.com>,
        "john.allen@....com" <john.allen@....com>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "corbet@....net" <corbet@....net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "gorcunov@...il.com" <gorcunov@...il.com>
Subject: Re: [PATCH v6 37/41] selftests/x86: Add shadow stack test

On Thu, 2023-02-23 at 14:47 +0100, Borislav Petkov wrote:
> On Sat, Feb 18, 2023 at 01:14:29PM -0800, Rick Edgecombe wrote:
> > Since this test exercises a recently added syscall manually, it
> > needs
> > to find the automatically created __NR_foo defines. Per the
> > selftest
> > documentation, KHDR_INCLUDES can be used to help the selftest
> > Makefile's
> 
> Well, why don't you make it easier for the user of this to not have
> to
> jump through hoops to get the test built?
> 
> IOW, something like the below ontop.
> 
> It works if I do
> 
> $ make -j<num> test_shadow_stack_64
> 
> It would only need to be fixed to work when you do
> 
> $ make -j<num>
> 
> without arguments as then make does a parallel build.
> 
> I guess something like
> 
> ifneq ($(filter test_shadow_stack_64, $(MAKECMDGOALS)),)
> .NOTPARALLEL:
> endif
> 
> needs to happen but I'm not sure...

Ah, I see. I had built the kernel with CONFIG_HEADERS_INSTALL and so
this was already done for me.

The proposed Makefile solution seems a bit unusual. What about this
less complicated solution to just make this case work?
diff --git a/tools/testing/selftests/x86/test_shadow_stack.c
b/tools/testing/selftests/x86/test_shadow_stack.c
index 71de3527c67a..02fe1b135ba8 100644
--- a/tools/testing/selftests/x86/test_shadow_stack.c
+++ b/tools/testing/selftests/x86/test_shadow_stack.c
@@ -34,6 +34,24 @@
 
 #define SS_SIZE 0x200000
 
+/*
+ * Define the ABI defines if needed, so people can run the tests
+ * without building the headers.
+ */
+#ifndef __NR_map_shadow_stack
+#define __NR_map_shadow_stack          451
+#define SHADOW_STACK_SET_TOKEN         (1ULL << 0)
+
+#define ARCH_SHSTK_ENABLE              0x5001
+#define ARCH_SHSTK_DISABLE             0x5002
+#define ARCH_SHSTK_LOCK                        0x5003
+#define ARCH_SHSTK_UNLOCK              0x5004
+#define ARCH_SHSTK_STATUS              0x5005
+
+#define ARCH_SHSTK_SHSTK               (1ULL <<  0)
+#define ARCH_SHSTK_WRSS                        (1ULL <<  1)
+#endif
+
 #if (__GNUC__ < 8) || (__GNUC__ == 8 && __GNUC_MINOR__ < 5)
 int main(int argc, char *argv[])
 {


I was a bit surprised this was even as tricky as the KHDR_INCLUDES
part. The other tests seem to be fine only because they use old header
definitions that have been there forever. So it seems like the proper
way to build the selftests should involve building the headers. So
alternatively, why not just always encourage building the headers
before running the selftests by warning if
${abs_srctree}/usr/include/linux is not found?

Do either of those seem any better?