Re: in_compat_syscall() on x86

[Eric W. Biederman]

Al Viro <viro@xxxxxxxxxxxxxxxxxx> writes:

> On Mon, Jan 04, 2021 at 06:47:38PM -0600, Eric W. Biederman wrote:
>> >> It is defined in the Ubuntu kernel configs I've got lurking:
>> >> Both 3.8.0-19_generic (Ubuntu 13.04) and 5.4.0-56_generic (probably 20.04).
>> >> Which is probably why it is in my test builds (I've just cut out
>> >> a lot of modules).
>> 
>> Interesting.  That sounds like something a gentle prod to the Ubuntu
>> kernel team might get them to disable.  Especially if there are not any
>> x32 binaries in sight.
>
> What for?

Any code that no one uses is better off disabled or deleted.

There are maintenance and support costs to such code as they cause extra
work when maintaining the kernel, and because the code is practically
never tested inevitably bugs some of which turn into security issues
slip through.

Maybe I am wrong and there are interesting users of x32.  All I remember
is that last time this was discussed someone found a distro that
actually shipped an x32 build to users.  Which was just enough users to
keep x32 alive.  Given that distros are in the process of dropping 32bit
support I suspect that distro may be going if it is not already gone.

There are a lot of weird x32 special cases that it would be nice to get
rid of if no one uses x32, and could certainly be made less of an issue
if distro's that don't actually care about x32 simply stopped compiling
it in.

>> The core dump code is currently tied to what binary you exec.
>> The code in exec sets mm->binfmt, and the coredump code uses mm->binfmt
>> to pick the coredump handler.
>> 
>> An x32 binary will make all kinds of 64bit calls where it doesn't need
>> the compat handling.  And of course x32 binaries run in 64bit mode with
>> 32bit pointers so looking at the current execution mode doesn't help.
>> 
>> Further fun compat_binfmt_elf is shared between x32 and ia32, because
>> except for a few stray places they do exactly the same thing.
>
> FWIW, there's a series cleaning that crap up nicely; as a side benefit,
> it converts both compats on mips (o32 and n32) to regular compat_binfmt_elf.c
> Yes, the current mainline is bloody awful in that area (PRSTATUS_SIZE and
> SET_PR_FPVALID are not for weak stomach), but that's really not hard to
> get into sane shape - -next had that done in last cycle and I'm currently
> testing (well, building the test kernel) of port of that to 5.11-rc1.

That does sound interesting.  Anytime we can clean up arch specific
weirdness so that it simply becomes generic weirdness and it can be
tested and maintained by more people is always nice.

> I really don't see the point of getting rid of x32 - mips n32 is *not*
> going away, and that's an exact parallel.

>From what I maintain x32 and n32 are *not* exact parallels.  The change
in size of the timestamps of SIGCHLD is not present on any other
architecture.

The truth is someone years ago royallying messed up struct siginfo and
took a structure that should have been the same on 32bit and 64bit and
would up with a horrible monstrosity of unions.

> PS: if anything, I wonder if we would better off with binfmt_elf{64,32}.o,
> built from fs/binfmt_elf.c; it's not that hard to do.  With arseloads
> of weirdness going away if we do that...

It sounds like we are already on our way to having that.  I suspect you
would need an binfmt_elf_ops to handle the architecture specific bits,
but it should not be too bad.

Eric