Hi Linus,
On 22/06/21 11:14 am, Linus Torvalds wrote:
On Mon, Jun 21, 2021 at 12:45 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
Looks like sys_exit() and do_group_exit() would be the two places to
do it (do_group_exit() would handle the signal case and
sys_group_exit()).
Maybe... I'm digging through that pile right now, will follow up when
I get a reasonably complete picture
We might have another possible way to solve this:
(a) make it the rule that everybody always saves the full (integer)
register set in pt_regs
(b) make m68k just always create that switch-stack for all system
calls (it's really not that big, I think it's like six words or
something)
Turns out that is harder than it looked at first glance (at least for me).
All syscalls that _do_ save the switch stack are currently called
through wrappers which pull the syscall arguments out of the saved
pt_regs on the stack (pushing the switch stack after the SAVE_ALL saved
stuff buries the syscall arguments on the stack, see comment about
m68k_clone(). We'd have to push the switch stack _first_ when entering
system_call to leave the syscall arguments in place, but that will
require further changes to the syscall exit path (currently shared with
the interrupt exit path). Not to mention the register offset
calculations in arch/m68k/kernel/ptrace.c, and perhaps a few other
dependencies that don't come to mind immediately.
We have both pt_regs and switch_stack in uapi/asm/ptrace.h, but the
ordering of the two is only mentioned in a comment. Can we reorder them
on the stack, as long as we don't change the struct definitions proper?
This will take a little more time to work out and test - certainly not
before the weekend. I'll send a corrected version of my debug patch
before that.
Cheers,
Michael
(c) admit that alpha is broken, but nobody really cares
In the meanwhile, do kernel/kthread.c uses look even remotely sane?
Intentional - sure, but it really looks wrong to use thread exit code
as communication channel there...
I really doubt that it is even "intentional".
I think it's "use some errno as a random exit code" and nobody ever
really thought about it, or thought about how that doesn't really
work. People are used to the error numbers, not thinking about how
do_exit() doesn't take an error number, but a signal number (and an
8-bit positive error code in bits 8-15).
Because no, it's not even remotely sane.
I think the do_exit(-EINTR) could be do_exit(SIGINT) and it would make
more sense. And the -ENOMEM might be SIGBUS, perhaps.
It does look like the usermode-helper code does save the exit code
with things like
kernel_wait(pid, &sub_info->retval);
and I see call_usermodehelper_exec() doing
retval = sub_info->retval;
and treating it as an error code. But I think those have never been
tested with that (bogus) exit code thing from kernel_wait(), because
it wouldn't have worked. It has only ever been tested with the (real)
exit code things like
if (pid < 0) {
sub_info->retval = pid;
which does actually assign a negative error code to it.
So I think that
kernel_wait(pid, &sub_info->retval);
line is buggy, and should be something like
int wstatus;
kernel_wait(pid, &wstatus);
sub_info->retval = WEXITSTATUS(wstatus) ? -EINVAL : 0;
or something.
Linus
