linux-kernel - Re: [PATCH 2/3] x86/entry/64: Use the TSS sp2 slot for rsp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFwOMt7Qw4HFyBiSNpHXoP3kGRsrQhi3LA_=uxwuNz8G0Q@mail.gmail.com>
Date:   Sat, 1 Sep 2018 09:33:35 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Andrew Lutomirski <luto@...nel.org>
Cc:     "the arch/x86 maintainers" <x86@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Joerg Roedel <joro@...tes.org>, Jiri Olsa <jolsa@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 2/3] x86/entry/64: Use the TSS sp2 slot for rsp_scratch

On Fri, Aug 31, 2018 at 3:21 PM Andy Lutomirski <luto@...nel.org> wrote:
>
>  #ifdef CONFIG_X86_64
>  # define cpu_current_top_of_stack (cpu_tss_rw + TSS_sp1)
> +# define rsp_scratch (cpu_tss_rw + TSS_sp2)
>  #endif

Ugh. The above gets used by *assembler* code. I was really confused by how this:

> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -59,8 +59,6 @@
>  #include <asm/unistd_32_ia32.h>
>  #endif
>
> -__visible DEFINE_PER_CPU(unsigned long, rsp_scratch);
> -

could continue to work despite the accesses to "rsp_scratch" still
remaining in the asm files.

Can yu humor me, and just not do something quite that subtle. I must
have missed this the first time around.

Please get rid of the define, and just make the asm code spell out
what it actually does.

We already do that for TSS_sp0 for the normal case:

      movq    PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp

so I think this should just change

-     movq    %rsp, PER_CPU_VAR(rsp_scratch)
+     movq    %rsp, PER_CPU_VAR(cpu_tss_rw + TSS_sp2)

instead of having that subtle rsp_scratch thing.

And honestly, I think we should strive to do the same thing with
cpu_current_top_of_stack. There at least the #define currently makes
sense (because on 32-bit, it's actually a percpu variable, on 64-bit
it's that sp1 field).

But wouldn't it be nice to just unify 32-bit and 64-bit in this
respect, and get rid of that subtle difference?

But regardless of whether we eventually do that kind of unification
change, the cpu_current_top_of_stack #define has a _reason_ for it as
things stand now.

The new rsp_scratch thing does not. Just spell out what you're doing.

                  Linus