Re: what the hell is compat_sys_x86_waitpid() for?

From: Al Viro
Date: Sat Mar 17 2018 - 15:07:18 EST


On Sat, Mar 17, 2018 at 11:19:55AM -0700, Linus Torvalds wrote:
> On Sat, Mar 17, 2018 at 11:01 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > and tell me what is the difference between those. In other words, the problem
> > with sys32_waitpid() was not that it didn't use proper wrappers - it's that
> > it was (and always had been) 100% pointless.
>
> That long long predates Dominik's patches, though.

Sure.

> It clearly is worth fixing, but don't blame Dominik for just
> mindlessly converting mindless code.

The thing is, this area probably won't be looked at for many years in the
future, so we'd better take the crap out now. Same problem as with mindless
checkpatch.pl "fixes" - something unusually-looking obviously has gotten
less attention, so removing the visual indication of that has a good chance
of hiding something dumb. Not a problem if the code is actually OK, but
that's worth checking.

> I suspect the reason is simply that the *regular* waitpid() was writtn
> in terms of sys_wait4(), and sys_wait4() needed a compat function, so
> then waitpid was basically just carried along.

Probably nobody remembers now...

Another thing touched by the same commit: pread64(). Comparing it with
other biarch architectures shows at least one dubious thing, also there
since way back. Namely, s390 has
if ((compat_ssize_t) count < 0)
return -EINVAL;
IOW, size >= 2G => -EINVAL. It *does* match the behaviour on 32bit -
on all everything including i386. rw_verify_area() starts with
int retval = -EINVAL;

inode = file_inode(file);
if (unlikely((ssize_t) count < 0))
return retval;
64bit read/write variants quietly truncate to 2Gb, so on amd64 a 32bit
binary calling pread() with negative size will do 2Gb read while on
i386 the same binary would've gotten -EINVAL. I'm not sure if it's
worth bothering with, though - it's not just pread(). read(2)
has exact same issue and yes, s390 does have a separate wrapper for
it, with the same check. The same goes for write(2) there.

Do we care about that, and if not - does s390 really need to bother?
The rest of biarch wrappers for sys_pread64() is very close to being
unifiable - the only real issue is whether this architecture has
a dummy padding argument between count and (halves of) pos. arm64,
mips and ppc do; parisc, s390, sparc and x86 do not...