Message-ID: <148522e4-a4ec-a35b-df25-e04eeb5f51c4@intel.com>
Date:   Fri, 1 Sep 2023 15:19:17 -0700
From:   Sohil Mehta <sohil.mehta@...el.com>
To:     "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "broonie@...nel.org" <broonie@...nel.org>
Subject: Re: [PATCH] selftests/x86: Update map_shadow_stack syscall nr

On 9/1/2023 1:35 PM, Edgecombe, Rick P wrote:
> On Fri, 2023-09-01 at 12:33 -0700, Sohil Mehta wrote:

>> 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?
> 
> There is some autogeneration that happens from the .tbl files.

Hmm.. I wasn't aware of this auto-generation. The last few system calls
additions have all updated the unistd.h file manually to add the
__NR_foo defines. So I assumed that's what gets included in the header
packages and eventually gets used by userspace in some form. But maybe
x86 headers might be built differently. I need to educate myself on this.

> But maybe also this whole duplicate defines thing is questionable.
> 

Yeah, having 2 defines is a bit annoying but as mentioned in the link it
makes it easier for someone to run selftests so it's probably worth the
effort. Also since syscall numbers typically never change it is just a
one time thing and not a big maintenance burden :)

FWIW, I see these syscall nr defines all across the selftests (io_uring,
powerpc/pkeys, seccomp, etc).

> 
>>
>> 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?
> 
> Originally there were no other shadow stack features, and so it was
> maybe going to be an x86-only syscall. I followed in the footsteps of 
> the secret mem syscall, but that one seems to have grown some similar
> reservation comments since then. It probably makes more sense for that
> one though, since it's sort of a generic functionality. An analogous
> x86 specific syscall would maybe be modify_ldt, which doesn't really
> have reservations.
> 
> But now we also have arm that plans to use it. So maybe it is worth
> trying to match syscall numbers? I could imagine scenarios where it
> could be useful. And I guess there is also the scenario where a generic
> type syscall is added, but only implemented on non-shadow stack
> architectures. So then when it makes it's way around, it can't match. I
> hadn't thought about it before, so just thinking it through...
> 

My suggestion is originating from the belief that at somepoint the
community decided that all *new* system call numbers would be the
consistent across architectures (except alpha). So that would mean
syscall number 453 has to be reserved on others even if it is an
x86-only syscall.

If we don't do this, and let say a generic sys_foo comes along which
uses the next available syscall number 453 on other archs, it would lead
to an inconsistency because 453 it is already used up on x86.

My memory of this is a bit hazy from my implementation of User
Interrupts more than a couple of years back. Also, I couldn't find any
handy documentation to support my belief. I'll try to dig more.

Sohil

Powered by blists - more mailing lists