Messages in this thread | | | From | Linus Torvalds <> | Date | Wed, 13 Feb 2019 15:19:10 -0800 | Subject | Re: [PATCH v3 3/4] uaccess: Check no rescheduling function is called in unsafe region |
| |
On Wed, Feb 13, 2019 at 7:45 AM Peter Zijlstra <peterz@infradead.org> wrote: > > Before that, x86_64 switch_to() read like (much simplified): > > asm volatile ( /* do RSP twiddle */ > : /* output */ > : /* input */ > : "memory", "cc", .... "flags"); > > (see __EXTRA_CLOBBER) > > Which I suppose means that GCC generates the PUSHF/POPF to preserve the > EFLAGS, since we mark those explicitly clobbered.
No, it only means that gcc won't keep conditionals in the flags over the asm. It doesn't make gcc save anything.
The push/pop got removed elsewhere as Andy says.
That said, I do agree that it's probably a good idea to save/restore flags anyway when scheduling. It's not just AC, actually, now that I look at it again I worry a bit about DF too.
We have the rule that we run with DF clear in the kernel, and all the kernel entry points do clear it properly (so that memcpy etc don't need to). But there are a few places that set DF temporarily because they do something odd (backwards memmove), and those atcually have the *exact* same issue as stac/clac has: it's ok to take a trap or interrupt, and schedule due to that (because the trap/irq will clear DF), but it would be a horrible bug to have a synchronous scheduling point there.
Arguably the DF issue really isn't even remotely likely to actually be a real issue (the code that sets DF really is very special and should never do any kind of preemption), but it's conceptually quite similar..
Of course, if DF is ever set, and we end up calling any C code at all, I guess it would already be a huge problem. If the C code then does memcpy or something, it would corrupt things quite badly.
So I guess save/restore isn't going to save us wrt DF. If we get anywhere close to the scheduler with the DF bit set, we've already lost.
But I still do kind of prefer saving flags. We have other sticky state in there too, although none of it matters in the kernel currently (eg iopl etc - only matters in user space, and user space will always reload eflags on return).
Linus
| |