public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sohil Mehta <sohil.mehta@intel.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>,
	"Lutomirski, Andy" <luto@kernel.org>,
	"bp@alien8.de" <bp@alien8.de>, "x86@kernel.org" <x86@kernel.org>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "broonie@kernel.org" <broonie@kernel.org>
Subject: Re: [PATCH] selftests/x86: Update map_shadow_stack syscall nr
Date: Fri, 1 Sep 2023 15:19:17 -0700	[thread overview]
Message-ID: <148522e4-a4ec-a35b-df25-e04eeb5f51c4@intel.com> (raw)
In-Reply-To: <b92afa6dbd074409095e525204d538f451ee4823.camel@intel.com>

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


  reply	other threads:[~2023-09-01 22:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-01 18:16 [PATCH] selftests/x86: Update map_shadow_stack syscall nr Rick Edgecombe
2023-09-01 19:33 ` Sohil Mehta
2023-09-01 20:35   ` Edgecombe, Rick P
2023-09-01 22:19     ` Sohil Mehta [this message]
2023-09-01 22:51       ` Edgecombe, Rick P
2023-09-02  0:12         ` Sohil Mehta
2023-09-01 21:39 ` [tip: x86/urgent] " tip-bot2 for Rick Edgecombe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=148522e4-a4ec-a35b-df25-e04eeb5f51c4@intel.com \
    --to=sohil.mehta@intel.com \
    --cc=bp@alien8.de \
    --cc=broonie@kernel.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox