2020-07-16 19:35:18

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v4 0/2] Syscall User Redirection

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?

Gabriel Krisman Bertazi (2):
kernel: Implement selective syscall userspace redirection
selftests: Add kselftest for syscall user dispatch

arch/Kconfig | 20 ++
arch/x86/Kconfig | 1 +
arch/x86/entry/common.c | 5 +
arch/x86/include/asm/thread_info.h | 4 +-
arch/x86/kernel/signal_compat.c | 2 +-
fs/exec.c | 2 +
include/linux/sched.h | 3 +
include/linux/syscall_user_dispatch.h | 50 ++++
include/uapi/asm-generic/siginfo.h | 3 +-
include/uapi/linux/prctl.h | 5 +
kernel/Makefile | 1 +
kernel/fork.c | 1 +
kernel/sys.c | 5 +
kernel/syscall_user_dispatch.c | 92 +++++++
tools/testing/selftests/Makefile | 1 +
.../syscall_user_dispatch/.gitignore | 2 +
.../selftests/syscall_user_dispatch/Makefile | 9 +
.../selftests/syscall_user_dispatch/config | 1 +
.../syscall_user_dispatch.c | 256 ++++++++++++++++++
19 files changed, 460 insertions(+), 3 deletions(-)
create mode 100644 include/linux/syscall_user_dispatch.h
create mode 100644 kernel/syscall_user_dispatch.c
create mode 100644 tools/testing/selftests/syscall_user_dispatch/.gitignore
create mode 100644 tools/testing/selftests/syscall_user_dispatch/Makefile
create mode 100644 tools/testing/selftests/syscall_user_dispatch/config
create mode 100644 tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c

--
2.27.0


2020-07-16 20:05:14

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] Syscall User Redirection

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?

--
Kees Cook

2020-07-16 20:25:09

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] Syscall User Redirection

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

2020-07-16 20:26:29

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] Syscall User Redirection

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?

--
Kees Cook

2020-07-16 20:31:35

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] Syscall User Redirection

Christian Brauner <[email protected]> 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.

--
Gabriel Krisman Bertazi

2020-07-16 20:32:19

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] Syscall User Redirection

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

2020-07-16 21:09:52

by Carlos O'Donell

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] Syscall User Redirection

On 7/16/20 4:30 PM, Gabriel Krisman Bertazi wrote:
> Christian Brauner <[email protected]> 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.

--
Cheers,
Carlos.

2020-08-02 12:03:24

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] Syscall User Redirection

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

2020-08-04 14:30:50

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] Syscall User Redirection

Pavel Machek <[email protected]> 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.

--
Gabriel Krisman Bertazi