Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-17 Thread Andy Lutomirski
On Thu, Sep 17, 2020 at 7:39 AM Borislav Petkov wrote: > > On Thu, Sep 17, 2020 at 02:25:50PM +, David Laight wrote: > > I actually wonder if there is any code that really benefits from > > the red-zone. > > The kernel has been without a red zone since 2002 at least: > > commit 47f16da277d10

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-17 Thread Borislav Petkov
On Thu, Sep 17, 2020 at 02:25:50PM +, David Laight wrote: > I actually wonder if there is any code that really benefits from > the red-zone. The kernel has been without a red zone since 2002 at least: commit 47f16da277d10ef9494f3e9da2a9113bb22bcd75 Author: Andi Kleen Date: Tue Feb 12

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-17 Thread Borislav Petkov
On Thu, Sep 17, 2020 at 10:05:37AM +, David Laight wrote: > The 'red-zone' allows leaf function to use stack memory for locals > that is below (ie the wrong side of) the stack pointer. After looking at "Figure 3.3: Stack Frame with Base Pointer" in the x86-64 ABI doc, you're probably right:

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-16 Thread Borislav Petkov
On Wed, Sep 16, 2020 at 11:48:42PM +0100, Andrew Cooper wrote: > Every day is a school day. Tell me about it... > This is very definitely one to be filed in obscure x86 corner cases. > > The code snippet above is actually wrong for the kernel, as it uses one > slot of the red-zone.  Recompiling

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-16 Thread Borislav Petkov
On Wed, Sep 16, 2020 at 12:40:30AM +0100, Andrew Cooper wrote: > It's worse than that.  Even when stating that %rsp is modified in the > asm, the generated code sequence is still buggy, for recent Clang and GCC. > > https://godbolt.org/z/ccz9v7 > > It's clearly not safe to ever use memory operand

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-16 Thread peterz
On Tue, Sep 15, 2020 at 01:55:58PM -0700, Andy Lutomirski wrote: > The old smap_save() code was: > > pushf > pop %0 > > with %0 defined by an "=rm" constraint. This is fine if the > compiler picked the register option, but it was incorrect with an > %rsp-relative memory operand. With some i

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-15 Thread Bill Wendling
On Tue, Sep 15, 2020 at 4:40 PM Andrew Cooper wrote: > > On 16/09/2020 00:11, Andy Lutomirski wrote: > >> On Sep 15, 2020, at 2:24 PM, Nick Desaulniers > >> wrote: > >> > >> On Tue, Sep 15, 2020 at 1:56 PM Andy Lutomirski wrote: > >>> The old smap_save() code was: > >>> > >>> pushf > >>> pop

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-15 Thread Andy Lutomirski
> On Sep 15, 2020, at 2:24 PM, Nick Desaulniers wrote: > > On Tue, Sep 15, 2020 at 1:56 PM Andy Lutomirski wrote: >> >> The old smap_save() code was: >> >> pushf >> pop %0 >> >> with %0 defined by an "=rm" constraint. This is fine if the >> compiler picked the register option, but it w

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-15 Thread Bill Wendling
On Tue, Sep 15, 2020 at 2:24 PM Nick Desaulniers wrote: > On Tue, Sep 15, 2020 at 1:56 PM Andy Lutomirski wrote: > > Fixes: e74deb11931f ("x86/uaccess: Introduce user_access_{save,restore}()") > > Cc: sta...@vger.kernel.org > > Reported-by: Bill Wendling # I think > > LOL, yes, the comment can b

Re: [PATCH] x86/smap: Fix the smap_save() asm

2020-09-15 Thread Nick Desaulniers
On Tue, Sep 15, 2020 at 1:56 PM Andy Lutomirski wrote: > > The old smap_save() code was: > > pushf > pop %0 > > with %0 defined by an "=rm" constraint. This is fine if the > compiler picked the register option, but it was incorrect with an > %rsp-relative memory operand. It is incorrect beca

[PATCH] x86/smap: Fix the smap_save() asm

2020-09-15 Thread Andy Lutomirski
The old smap_save() code was: pushf pop %0 with %0 defined by an "=rm" constraint. This is fine if the compiler picked the register option, but it was incorrect with an %rsp-relative memory operand. With some intentional abuse, I can get both gcc and clang to generate code along these lines