----- On Mar 29, 2018, at 10:23 AM, Peter Zijlstra peterz@xxxxxxxxxxxxx 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 -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
