Re: [PATCH] faccessat: fix error code on setreXid failure

[Rich Felker <dalias@libc.org>]

On Wed, Jan 31, 2018 at 12:51:58AM +0300, Alexander Monakov wrote:
> On Tue, 30 Jan 2018, Rich Felker wrote:
> > 
> > AFAIK wait4 can also return due to Stopped status or trace-related
> > reasons, not just exit. That was the motivation I think.
> 
> We know we are not tracing this child, and stop notifications are only
> delivered if WUNTRACED is given in flags, aren't they?

I'm not sure what can happen if it's all running under strace -f or
something. And I'm not sure what the conditions for stop notifications
are. If it's assured that they can't happen then maybe the loop can be
removed.

> > > - the code seems to assume that the zombie will not be auto-collected even if
> > >   SIGCHLD disposition is set to SIG_IGN; this sounds logical, but not explicitly
> > >   documented as far as I can tell;
> > 
> > Indeed, I'm not sure, but I don't know any good fix.
> 
> Bring back the pipe (similar to how posix_spawn receives the status)?

Ah yes.

> > > --- a/src/unistd/faccessat.c
> > > +++ b/src/unistd/faccessat.c
> > > @@ -25,7 +25,7 @@ static int checker(void *p)
> > >  	int i;
> > >  	if (__syscall(SYS_setregid, __syscall(SYS_getegid), -1)
> > >  	    || __syscall(SYS_setreuid, __syscall(SYS_geteuid), -1))
> > > -		__syscall(SYS_exit, 1);
> > > +		return sizeof errors/sizeof *errors - 1;
> > >  	ret = __syscall(SYS_faccessat, c->fd, c->filename, c->amode, 0);
> > >  	for (i=0; i < sizeof errors/sizeof *errors - 1 && ret!=errors[i]; i++);
> > >  	return i;
> > 
> > Looks ok except it encodes an assumption that EBUSY is last. It might
> > make more sense to goto the errno-searching loop.
> 
> Well, the loop also implicitly encodes that assumption anyway: it stops
> at the last entry regardless if it matches, making EBUSY the fallback code
> for unrecognized SYS_faccessat return values.

Indeed.

> The loop will be gone if the pipe method is re-introduced.

Maybe this is the right approach. Anyone else have opinions on it?
This would just mean reverting the most recent commit to it then
possibly fixing other issues you found, right?

Rich