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] [day] [month] [year] [list]

Message-ID: <CALCETrUvz-VWpLYsYoC-buKMUBPPTHdOj6OeRE7Vv+--eZPKAQ@mail.gmail.com>
Date:   Sat, 1 Sep 2018 10:29:48 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andrew Lutomirski <luto@...nel.org>,
        "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 Sat, Sep 1, 2018 at 9:33 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> 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.

Done for v2.

>
> 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?
>

Yes.  But ugh, the way that thing has worked has changed so many times
on 32-bit and 64-bit that I've lost track a little bit.  I'll put it
on my long list of things to clean up.