Subject: bug#30918: Don't use syscall() to call renameat2()

From: Seebs <[email-masked]>
Date: Sun, 25 Mar 2018 20:52:32 -0500

On Sun, 25 Mar 2018 18:20:56 -0700
Paul Eggert <[email-masked]> wrote:

> Seebs wrote:
> > I have significant concerns about the feasibility of a generic
> > wrapper for syscall(). In particular, a "wrapper" which does
> > *nothing* but forward arguments may well be practical. Or one which
> > just fails immediately and claims ENOTSUPP -- but this creates the
> > risk that we'll break things which were using perfectly valid
> > syscalls which work fine and which we don't need to intercept or do
> > anything with.
> 
> For this particular issue, failing with ENOTSUPP should do. Perhaps
> such a behavior could be available as a link-time or runtime option.
> 
> More precise would be to have syscall to do nothing but forward
> arguments, *except* for the renameat2 syscall which would work much
> like the renameat wrapper that I assume you already have. This would
> work for coreutils, shouldn't break anything else, shouldn't require
> a link-time or runtime option, and shouldn't be that much harder than
> always forwarding syscall arguments.

We actually hadn't added a wrapper for it; so far as I can tell, no
one's ever used it before. (Or at least, no one's ever used it in such
a way that failing to catch it was causing detectable issues.) It's on
my list now that I've seen an actual program attempting to use it.

And I don't think it's that simple; as noted, I'm not convinced that
a function written in C can reliably interpret the arguments of some
syscalls across targets. (renameat2 may not be one of those affected.)
For instance, as noted in the man page, if I wanted to try to interpret
the arguments passed to SYS_readahead, I'd have to do things differently
for EABI ARM than I would for x86. That's a degree of additional magic
not previously present in any of the wrappers, perhaps surprisingly.

I haven't got a MIPS machine handy to go look, but the kind of thing
I'm concerned about is the description from syscall(2):

>       On a few architectures, a register is used to indicate simple
>       boolean failure of the system call:  ia64 uses r10 for this
>       purpose, and mips uses a3.

If I'm writing a wrapper in C, I can't preserve that value, but I have
to make *other* system calls before and after calling the underlying
wrapper.

So if it's the case that, after a call into syscall(), some value has
been stored in register a3 on MIPS... There's nothing I can write in
C that will preserve that value for my caller, and the other system
calls I make after the call to the "real" syscall() may overwrite it.
So the caller will get the wrong value, and if they were assuming that
syscall() would perform as expected...

This function may actually be Too Magic to sanely wrap. I think this is
the only library function I've ever seen document a need to insert an
unused argument between two arguments, but only for a specific ABI.

(We do have a couple of arch-specific hooks, but they're at the level
of "which compatibility version to specify for a particular function".)

-s