On December 1, 2018 9:51:18 PM GMT+13:00, Arnd Bergmann [off-list ref] wrote:

On Sat, Dec 1, 2018 at 12:54 AM Andy Lutomirski [off-list ref]
wrote:

quoted

On Fri, Nov 30, 2018 at 2:10 PM Arnd Bergmann [off-list ref] wrote:

quoted

On Fri, Nov 30, 2018 at 5:36 PM Andy Lutomirski [off-list ref]

wrote:

quoted

quoted

quoted

On Fri, Nov 30, 2018 at 3:41 AM Arnd Bergmann [off-list ref]

wrote:

quoted

quoted

quoted

quoted

siginfo_t as it is now still has a number of other downsides,

and Andy in

quoted

quoted

quoted

quoted

particular didn't like the idea of having three new variants on

x86

quoted

quoted

quoted

quoted

(depending on how you count). His alternative suggestion of

having

quoted

quoted

quoted

quoted

a single syscall entry point that takes a 'signfo_t __user *'

but interprets

quoted

quoted

quoted

quoted

it as compat_siginfo depending on

in_compat_syscall()/in_x32_syscall()

quoted

quoted

quoted

quoted

should work correctly, but feels wrong to me, or at least

inconsistent

quoted

quoted

quoted

quoted

with how we do this elsewhere.

quoted

quoted

The '548 | 0x40000000' part seems to be the only sensible
way to handle x32 here. What exactly would you propose to
avoid defining the other entry points?

I would propose that it should be 335 | 0x40000000.  I can't see any
reasonable way to teach the kernel to reject 335 | 0x40000000 that
wouldn't work just as well to accept it and make it do the right
thing.  Currently we accept it and do the *wrong* thing, which is no
good.

quoted

and we have to
add more complexity to the copy_siginfo_from_user()
implementation to duplicate the hack that exists in
copy_siginfo_from_user32().

What hack are you referring to here?

I mean this part:

#ifdef CONFIG_COMPAT
int copy_siginfo_to_user32(struct compat_siginfo __user *to,
                          const struct kernel_siginfo *from)
#if defined(CONFIG_X86_X32_ABI) || defined(CONFIG_IA32_EMULATION)
{
       return __copy_siginfo_to_user32(to, from, in_x32_syscall());
}
int __copy_siginfo_to_user32(struct compat_siginfo __user *to,
                      const struct kernel_siginfo *from, bool x32_ABI)
#endif
{
...
       case SIL_CHLD:
               new.si_pid    = from->si_pid;
               new.si_uid    = from->si_uid;
               new.si_status = from->si_status;
#ifdef CONFIG_X86_X32_ABI
               if (x32_ABI) {
                   new._sifields._sigchld_x32._utime = from->si_utime;
                   new._sifields._sigchld_x32._stime = from->si_stime;
               } else
#endif
               {
                       new.si_utime = from->si_utime;
                       new.si_stime = from->si_stime;
               }
               break;
...
}
#endif

If we have a '548 | 0x40000000' entry pointing to
__x32_compat_sys_procfd_kill, then that will do the right
thing. If you instead try to have x32 call into the native
sys_procfd_kill, then copy_siginfo_to_user() will also have
to know about x32, effectively duplicating that mess above,
unless you want to also change all users of
copy_siginfo_to_user32() to use copy_siginfo_to_user()
and handle all cases in one function.

I've been looking into having siginfo64_t
with the new copy_siginfo_to_user64()
function. It looks like a pretty intricate 
task. Are we sure that we want to go
down this road? I'm not sure that it'll be
worth it. Especially since we force yet
another signal struct on user space.