From: Jann Horn <jannh@google.com> To: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> Cc: Tycho Andersen <tycho@tycho.pizza>, Sargun Dhillon <sargun@sargun.me>, Kees Cook <keescook@chromium.org>, Christian Brauner <christian@brauner.io>, linux-man <linux-man@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>, Aleksa Sarai <cyphar@cyphar.com>, Alexei Starovoitov <ast@kernel.org>, Will Drewry <wad@chromium.org>, bpf <bpf@vger.kernel.org>, Song Liu <songliubraving@fb.com>, Daniel Borkmann <daniel@iogearbox.net>, Andy Lutomirski <luto@amacapital.net>, Linux Containers <containers@lists.linux-foundation.org>, Giuseppe Scrivano <gscrivan@redhat.com>, Robert Sesek <rsesek@google.com> Subject: Re: For review: seccomp_user_notif(2) manual page Date: Mon, 26 Oct 2020 10:32:03 +0100 [thread overview] Message-ID: <CAG48ez2TcWb6SQ86XRJDdN-Ab_gO9-sXgpFnJODMXH60mCkBJQ@mail.gmail.com> (raw) In-Reply-To: <887d5a29-edaa-2761-1512-370c1f5c3a6f@gmail.com> On Sat, Oct 24, 2020 at 2:53 PM Michael Kerrisk (man-pages) <mtk.manpages@gmail.com> wrote: > On 10/17/20 2:25 AM, Jann Horn wrote: > > On Fri, Oct 16, 2020 at 8:29 PM Michael Kerrisk (man-pages) > > <mtk.manpages@gmail.com> wrote: [...] > >> I'm not sure if I should write anything about this small UAPI > >> breakage in BUGS, or not. Your thoughts? > > > > Thinking about it a bit more: Any code that relies on pause() or > > epoll_wait() not restarting is buggy anyway, right? Because a signal > > could also arrive directly before entering the syscall, while > > userspace code is still executing? So one could argue that we're just > > enlarging a preexisting race. (Unless the signal handler checks the > > interrupted register state to figure out whether we already entered > > syscall handling?) > > Yes, that all makes sense. > > > If userspace relies on non-restarting behavior, it should be using > > something like epoll_pwait(). And that stuff only unblocks signals > > after we've already past the seccomp checks on entry. > > Thanks for elaborating that detail, since as soon as you talked > about "enlarging a preexisting race" above, I immediately wondered > sigsuspend(), pselect(), etc. > > (Mind you, I still wonder about the effect on system calls that > are normally nonrestartable because they have timeouts. My > understanding is that the kernel doesn't restart those system > calls because it's impossible for the kernel to restart the call > with the right timeout value. I wonder what happens when those > system calls are restarted in the scenario we're discussing.) Ah, that's an interesting edge case... > Anyway, returning to your point... So, to be clear (and to > quickly remind myself in case I one day reread this thread), > there is not a problem with sigsuspend(), pselect(), ppoll(), > and epoll_pwait() since: > > * Before the syscall, signals are blocked in the target. > * Inside the syscall, signals are still blocked at the time > the check is made for seccomp filters. > * If a seccomp user-space notification event kicks, the target > is put to sleep with the signals still blocked. > * The signal will only get delivered after the supervisor either > triggers a spoofed success/failure return in the target or the > supervisor sends a CONTINUE response to the kernel telling it > to execute the target's system call. Either way, there won't be > any restarting of the target's system call (and the supervisor > thus won't see multiple notifications). > > (Right?) Yeah. [...] > > So we should probably document the restarting behavior as something > > the supervisor has to deal with in the manpage; but for the > > "non-restarting syscalls can restart from the target's perspective" > > aspect, it might be enough to document this as quirky behavior that > > can't actually break correct code? (Or not document it at all. Dunno.) > > So, I've added the following to the page: > > Interaction with SA_RESTART signal handlers > Consider the following scenario: > > · The target process has used sigaction(2) to install a signal > handler with the SA_RESTART flag. > > · The target has made a system call that triggered a seccomp user- > space notification and the target is currently blocked until the > supervisor sends a notification response. > > · A signal is delivered to the target and the signal handler is > executed. > > · When (if) the supervisor attempts to send a notification > response, the SECCOMP_IOCTL_NOTIF_SEND ioctl(2)) operation will > fail with the ENOENT error. > > In this scenario, the kernel will restart the target's system > call. Consequently, the supervisor will receive another user- > space notification. Thus, depending on how many times the blocked > system call is interrupted by a signal handler, the supervisor may > receive multiple notifications for the same system call in the > target. > > One oddity is that system call restarting as described in this > scenario will occur even for the blocking system calls listed in > signal(7) that would never normally be restarted by the SA_RESTART > flag. > > Does that seem okay? Sounds good to me. > In addition, I've queued a cross-reference in signal(7): > > In certain circumstances, the seccomp(2) user-space notifi‐ > cation feature can lead to restarting of system calls that > would otherwise never be restarted by SA_RESTART; for > details, see seccomp_user_notif(2).
next prev parent reply other threads:[~2020-10-26 9:42 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-30 11:07 Michael Kerrisk (man-pages) 2020-09-30 15:03 ` Tycho Andersen 2020-09-30 15:11 ` Tycho Andersen 2020-09-30 20:34 ` Michael Kerrisk (man-pages) 2020-09-30 23:03 ` Tycho Andersen 2020-09-30 23:11 ` Jann Horn 2020-09-30 23:24 ` Tycho Andersen 2020-10-01 1:52 ` Jann Horn 2020-10-01 2:14 ` Jann Horn 2020-10-25 16:31 ` Michael Kerrisk (man-pages) 2020-10-26 15:54 ` Jann Horn 2020-10-27 6:14 ` Michael Kerrisk (man-pages) 2020-10-27 10:28 ` Jann Horn 2020-10-28 6:31 ` Sargun Dhillon 2020-10-28 9:43 ` Jann Horn 2020-10-28 17:43 ` Sargun Dhillon 2020-10-28 18:20 ` Jann Horn 2020-10-01 7:49 ` Michael Kerrisk (man-pages) 2020-10-26 0:32 ` Kees Cook 2020-10-26 9:51 ` Jann Horn 2020-10-26 10:31 ` Jann Horn 2020-10-28 22:56 ` Kees Cook 2020-10-29 1:11 ` Jann Horn [not found] ` <20201029021348.GB25673@cisco> 2020-10-29 4:26 ` Jann Horn 2020-10-28 22:53 ` Kees Cook 2020-10-29 1:25 ` Jann Horn 2020-10-01 7:45 ` Michael Kerrisk (man-pages) 2020-10-14 4:40 ` Michael Kerrisk (man-pages) 2020-09-30 15:53 ` Jann Horn 2020-10-01 12:54 ` Christian Brauner 2020-10-01 15:47 ` Jann Horn 2020-10-01 16:58 ` Tycho Andersen 2020-10-01 17:12 ` Christian Brauner 2020-10-14 5:41 ` Michael Kerrisk (man-pages) 2020-10-01 18:18 ` Jann Horn 2020-10-01 18:56 ` Tycho Andersen 2020-10-01 17:05 ` Christian Brauner 2020-10-15 11:24 ` Michael Kerrisk (man-pages) 2020-10-15 20:32 ` Jann Horn 2020-10-16 18:29 ` Michael Kerrisk (man-pages) 2020-10-17 0:25 ` Jann Horn 2020-10-24 12:52 ` Michael Kerrisk (man-pages) 2020-10-26 9:32 ` Jann Horn [this message] 2020-10-26 9:47 ` Michael Kerrisk (man-pages) 2020-09-30 23:39 ` Kees Cook 2020-10-15 11:24 ` Michael Kerrisk (man-pages) 2020-10-26 0:19 ` Kees Cook 2020-10-26 9:39 ` Michael Kerrisk (man-pages) 2020-10-01 12:36 ` Christian Brauner 2020-10-15 11:23 ` Michael Kerrisk (man-pages) 2020-10-01 21:06 ` Sargun Dhillon 2020-10-01 23:19 ` Tycho Andersen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAG48ez2TcWb6SQ86XRJDdN-Ab_gO9-sXgpFnJODMXH60mCkBJQ@mail.gmail.com \ --to=jannh@google.com \ --cc=ast@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=christian@brauner.io \ --cc=containers@lists.linux-foundation.org \ --cc=cyphar@cyphar.com \ --cc=daniel@iogearbox.net \ --cc=gscrivan@redhat.com \ --cc=keescook@chromium.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-man@vger.kernel.org \ --cc=luto@amacapital.net \ --cc=mtk.manpages@gmail.com \ --cc=rsesek@google.com \ --cc=sargun@sargun.me \ --cc=songliubraving@fb.com \ --cc=tycho@tycho.pizza \ --cc=wad@chromium.org \ --subject='Re: For review: seccomp_user_notif(2) manual page' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).