Message ID | 20200716193141.4068476-1-krisman@collabora.com (mailing list archive) |
---|---|
Headers | show |
Series | Syscall User Redirection | expand |
On Thu, Jul 16, 2020 at 03:31:39PM -0400, Gabriel Krisman Bertazi wrote: > This is v4 of Syscall User Redirection. The implementation itself is > not modified from v3, it only applies the latest round of reviews to the > selftests. > > __NR_syscalls is not really exported in header files other than > asm-generic for every architecture, so it felt safer to optionally > expose it with a fallback to a high value. > > Also, I didn't expose tests for PR_GET as that is not currently > implemented. If possible, I'd have it supported by a future patchset, > since it is not immediately necessary to support this feature. Thanks! That all looks good to me. > Finally, one question: Which tree would this go through? I haven't heard from several other x86 maintainers yet (which is where I would normally expect this series to land), but I would be comfortable taking this through my seccomp tree if I got Acks/Reviews at least from Andy and Matthew. Andy, Matthew, what do you think of this?
On Thu, Jul 16, 2020 at 01:04:38PM -0700, Kees Cook wrote: > On Thu, Jul 16, 2020 at 03:31:39PM -0400, Gabriel Krisman Bertazi wrote: > > This is v4 of Syscall User Redirection. The implementation itself is > > not modified from v3, it only applies the latest round of reviews to the > > selftests. > > > > __NR_syscalls is not really exported in header files other than > > asm-generic for every architecture, so it felt safer to optionally > > expose it with a fallback to a high value. > > > > Also, I didn't expose tests for PR_GET as that is not currently > > implemented. If possible, I'd have it supported by a future patchset, > > since it is not immediately necessary to support this feature. > > Thanks! That all looks good to me. Don't have any problem with this but did this ever get exposure on linux-api? This is the first time I see this pop up. Christian
On Thu, Jul 16, 2020 at 10:22:34PM +0200, Christian Brauner wrote: > On Thu, Jul 16, 2020 at 01:04:38PM -0700, Kees Cook wrote: > > On Thu, Jul 16, 2020 at 03:31:39PM -0400, Gabriel Krisman Bertazi wrote: > > > This is v4 of Syscall User Redirection. The implementation itself is > > > not modified from v3, it only applies the latest round of reviews to the > > > selftests. > > > > > > __NR_syscalls is not really exported in header files other than > > > asm-generic for every architecture, so it felt safer to optionally > > > expose it with a fallback to a high value. > > > > > > Also, I didn't expose tests for PR_GET as that is not currently > > > implemented. If possible, I'd have it supported by a future patchset, > > > since it is not immediately necessary to support this feature. > > > > Thanks! That all looks good to me. > > Don't have any problem with this but did this ever get exposure on > linux-api? This is the first time I see this pop up. I thought I'd added it to CC in the past, but that might have been other recent unrelated threads. Does this need a full repost there too, you think?
On Thu, Jul 16, 2020 at 01:25:43PM -0700, Kees Cook wrote: > On Thu, Jul 16, 2020 at 10:22:34PM +0200, Christian Brauner wrote: > > On Thu, Jul 16, 2020 at 01:04:38PM -0700, Kees Cook wrote: > > > On Thu, Jul 16, 2020 at 03:31:39PM -0400, Gabriel Krisman Bertazi wrote: > > > > This is v4 of Syscall User Redirection. The implementation itself is > > > > not modified from v3, it only applies the latest round of reviews to the > > > > selftests. > > > > > > > > __NR_syscalls is not really exported in header files other than > > > > asm-generic for every architecture, so it felt safer to optionally > > > > expose it with a fallback to a high value. > > > > > > > > Also, I didn't expose tests for PR_GET as that is not currently > > > > implemented. If possible, I'd have it supported by a future patchset, > > > > since it is not immediately necessary to support this feature. > > > > > > Thanks! That all looks good to me. > > > > Don't have any problem with this but did this ever get exposure on > > linux-api? This is the first time I see this pop up. > > I thought I'd added it to CC in the past, but that might have been other > recent unrelated threads. Does this need a full repost there too, you > think? Nah, wasn't my intention to force a repost. Seems that several people have looked this over. :) Just curious why it didn't get to linux-api and we know quite some people who only do look at linux-api (for sanity). :) Christian
Christian Brauner <christian.brauner@ubuntu.com> writes: > On Thu, Jul 16, 2020 at 01:25:43PM -0700, Kees Cook wrote: >> On Thu, Jul 16, 2020 at 10:22:34PM +0200, Christian Brauner wrote: >> > On Thu, Jul 16, 2020 at 01:04:38PM -0700, Kees Cook wrote: >> > > On Thu, Jul 16, 2020 at 03:31:39PM -0400, Gabriel Krisman Bertazi wrote: >> > > > This is v4 of Syscall User Redirection. The implementation itself is >> > > > not modified from v3, it only applies the latest round of reviews to the >> > > > selftests. >> > > > >> > > > __NR_syscalls is not really exported in header files other than >> > > > asm-generic for every architecture, so it felt safer to optionally >> > > > expose it with a fallback to a high value. >> > > > >> > > > Also, I didn't expose tests for PR_GET as that is not currently >> > > > implemented. If possible, I'd have it supported by a future patchset, >> > > > since it is not immediately necessary to support this feature. >> > > >> > > Thanks! That all looks good to me. >> > >> > Don't have any problem with this but did this ever get exposure on >> > linux-api? This is the first time I see this pop up. >> >> I thought I'd added it to CC in the past, but that might have been other >> recent unrelated threads. Does this need a full repost there too, you >> think? > > Nah, wasn't my intention to force a repost. Seems that several people > have looked this over. :) Just curious why it didn't get to linux-api > and we know quite some people who only do look at linux-api (for sanity). :) That's my mistake. I didn't think about it when submitting :( If this get re-spinned again I will make sure to CC linux-api.
On 7/16/20 4:30 PM, Gabriel Krisman Bertazi wrote: > Christian Brauner <christian.brauner@ubuntu.com> writes: > >> On Thu, Jul 16, 2020 at 01:25:43PM -0700, Kees Cook wrote: >>> On Thu, Jul 16, 2020 at 10:22:34PM +0200, Christian Brauner wrote: >>>> On Thu, Jul 16, 2020 at 01:04:38PM -0700, Kees Cook wrote: >>>>> On Thu, Jul 16, 2020 at 03:31:39PM -0400, Gabriel Krisman Bertazi wrote: >>>>>> This is v4 of Syscall User Redirection. The implementation itself is >>>>>> not modified from v3, it only applies the latest round of reviews to the >>>>>> selftests. >>>>>> >>>>>> __NR_syscalls is not really exported in header files other than >>>>>> asm-generic for every architecture, so it felt safer to optionally >>>>>> expose it with a fallback to a high value. >>>>>> >>>>>> Also, I didn't expose tests for PR_GET as that is not currently >>>>>> implemented. If possible, I'd have it supported by a future patchset, >>>>>> since it is not immediately necessary to support this feature. >>>>> >>>>> Thanks! That all looks good to me. >>>> >>>> Don't have any problem with this but did this ever get exposure on >>>> linux-api? This is the first time I see this pop up. >>> >>> I thought I'd added it to CC in the past, but that might have been other >>> recent unrelated threads. Does this need a full repost there too, you >>> think? >> >> Nah, wasn't my intention to force a repost. Seems that several people >> have looked this over. :) Just curious why it didn't get to linux-api >> and we know quite some people who only do look at linux-api (for sanity). :) > > That's my mistake. I didn't think about it when submitting :( > > If this get re-spinned again I will make sure to CC linux-api. Thank you! It helps C library implementors stay up to date and comment on changes that impact userspace ABIs and APIs. This patch set was new to me. Interesting new feature.
Hi! > This is v4 of Syscall User Redirection. The implementation itself is > not modified from v3, it only applies the latest round of reviews to the > selftests. > > __NR_syscalls is not really exported in header files other than > asm-generic for every architecture, so it felt safer to optionally > expose it with a fallback to a high value. > > Also, I didn't expose tests for PR_GET as that is not currently > implemented. If possible, I'd have it supported by a future patchset, > since it is not immediately necessary to support this feature. > > Finally, one question: Which tree would this go through? Should it come with Documentation? How does it interact with ptrace, seccomp and similar? Pavel (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Pavel Machek <pavel@ucw.cz> writes: > Hi! > >> This is v4 of Syscall User Redirection. The implementation itself is >> not modified from v3, it only applies the latest round of reviews to the >> selftests. >> >> __NR_syscalls is not really exported in header files other than >> asm-generic for every architecture, so it felt safer to optionally >> expose it with a fallback to a high value. >> >> Also, I didn't expose tests for PR_GET as that is not currently >> implemented. If possible, I'd have it supported by a future patchset, >> since it is not immediately necessary to support this feature. >> >> Finally, one question: Which tree would this go through? > > Should it come with Documentation? Hi, Thanks for the review. I will prepare it for the next iteration. > How does it interact with ptrace, seccomp and similar? That is a very good question. Regarding seccomp, this must take precedence, since the use case is emulation (it can be invoked with a different ABI) such that seccomp filtering by syscall number doesn't make sense in the first place. In addition, either the syscall is dispatched back to userspace, in which case there is no resource for seccomp to protect, or the syscall will be executed, and seccomp will execute next. Regarding ptrace, I experimented with before and after, and while the same ABI argument applies, I felt it was easier to debug if I let ptrace happen for syscalls that are dispatched back to userspace. In addition, doing it after ptrace makes the code in syscall_exit_work slightly simpler, since it doesn't require special handling for this feature. For PTRACE_SYSEMU in particular, either placing this before or after is a bit odd. I don't think there is a right answer for this one, but I don't see anyone wanting to use these features at the same time. I think as long as it is documented behavior, it should be fine either way.