Messages in this thread | | | Date | Wed, 13 Feb 2019 23:21:46 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH v3 3/4] uaccess: Check no rescheduling function is called in unsafe region |
| |
On Wed, Feb 13, 2019 at 10:51:24AM -0800, Andy Lutomirski wrote: > > On Feb 13, 2019, at 7:45 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > Which I suppose means that GCC generates the PUSHF/POPF to preserve the > > EFLAGS, since we mark those explicitly clobbered. > > > > Not quite. A flags clobber doesn’t save the control bits like AC > except on certain rather buggy llvm compilers. The change you’re > looking for is: > > http://git.kernel.org/tip/2c7577a7583747c9b71f26dced7f696b739da745
Indeed, failed to find that.
> > For a little bit of context; it turns out that user_access_begin() / > > user_access_end() sets EFLAGS.AC and scheduling in between there wrecks > > that because we're apparently not saving that anymore. > > But only explicit scheduling — preemption and sleepy page faults are > fine because the interrupt frame saves flags.
No, like pointed out elsewhere in this thread, anything that does preempt_disable() is utterly broken with this.
Because at that point the IRQ return path doesn't reschedule but preempt_enable() will, and that doesn't preserve EFLAGS again.
> > Now, I'm tempted to add the PUSHF / POPF right back because of this, but > > first I suppose we need to figure out if that change was on purpose and > > why that went missing from the Changelog. > > That’s certainly the easy solution. Or we could teach the might_sleep > checks about this, but that could be a mess.
That's not enough, we'd have to teach preempt_disable(), but worse, preempt_disable_notrace().
Anything that lands in ftrace, which _will_ use preempt_disable_notrace(), will screw this thing up.
| |