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

From: peterz
Date: Wed Sep 16 2020 - 03:28:22 EST


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 intentional abuse, I can
> get both gcc and clang to generate code along these lines:
>
> pushfq
> popq 0x8(%rsp)
> mov 0x8(%rsp),%rax
>
> which is incorrect and will not work as intended.

We need another constraint :-)

> Fix it by removing the memory option. This issue is exacerbated by
> a clang optimization bug:
>
> https://bugs.llvm.org/show_bug.cgi?id=47530
>
> Fixes: e74deb11931f ("x86/uaccess: Introduce user_access_{save,restore}()")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Bill Wendling <morbo@xxxxxxxxxx> # I think
> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
> ---
> arch/x86/include/asm/smap.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/smap.h b/arch/x86/include/asm/smap.h
> index 8b58d6975d5d..be6d675ae3ac 100644
> --- a/arch/x86/include/asm/smap.h
> +++ b/arch/x86/include/asm/smap.h
> @@ -61,7 +61,7 @@ static __always_inline unsigned long smap_save(void)
> ALTERNATIVE("jmp 1f", "", X86_FEATURE_SMAP)
> "pushf; pop %0; " __ASM_CLAC "\n\t"
> "1:"
> - : "=rm" (flags) : : "memory", "cc");
> + : "=r" (flags) : : "memory", "cc");

native_save_fl() has the exact same code; you'll need to fix both.