linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Weimer <fw@deneb.enyo.de>
To: Andy Lutomirski <luto@kernel.org>
Cc: Linux API <linux-api@vger.kernel.org>,
	Peter Oskolkov <posk@google.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-toolchains@vger.kernel.org
Subject: Re: Is adding an argument to an existing syscall okay?
Date: Tue, 17 Nov 2020 18:16:28 +0100	[thread overview]
Message-ID: <87k0uj6h03.fsf@mid.deneb.enyo.de> (raw)
In-Reply-To: <CALCETrXU2KcH0nsH_vd-fmvpZt_yW2+=VnYtN_BQJ6xsSvm+6A@mail.gmail.com> (Andy Lutomirski's message of "Mon, 16 Nov 2020 15:57:40 -0800")

* 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.)  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.

  parent reply	other threads:[~2020-11-17 17:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-16 23:57 Is adding an argument to an existing syscall okay? Andy Lutomirski
2020-11-17 14:28 ` Mathieu Desnoyers
2020-11-17 17:05 ` Theodore Y. Ts'o
2020-11-17 17:16 ` Florian Weimer [this message]
2020-11-17 18:36   ` Segher Boessenkool
2020-11-17 18:44     ` Florian Weimer
2020-11-17 18:58       ` Peter Oskolkov
2020-11-17 19:21         ` Mathieu Desnoyers
2020-11-17 19:32           ` Peter Oskolkov
2020-11-17 19:45           ` Florian Weimer
2020-11-19  3:08 ` Aleksa Sarai

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=87k0uj6h03.fsf@mid.deneb.enyo.de \
    --to=fw@deneb.enyo.de \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=peterz@infradead.org \
    --cc=posk@google.com \
    /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;
as well as URLs for NNTP newsgroup(s).