linux-kernel - Re: [PATCH] selftests/x86: Update map_shadow

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]

Date:   Fri, 1 Sep 2023 12:33:28 -0700
From:   Sohil Mehta <sohil.mehta@...el.com>
To:     Rick Edgecombe <rick.p.edgecombe@...el.com>, <x86@...nel.org>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "Andy Lutomirski" <luto@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Peter Zijlstra" <peterz@...radead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] selftests/x86: Update map_shadow_stack syscall nr

Hi Rick,

On 9/1/2023 11:16 AM, Rick Edgecombe wrote:
> Shadow stack's selftest utilizes the map_shadow_stack syscall. The
> syscall is new with the feature, but the selftests cannot automatically
> find the headers for the kernel source tree they are located in. This
> resulted in the shadow stack test failing to build until the brand new
> headers were installed.
> 

I am wondering why a definition for __NR_map_shadow_stack is missing in
include/uapi/asm-generic/unistd.h?

Wouldn't this mean that even if someone were to install the headers they
still wouldn't get the syscall number definition. Am I missing something?

> To avoid this, a copy of the new uapi defines needed by the test were
> included in the selftest (see link for discussion). When shadow stack was
> merged the syscall number was changed, but the copy in the selftest was
> not updated.
> 
> So update the copy of the syscall number define used when the required
> headers are not installed, to have the final syscall number from the
> merge.
> 

How about adding a fixes tag to make it a tiny bit easier for someone
who backports the shstk series?

Fixes: 81f30337ef4f ("selftests/x86: Add shadow stack test")

> Link: https://lore.kernel.org/lkml/Y%2FijdXoTAATt0+Ct@zn.tnic/
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> ---
>  tools/testing/selftests/x86/test_shadow_stack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/x86/test_shadow_stack.c b/tools/testing/selftests/x86/test_shadow_stack.c
> index 2188968674cb..757e6527f67e 100644
> --- a/tools/testing/selftests/x86/test_shadow_stack.c
> +++ b/tools/testing/selftests/x86/test_shadow_stack.c
> @@ -40,7 +40,7 @@
>   * without building the headers.
>   */
>  #ifndef __NR_map_shadow_stack
> -#define __NR_map_shadow_stack	452
> +#define __NR_map_shadow_stack	453
>  
>  #define SHADOW_STACK_SET_TOKEN	(1ULL << 0)
>  

Reviewed-by: Sohil Mehta <sohil.mehta@...el.com>

Apart from this patch, I think we also need something like commit
78252deb023c ("arch: Register fchmodat2, usually as syscall 452") to
reserve the 453 syscall number for the rest of the architectures.

Should I send one out if you don't have something prepared already?

Thanks,
Sohil