linux-kernel - Re: [patch 05/12] x86/irq: Provide macro for inlining irq stack switching

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

Message-ID: <CAFULd4agX8oOyoHWVu=HKNcegVJNuv2Z6JKwg4mZqUESSWTuVw@mail.gmail.com>
Date:   Fri, 5 Feb 2021 12:03:44 +0100
From:   Uros Bizjak <ubizjak@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [patch 05/12] x86/irq: Provide macro for inlining irq stack switching

> The effort to make the ASM entry code slim and unified moved the irq stack
> switching out of the low level ASM code so that the whole return from
> interrupt work and state handling can be done in C and the ASM code just
> handles the low level details of entry and exit.
>
> This ended up being a suboptimal implementation for various reasons
> (including tooling). The main pain points are:
>
>  - The indirect call which is expensive thanks to retpoline
>
>  - The inability to stay on the irq stack for softirq processing on return
>    from interrupt
>
>  - The fact that the stack switching code ends up being an easy to target
>    exploit gadget.
>
> Prepare for inlining the stack switching logic into the C entry points by
> providing a ASM macro which contains the guts of the switching mechanism:
>
>   1) Store RSP at the top of the irq stack
>   2) Switch RSP to the irq stack
>   3) Invoke code
>   4) Pop the original RSP back
>
> Document the unholy asm() logic while at it to reduce the amount of head
> scratching required a half year from now.

#define __call_on_irqstack(func, asm_call, constr...) \
+{ \
+ register void *tos asm("r11"); \
+ \
+ tos = ((void *)__this_cpu_read(hardirq_stack_ptr)); \
+ \
+ asm_inline volatile( \
+ "movq %%rsp, (%[__tos]) \n" \
+ "movq %[__tos], %%rsp \n" \
+ \
+ asm_call \
+ \
+ "popq %%rsp \n" \
+ \
+ : "+r" (tos) IRQSTACK_CALL_CONSTRAINT \

Please note that GCC documents "U" register constraint that can be
used here instead of declaring hard register in the variable
declaration:

    'U'
         The call-clobbered integer registers.

+ : [__func] "i" (func), [__tos] "r" (tos) constr \

There is no need to declare "tos" as read operand again, it is already
declared above as readwrite (+) operand.

Considering that (according to the above documentation) it is
necessary to list all input registers that pass function arguments,
the compiler is free to allocate any remaining register from "U"
register class, not only r11. Using an earlyclobber modifier prevents
the compiler from allocating a register that carries input argument,
so:

: [__tos] "+&U" (tos) IRQSTACK_CALL_CONSTRAINT \
: [__func] "i" (func) constr \

could be used.

Also note that functions with variable arguments pass information
about the number of vector registers used in %rax, so %rax should be
listed as input argument in this case. But this should be of no issue
here.

Uros.

+ : "cc", "rax", "rcx", "rdx", "rsi", "rdi", "r8", "r9", "r10", \
+  "memory" \
+ ); \