Re: Q: Can we get rid of __copy_siginfo_to_user32?

[Andy Lutomirski]

> On Apr 10, 2018, at 6:26 PM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
>
>
> Andy,
>
> I am looking at copy_siginfo_to_user32 and find it very unfortunate
> that x86 with _sigchld_x32 needs to be the odd man out.  I am looking
> at ways to simplify the special case.
>
> The core of the special case comes from:
> exit_to_usermode_loop
>  do_signal
>    handle_signal
>       setup_rt_frame
>
>
> In setup_rt_frame the code looks at ksig to see which kind of signal
> frame should be written for the signal.
>
> This leads to the one case in the kernel where copy_siginfo_to_user32
> does not use is_ia32_syscall() or is_x32_syscall() to see which kind of
> signal frame it needs to create.
>
> Andy, since you have been all over the entry point code in recent years
> do you know if we allow tasks that can do both ia32 and x86_64 system
> calls?  That seems to be what we the testing of ksig to see which kind
> of signal frame to setup is all about.

We do :(

> If we don't allow mixed abi's on x86_64 then can I see if I have a ia32
> task in setup_rt_frame by just calling is_ia32_syscall()?
>
> If we do allow mixed abi's do you know if it would be safe to
> temporarily play with orig_ax or current_thread_info()->status?

Maybe, but itâs a real minefield. I think the right fix is to use
sa_flags's SA_X32_ABI bit instead for the sigchld bit.  In general,
the is_..._syscall() helpers can't be expected to return anything
valid in any context other than a syscall, and handle_signal() is not
a syscall.

>
> My goal is to write two wrappers: copy_siginfo_to_user32_ia32, and
> copy_siginfo_to_user32_x32 around the ordinary copy_siginfo_to_user32.
> With only a runtime test to see which ABI we need to implement.
>
> Aka change:
>>    case SIL_CHLD:
>>        to->si_pid    = from->si_pid;
>>        to->si_uid    = from->si_uid;
>>        to->si_status = from->si_status;
>> #ifdef CONFIG_X86_X32_ABI
>>        if (x32_ABI) {
>>            to->_sifields._sigchld_x32._utime = from->si_utime;
>>            to->_sifields._sigchld_x32._stime = from->si_stime;
>>        } else
>> #endif
>>        {
>>            to->si_utime = from->si_utime;
>>            to->si_stime = from->si_stime;
>>        }
>>        break;
> to something like:
>>    case SIL_CHLD:
>>        to->si_pid    = from->si_pid;
>>        to->si_uid    = from->si_uid;
>>        to->si_status = from->si_status;
>> #ifdef CONFIG_X86_X32_ABI
>>        if (!is_ia32_syscall()) {
>>            to->_sifields._sigchld_x32._utime = from->si_utime;
>>            to->_sifields._sigchld_x32._stime = from->si_stime;
>>        } else
>> #endif
>>        {
>>            to->si_utime = from->si_utime;
>>            to->si_stime = from->si_stime;
>>        }
>>        break;
>

Makes sense, but can you get to sa_flags in there instead?

FWIW, I have a branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=execve

that contains a *massive* cleanup of some other x86 signal stuff.  I
need to dust it off and test it better.