linux-kernel - Re: [PATCH] x86/head/64: Avoid Clang < 17 stack protector in startup code

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: <CAMj1kXH=nLb-KTJNz37NAvNHOgx60WRvq+-w-QzmzrtGg1PV8g@mail.gmail.com>
Date: Wed, 12 Mar 2025 12:17:31 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Brian Gerst <brgerst@...il.com>
Cc: Ard Biesheuvel <ardb+git@...gle.com>, linux-kernel@...r.kernel.org, llvm@...ts.linux.dev, 
	x86@...nel.org, Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH] x86/head/64: Avoid Clang < 17 stack protector in startup code

On Wed, 12 Mar 2025 at 12:09, Brian Gerst <brgerst@...il.com> wrote:
>
> On Wed, Mar 12, 2025 at 6:27 AM Ard Biesheuvel <ardb+git@...gle.com> wrote:
> >
> > From: Ard Biesheuvel <ardb@...nel.org>
> >
> > Clang versions before 17 will not honour -fdirect-access-external-data
> > for the load of the stack cookie emitted into each function's prologue
> > and epilogue, and will emit a GOT based reference instead, e.g.,
> >
> >   4c 8b 2d 00 00 00 00    mov    0x0(%rip),%r13
> >           18a: R_X86_64_REX_GOTPCRELX     __ref_stack_chk_guard-0x4
> >   65 49 8b 45 00          mov    %gs:0x0(%r13),%rax
> >
> > This is inefficient, but at least, the linker will usually follow the
> > rules of the x86 psABI, and relax the GOT load into a RIP-relative LEA
> > instruction.  This is still suboptimal, as the per-CPU load could use a
> > RIP-relative reference directly, but at least it gets rid of the first
> > load from memory.
> >
> > However, Boris reports that in some cases, when using distro builds of
> > Clang/LLD 15, the first load gets relaxed into
> >
> >   49 c7 c6 20 c0 55 86  mov    $0xffffffff8655c020,%r14
> >   ffffffff8373bf0f: R_X86_64_32S        __ref_stack_chk_guard
> >   65 49 8b 06           mov    %gs:(%r14),%rax
> >
> > instead, which is fine in principle, as MOV may be cheaper than LEA on
> > some micro-architectures. However, such absolute references assume that
> > the variable in question can be accessed via the kernel virtual mapping,
> > and this is not guaranteed for the startup code residing in .head.text.
> >
> > This is therefore a true positive, that was caught using the recently
> > introduced relocs check for absolute references in the startup code:
> >
> >   Absolute reference to symbol '__ref_stack_chk_guard' not permitted in .head.text
> >
> > Work around the issue by disabling the stack protector in the startup
> > code for Clang versions older than 17.
> >
> > Fixes: 80d47defddc0 ("x86/stackprotector/64: Convert to normal per-CPU variable")
> > Cc: Borislav Petkov <bp@...en8.de>
> > Cc: Brian Gerst <brgerst@...il.com>
> > Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> > ---
> >  arch/x86/include/asm/init.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/init.h b/arch/x86/include/asm/init.h
> > index 0e82ebc5d1e1..8b1b1abcef15 100644
> > --- a/arch/x86/include/asm/init.h
> > +++ b/arch/x86/include/asm/init.h
> > @@ -2,7 +2,11 @@
> >  #ifndef _ASM_X86_INIT_H
> >  #define _ASM_X86_INIT_H
> >
> > +#if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 170000
> > +#define __head __section(".head.text") __no_sanitize_undefined __no_stack_protector
> > +#else
> >  #define __head __section(".head.text") __no_sanitize_undefined
> > +#endif
>
> Just disable it for all __head code.  This runs long before userspace
> can mount a stack smashing attack.
>

Not all of it - some code is emitted into .head.text because it is
called early, but it could still be called much later as well.