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

[Seebs]

On Sun, 25 Mar 2018 11:15:48 +0100
Richard Purdie <address@hidden> wrote:

> > On 03/23/2018 10:38 AM, Ross Burton wrote:
> > > Please consider changing renameat2.c so that it doesn't hit 
> > > syscall() if the wrapper isn't available.
> > 
> > That would reintroduce race-condition security holes in the
> > ordinary build of GNU Coreutils on GNU/Linux, which would not be a
> > good thing. Instead, how about fixing fakeroot so that it traps
> > 'syscall' and fails with errno == ENOTSUP? Better yet, fix fakeroot
> > so that it implements the renameat2 semantics with that syscall.
> > (Or even better, add renameat2 to both glibc and fakeroot. :-)
> 
> I've just had a look at this situation and its not as simple as it may
> first appear. The function prototype for syscall() in posix/unistd.h
> is:
> 
> extern long int syscall (long int __sysno, ...)
> 
> and the implementation in glibc is in assembler for each architecture.
> The syscall(2) man page also gives a little bit more of a hint of the
> challenges in the syscall() function with register splits and
> alignment along with different forms of error handling. You can call
> it with varying numbers of options and the register usage needs to be
> tightly controlled, its not a "normal" function where standard
> function calling conventions will always work.
> 
> So yes, we could add a wrapper in pseudo however we're likely going to
> have to end up using assembler to avoid smashing the calling stack in
> the general case. That would be on a per architecture basis and comes
> with all the complexities that brings.
> 
> I'd therefore like to add my own plea to figure out and use some glibc
> API for this even if we have to establish it.

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.

But if we don't want to break code which is using syscall() for other
operations, we would have to (1) successfully forward all system calls
*and* handle their returns, (2) also intercept specific cases and
modify their parameters. Which requires us to *comprehend* their
parameters. For instance, in the pseudo environment, we may be
virtualizing a chroot() operation, so a literal renameat2() argument of
"/a" gets translated into "/chroot/path/a" before it gets handed to the
kernel.

Take a look at the man page for syscall(2), and consider what we have
to do if we want to *handle* the arguments in any way. For instance, if
we needed to intercept SYS_readahead on EABI (we wouldn't, but it's
the example they give in the man page), we'd have to process arguments
completely differently from if we were processing it on x86. I am not
sure whether there's parallel concerns for 64-bit pointers on a 64-bit
ARM system. I would also have concerns about the "sets registers to
indicate success" behavior; wrapper functions are going to make *other
system calls* after calling the underlying syscall, so things like that
could (and in that case, I think probably would) get smashed by the
later syscalls.

I'm assuming the race condition refers to the behavior of
RENAME_EXCHANGE. I hadn't seen that before, and I don't know of an
existing mv(1) usage which would use it, but it does seem an
exceptionally desireable thing to have available. On the other hand,
I'm not sure it's technically *possible* to fix this in pseudo.
(I'm aware that pseudo as a whole is well past the realm of "merely
undefined" behavior and into "why would you do that, what's wrong with
you", but we haven't been able to make the requirement go away.)

I will be adding a wrapper for renameat2() to pseudo, but I can't make
glibc change its behavior so quickly.

(And now that I look more closely at the flags, supporting
RENAME_EXCHANGE will require more complicated effort than I'd initially
realized.)

-s