# Re: \[PATCH v2\] signal: add procfd\_signal() syscall - List: linux-api - 4 messages, 3 authors - Page: https://ratatoskr.run/linux-api/2018/12/147184/t ## Re: \[PATCH v2\] signal: add procfd\_signal() syscall - From: Eric W. Biederman \ - Date: 2018-12-02 02:41:40 - Message: https://ratatoskr.run/linux-api/2018/12/147184.md ```text It just occurs to me that the simple way to implement procfd_sigqueueinfo info is like: int copy_siginfo_from_user_any(kernel_siginfo_t *info, siginfo_t *uinfo) { #ifdef CONFIG_COMPAT if (in_compat_syscall) return copy_siginfo_from_user32(info, uinfo); #endif return copy_siginfo_from_user(info, uinfo); } long procfd_sigqueueinfo(int fd, siginfo_t *uinfo) { kernel_siginfo info; if (copy_siginfo_from_user_any(&info, uinfo)) return -EFAULT; ...; } It looks like there is already a place in ptrace.c that already hand rolls copy_siginfo_from_user_any. So while I would love to figure out the subset of siginfo_t tha we can just pass through, as I think that would make a better more forward compatible copy_siginfo_from_user32. I think for this use case we just add the in_compat_syscall test and then we just need to ensure this new system call is placed in the proper places in the syscall table. Because we will need 3 call sights: x86_64, x32 and ia32. As the layout changes between those three subarchitecuters. Eric ``` ## Re: \[PATCH v2\] signal: add procfd\_signal() syscall - From: Andy Lutomirski \ - Date: 2018-12-02 03:05:29 - Message: https://ratatoskr.run/linux-api/2018/12/147183.md ```text > On Dec 1, 2018, at 7:28 AM, Eric W. Biederman [ref] wrote: > > > It just occurs to me that the simple way to implement > procfd_sigqueueinfo info is like: > > int copy_siginfo_from_user_any(kernel_siginfo_t *info, siginfo_t *uinfo) > { > #ifdef CONFIG_COMPAT > if (in_compat_syscall) > return copy_siginfo_from_user32(info, uinfo); > #endif > return copy_siginfo_from_user(info, uinfo); > } > > long procfd_sigqueueinfo(int fd, siginfo_t *uinfo) > { > kernel_siginfo info; > > if (copy_siginfo_from_user_any(&info, uinfo)) > return -EFAULT; > ...; > } > > It looks like there is already a place in ptrace.c that already > hand rolls copy_siginfo_from_user_any. > > So while I would love to figure out the subset of siginfo_t tha we can > just pass through, as I think that would make a better more forward > compatible copy_siginfo_from_user32. Seems reasonable to me. It’s less code overall than any other suggestion, too. > I think for this use case we just > add the in_compat_syscall test and then we just need to ensure this new > system call is placed in the proper places in the syscall table. > > Because we will need 3 call sights: x86_64, x32 and ia32. As the layout > changes between those three subarchitecuters. > > If it’s done this way, it can just be “common” in the 64-bit table. And we kick the can a bit farther down the road :) I’m working on patches to clean up x86’s syscall mess. It’s slow because I keep finding new messes. So far I have rt_sigreturn working like every other syscall — whee. Also, Eric, for your edification, I have a draft patch set to radically simplify x86’s signal delivery and return. Once that’s done, I can trivially speed up delivery by a ton by using sysret. ``` ## Re: \[PATCH v2\] signal: add procfd\_signal() syscall - From: Christian Brauner \ - Date: 2018-12-02 03:40:45 - Message: https://ratatoskr.run/linux-api/2018/12/147182.md ```text On December 2, 2018 4:52:37 AM GMT+13:00, Andy Lutomirski [ref] wrote: > > >> On Dec 1, 2018, at 7:28 AM, Eric W. Biederman [ref] >wrote: >> >> >> It just occurs to me that the simple way to implement >> procfd_sigqueueinfo info is like: >> >> int copy_siginfo_from_user_any(kernel_siginfo_t *info, siginfo_t >*uinfo) >> { >> #ifdef CONFIG_COMPAT >> if (in_compat_syscall) >> return copy_siginfo_from_user32(info, uinfo); >> #endif >> return copy_siginfo_from_user(info, uinfo); > >> } >> >> long procfd_sigqueueinfo(int fd, siginfo_t *uinfo) >> { >> kernel_siginfo info; >> >> if (copy_siginfo_from_user_any(&info, uinfo)) >> return -EFAULT; >> ...; >> } >> >> It looks like there is already a place in ptrace.c that already >> hand rolls copy_siginfo_from_user_any. >> >> So while I would love to figure out the subset of siginfo_t tha we >can >> just pass through, as I think that would make a better more forward >> compatible copy_siginfo_from_user32. > >Seems reasonable to me. It’s less code overall than any other >suggestion, too. Thanks everyone, that was super helpful! All things equal I'm going to send out an updated version of the patch latest next week! > >> I think for this use case we just >> add the in_compat_syscall test and then we just need to ensure this >new >> system call is placed in the proper places in the syscall table. >> >> Because we will need 3 call sights: x86_64, x32 and ia32. As the >layout >> changes between those three subarchitecuters. >> >> > >If it’s done this way, it can just be “common” in the 64-bit table. And >we kick the can a bit farther down the road :) > >I’m working on patches to clean up x86’s syscall mess. It’s slow >because I keep finding new messes. So far I have rt_sigreturn working >like every other syscall — whee. > >Also, Eric, for your edification, I have a draft patch set to radically >simplify x86’s signal delivery and return. Once that’s done, I can >trivially speed up delivery by a ton by using sysret. ``` ## Re: \[PATCH v2\] signal: add procfd\_signal() syscall - From: Christian Brauner \ - Date: 2018-12-02 08:52:48 - Message: https://ratatoskr.run/linux-api/2018/12/147179.md ```text On Sat, Dec 01, 2018 at 09:28:47AM -0600, Eric W. Biederman wrote: > > It just occurs to me that the simple way to implement > procfd_sigqueueinfo info is like: > > int copy_siginfo_from_user_any(kernel_siginfo_t *info, siginfo_t *uinfo) > { > #ifdef CONFIG_COMPAT > if (in_compat_syscall) > return copy_siginfo_from_user32(info, uinfo); Right, though that would require a cast afaict. static int __copy_siginfo_from_user_generic(int signo, kernel_siginfo_t *kinfo, siginfo_t *info) { #ifdef CONFIG_COMPAT if (in_compat_syscall()) return __copy_siginfo_from_user32( signo, kinfo, (struct compat_siginfo __user *)info); #endif return __copy_siginfo_from_user(signo, kinfo, info); } It seems that a cast to (compat_siginfo __user *) should be safe in this context? I've at least seen similar things done for __sys_sendmsg(). > #endif > return copy_siginfo_from_user(info, uinfo); > } > > long procfd_sigqueueinfo(int fd, siginfo_t *uinfo) **bikeshedding** Not a fan of that name. I'm going to go with procfd_send_signal(). sigqueue gives non-native speakers a lot of room for spelling errors and it always seemed opaque to me what this function is doing without consulting the manpage. :) > { > kernel_siginfo info; > > if (copy_siginfo_from_user_any(&info, uinfo)) > return -EFAULT; > ...; > } > > It looks like there is already a place in ptrace.c that already > hand rolls copy_siginfo_from_user_any. > > So while I would love to figure out the subset of siginfo_t tha we can > just pass through, as I think that would make a better more forward > compatible copy_siginfo_from_user32. I think for this use case we just > add the in_compat_syscall test and then we just need to ensure this new > system call is placed in the proper places in the syscall table. > > Because we will need 3 call sights: x86_64, x32 and ia32. As the layout > changes between those three subarchitecuters. > > Eric > ```