Re: Is adding an argument to an existing syscall okay?

[Segher Boessenkool <segher@kernel.crashing.org>]

On Tue, Nov 17, 2020 at 06:16:28PM +0100, Florian Weimer wrote:
> * Andy Lutomirski:
> 
> > Linux 5.10 contains this patch:
> >
> > commit 2a36ab717e8fe678d98f81c14a0b124712719840
> > Author: Peter Oskolkov <posk@google.com>
> > Date:   Wed Sep 23 16:36:16 2020 -0700
> >
> >     rseq/membarrier: Add MEMBARRIER_CMD_PRIVATE_EXPEDITED_RSEQ
> >
> > This adds an argument to an existing syscall.  Before the patch,
> > membarrier had 2 parameters; now it has 3.  Is this really okay?  At
> > least the patch is careful and ignores the third parameter unless a
> > previously unused flag bit is set.
> 
> It's really iffy.  It's hard to break this in system call wrappers on
> x86-64, where we just load %eax and call into the kernel.  But on
> architectures which require argument shuffling, it will break.
> 
> If there were a system call wrapper in glibc (my patch was rejected
> due to lack of documentation fo the semantics, so we got lucky there),
> we'd have to add a new symbol version for this.  It happened before in
> the dark ages, repeatedly, but it's a bit disappointing to be in this
> situation again.
> 
> In general the main problem I see is the poor source code
> compatibility.  We really, really don't want variadic system call
> wrappers, and we specifically do not want to introduce them
> retroactively.  (Changing an implementation from non-variadic to
> variadic is not an ABI-safe change on POWER and probably other
> targets.)

But this isn't variadic in the sense of "..." -- on Power that always
passes the unspecified arguments in memory, while in this case it just
passes in either two or three registers.  I don't know any arg where
that would not work, given the Linux system call restrictions.

This is similar to the "open" system call.

> So we'd require that from now on, the programmer has to
> pass the zero argument explicitly.  Porting is simpler than the recent
> futex_time64 breakage, but the downside is that immediately impacts
> all targets.
> 
> Cc: linux-toolchains for ABI impact.

It certainly would simplify matters if this was simply not done ;-)


Segher