From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> To: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, Boqun Feng <boqun.feng@gmail.com>, Andy Lutomirski <luto@amacapital.net>, Dave Watson <davejwatson@fb.com>, linux-kernel <linux-kernel@vger.kernel.org>, linux-api <linux-api@vger.kernel.org>, Paul Turner <pjt@google.com>, Andrew Morton <akpm@linux-foundation.org>, Russell King <linux@arm.linux.org.uk>, Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>, Andrew Hunter <ahh@google.com>, Andi Kleen <andi@firstfloor.org>, Chris Lameter <cl@linux.com>, Ben Maurer <bmaurer@fb.com>, rostedt <rostedt@goodmis.org>, Josh Triplett <josh@joshtriplett.org>, Linus Torvalds <torvalds@linux-foundation.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, Michael Kerrisk <mtk.manpages@gmail.com>, Alexander Viro <viro@zeniv.linux.org.uk> Subject: Re: [RFC PATCH for 4.17 02/21] rseq: Introduce restartable sequences system call (v12) Date: Thu, 29 Mar 2018 11:39:00 -0400 (EDT) [thread overview] Message-ID: <544124089.623.1522337940950.JavaMail.zimbra@efficios.com> (raw) In-Reply-To: <20180329142338.GD4043@hirez.programming.kicks-ass.net> ----- On Mar 29, 2018, at 10:23 AM, Peter Zijlstra peterz@infradead.org wrote: > On Thu, Mar 29, 2018 at 09:54:01AM -0400, Mathieu Desnoyers wrote: >> Let's say we disallow system calls from rseq critical sections. A few points >> arise: >> >> - We still need to allow traps (page faults, breakpoints, ...) within rseq c.s., >> >> - We still need to allow interrupts within rseq c.s., > > Sure, but all those are different entry points, so that shouldn't be a > problem. Yes, indeed. > >> - We need to decide whether we just document that syscalls within rseq c.s. >> are not supported, or we enforce a behavior if this happens (e.g. SIGSEGV). >> If we enforce a SIGSEGV, we'd have to figure out whether it's worth it to >> add extra branches to the system call fast path to validate this. > > Without enforcement someone will eventually do this :/ We might (maybe) > get away with it being a debug option somewhere, but even that sounds > like trouble. I find it unlikely that someone will issue a syscall from a rseq critical section without really intending it. The system call would need to be crafted within the rseq assembly block. Enforcing SIGSEGV on syscall entry when nested in a rseq critical section will not be free both in terms of syscall overhead, and in terms of code maintenance: we'd need to add those checks into entry.S for each architecture supported, which pretty much doubles the amount of architecture-specific code we need to implement for rseq. Currently, all we need is to hook in signal delivery and wire up the system call numbers. If there is some clever arch-agnostic way to enforce SIGSEGV in those situations, I'm all ears. But I don't think it's worthwhile to enforce this if it costs in terms of system call speed and adds extra arch-specific code to maintain. We could simply document that issuing a system call within a rseq critical section will cause the restart behavior (whether the critical section is restarted or not) to be undefined. > >> - We need to carefully consider the case of system calls issued within signal >> handlers nested on top of rseq. When RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL is >> _not_ set, neither in the rseq c.s. descriptor nor in the TLS @flags, >> it's pretty much straightforward: upon signal delivery, the kernel moves the >> ip to abort, and clears the tls @rseq_cs pointer. This means that any system >> call issued within the signal handler is not actually within the rseq c.s. >> upon which the signal is nested. >> >> The case I worry about is if a thread sets the RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL >> flag in its TLS @flags field (useful in a debugging scenario where we want a >> debugger to single-step through the rseq c.s. and observe registers at each >> step). >> Arguably, this is only ever used in development. However, it does allow a >> situation >> where a system call executed within a signal handler can nest over a rseq c.s.. >> So if we choose to be very strict and SIGSEGV any syscall nested over rseq >> c.s., we may very well end up killing the process for no good reason in this >> scenario. > > Yes, that needs a little thought; but when we run the signal handler, > the IP would no longer be inside the active RSEQ, right? Good point, I missed that. So yes, even with the RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL flag set, the instruction pointer comparison would detect that we're not actually running in the rseq critical section if a syscall is issued from the signal handler. > >> - We need to decide whether all syscalls are disallowed, or if we want to pick >> specific ones (e.g. fork()). > > All. I'm fine with that. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> To: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, Boqun Feng <boqun.feng@gmail.com>, Andy Lutomirski <luto@amacapital.net>, Dave Watson <davejwatson@fb.com>, linux-kernel <linux-kernel@vger.kernel.org>, linux-api <linux-api@vger.kernel.org>, Paul Turner <pjt@google.com>, Andrew Morton <akpm@linux-foundation.org>, Russell King <linux@arm.linux.org.uk>, Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>, Andrew Hunter <ahh@google.com>, Andi Kleen <andi@firstfloor.org>, Chris Lameter <cl@linux.com>, Ben Maurer <bmaurer@fb.com>, rostedt <rostedt@goodmis.org>, Josh Triplett <josh@joshtriplett.org>, Linus Torvalds <torvalds@linux-foundation.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will> Subject: Re: [RFC PATCH for 4.17 02/21] rseq: Introduce restartable sequences system call (v12) Date: Thu, 29 Mar 2018 11:39:00 -0400 (EDT) [thread overview] Message-ID: <544124089.623.1522337940950.JavaMail.zimbra@efficios.com> (raw) In-Reply-To: <20180329142338.GD4043@hirez.programming.kicks-ass.net> ----- On Mar 29, 2018, at 10:23 AM, Peter Zijlstra peterz@infradead.org wrote: > On Thu, Mar 29, 2018 at 09:54:01AM -0400, Mathieu Desnoyers wrote: >> Let's say we disallow system calls from rseq critical sections. A few points >> arise: >> >> - We still need to allow traps (page faults, breakpoints, ...) within rseq c.s., >> >> - We still need to allow interrupts within rseq c.s., > > Sure, but all those are different entry points, so that shouldn't be a > problem. Yes, indeed. > >> - We need to decide whether we just document that syscalls within rseq c.s. >> are not supported, or we enforce a behavior if this happens (e.g. SIGSEGV). >> If we enforce a SIGSEGV, we'd have to figure out whether it's worth it to >> add extra branches to the system call fast path to validate this. > > Without enforcement someone will eventually do this :/ We might (maybe) > get away with it being a debug option somewhere, but even that sounds > like trouble. I find it unlikely that someone will issue a syscall from a rseq critical section without really intending it. The system call would need to be crafted within the rseq assembly block. Enforcing SIGSEGV on syscall entry when nested in a rseq critical section will not be free both in terms of syscall overhead, and in terms of code maintenance: we'd need to add those checks into entry.S for each architecture supported, which pretty much doubles the amount of architecture-specific code we need to implement for rseq. Currently, all we need is to hook in signal delivery and wire up the system call numbers. If there is some clever arch-agnostic way to enforce SIGSEGV in those situations, I'm all ears. But I don't think it's worthwhile to enforce this if it costs in terms of system call speed and adds extra arch-specific code to maintain. We could simply document that issuing a system call within a rseq critical section will cause the restart behavior (whether the critical section is restarted or not) to be undefined. > >> - We need to carefully consider the case of system calls issued within signal >> handlers nested on top of rseq. When RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL is >> _not_ set, neither in the rseq c.s. descriptor nor in the TLS @flags, >> it's pretty much straightforward: upon signal delivery, the kernel moves the >> ip to abort, and clears the tls @rseq_cs pointer. This means that any system >> call issued within the signal handler is not actually within the rseq c.s. >> upon which the signal is nested. >> >> The case I worry about is if a thread sets the RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL >> flag in its TLS @flags field (useful in a debugging scenario where we want a >> debugger to single-step through the rseq c.s. and observe registers at each >> step). >> Arguably, this is only ever used in development. However, it does allow a >> situation >> where a system call executed within a signal handler can nest over a rseq c.s.. >> So if we choose to be very strict and SIGSEGV any syscall nested over rseq >> c.s., we may very well end up killing the process for no good reason in this >> scenario. > > Yes, that needs a little thought; but when we run the signal handler, > the IP would no longer be inside the active RSEQ, right? Good point, I missed that. So yes, even with the RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL flag set, the instruction pointer comparison would detect that we're not actually running in the rseq critical section if a syscall is issued from the signal handler. > >> - We need to decide whether all syscalls are disallowed, or if we want to pick >> specific ones (e.g. fork()). > > All. I'm fine with that. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
next prev parent reply other threads:[~2018-03-29 15:39 UTC|newest] Thread overview: 123+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-27 16:05 [RFC PATCH for 4.17 00/21] Restartable sequences and CPU op vector Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 01/21] uapi headers: Provide types_32_64.h Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 02/21] rseq: Introduce restartable sequences system call (v12) Mathieu Desnoyers 2018-03-28 6:47 ` Boqun Feng 2018-03-28 6:47 ` Boqun Feng 2018-03-28 14:06 ` Mathieu Desnoyers 2018-03-28 14:06 ` Mathieu Desnoyers 2018-03-28 14:31 ` Mathieu Desnoyers 2018-03-28 14:31 ` Mathieu Desnoyers 2018-03-28 11:19 ` Peter Zijlstra 2018-03-28 11:19 ` Peter Zijlstra 2018-03-28 14:19 ` Mathieu Desnoyers 2018-03-28 14:19 ` Mathieu Desnoyers 2018-03-28 11:22 ` Peter Zijlstra 2018-03-28 11:22 ` Peter Zijlstra 2018-03-28 14:26 ` Mathieu Desnoyers 2018-03-28 14:26 ` Mathieu Desnoyers 2018-03-28 12:29 ` Peter Zijlstra 2018-03-28 12:29 ` Peter Zijlstra 2018-03-28 12:52 ` Peter Zijlstra 2018-03-28 12:52 ` Peter Zijlstra 2018-03-28 15:03 ` Mathieu Desnoyers 2018-03-28 15:03 ` Mathieu Desnoyers 2018-03-28 16:19 ` Mathieu Desnoyers 2018-03-28 16:19 ` Mathieu Desnoyers 2018-03-28 12:50 ` Peter Zijlstra 2018-03-28 12:50 ` Peter Zijlstra 2018-03-28 14:47 ` Mathieu Desnoyers 2018-03-28 14:47 ` Mathieu Desnoyers 2018-03-28 14:59 ` Peter Zijlstra 2018-03-28 14:59 ` Peter Zijlstra 2018-03-28 15:14 ` Mathieu Desnoyers 2018-03-28 15:14 ` Mathieu Desnoyers 2018-03-28 15:28 ` Peter Zijlstra 2018-03-28 15:28 ` Peter Zijlstra 2018-03-28 15:37 ` Mathieu Desnoyers 2018-03-28 15:37 ` Mathieu Desnoyers 2018-03-28 17:49 ` Peter Zijlstra 2018-03-28 17:49 ` Peter Zijlstra 2018-03-28 20:19 ` Mathieu Desnoyers 2018-03-28 20:19 ` Mathieu Desnoyers 2018-03-28 21:25 ` Thomas Gleixner 2018-03-28 21:25 ` Thomas Gleixner 2018-03-29 13:54 ` Mathieu Desnoyers 2018-03-29 13:54 ` Mathieu Desnoyers 2018-03-29 14:23 ` Peter Zijlstra 2018-03-29 14:23 ` Peter Zijlstra 2018-03-29 15:39 ` Mathieu Desnoyers [this message] 2018-03-29 15:39 ` Mathieu Desnoyers 2018-03-29 16:24 ` Steven Rostedt 2018-03-29 16:24 ` Steven Rostedt 2018-03-29 18:02 ` Mathieu Desnoyers 2018-03-29 18:02 ` Mathieu Desnoyers 2018-03-29 18:07 ` Steven Rostedt 2018-03-29 18:07 ` Steven Rostedt 2018-03-29 18:35 ` Mathieu Desnoyers 2018-03-29 18:35 ` Mathieu Desnoyers 2018-03-29 18:46 ` Steven Rostedt 2018-03-29 18:46 ` Steven Rostedt 2018-03-29 18:47 ` Steven Rostedt 2018-03-29 18:47 ` Steven Rostedt 2018-04-01 16:13 ` Alan Cox 2018-04-01 16:13 ` Alan Cox 2018-04-02 15:03 ` Christopher Lameter 2018-04-02 15:03 ` Christopher Lameter 2018-04-02 15:27 ` Paul E. McKenney 2018-04-02 15:27 ` Paul E. McKenney 2018-04-02 15:33 ` Mathieu Desnoyers 2018-04-02 15:33 ` Mathieu Desnoyers 2018-04-03 16:36 ` Mathieu Desnoyers 2018-04-03 16:36 ` Mathieu Desnoyers 2018-04-03 20:32 ` Mathieu Desnoyers 2018-04-03 20:32 ` Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 03/21] arm: Add restartable sequences support Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 04/21] arm: Wire up restartable sequences system call Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 05/21] x86: Add support for restartable sequences Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 06/21] x86: Wire up restartable sequence system call Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 07/21] powerpc: Add support for restartable sequences Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 08/21] powerpc: Wire up restartable sequences system call Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 09/21] sched: Implement push_task_to_cpu (v2) Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 10/21] cpu_opv: Provide cpu_opv system call (v6) Mathieu Desnoyers 2018-03-28 15:22 ` Peter Zijlstra 2018-03-28 15:22 ` Peter Zijlstra 2018-03-28 17:54 ` Mathieu Desnoyers 2018-03-28 17:54 ` Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 11/21] x86: Wire up cpu_opv system call Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 12/21] powerpc: " Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 13/21] arm: " Mathieu Desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 14/21] selftests: lib.mk: Introduce OVERRIDE_TARGETS Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` mathieu.desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 15/21] cpu_opv: selftests: Implement selftests (v7) Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` mathieu.desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 16/21] rseq: selftests: Provide rseq library (v5) Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` mathieu.desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 17/21] rseq: selftests: Provide percpu_op API Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` mathieu.desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 18/21] rseq: selftests: Provide basic test Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` mathieu.desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 19/21] rseq: selftests: Provide basic percpu ops test Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` mathieu.desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 20/21] rseq: selftests: Provide parametrized tests Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` mathieu.desnoyers 2018-03-27 16:05 ` [RFC PATCH for 4.17 21/21] rseq: selftests: Provide Makefile, scripts, gitignore Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` Mathieu Desnoyers 2018-03-27 16:05 ` mathieu.desnoyers 2018-03-27 19:09 ` [RFC PATCH for 4.17 00/21] Restartable sequences and CPU op vector Peter Zijlstra 2018-03-27 19:09 ` Peter Zijlstra
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=544124089.623.1522337940950.JavaMail.zimbra@efficios.com \ --to=mathieu.desnoyers@efficios.com \ --cc=ahh@google.com \ --cc=akpm@linux-foundation.org \ --cc=andi@firstfloor.org \ --cc=bmaurer@fb.com \ --cc=boqun.feng@gmail.com \ --cc=catalin.marinas@arm.com \ --cc=cl@linux.com \ --cc=davejwatson@fb.com \ --cc=hpa@zytor.com \ --cc=josh@joshtriplett.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=luto@amacapital.net \ --cc=mingo@redhat.com \ --cc=mtk.manpages@gmail.com \ --cc=paulmck@linux.vnet.ibm.com \ --cc=peterz@infradead.org \ --cc=pjt@google.com \ --cc=rostedt@goodmis.org \ --cc=tglx@linutronix.de \ --cc=torvalds@linux-foundation.org \ --cc=viro@zeniv.linux.org.uk \ --cc=will.deacon@arm.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.