linux-kernel - Re: gcc-10: kernel stack is corrupted and fails to boot

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20200513233616.GD6733@zn.tnic>
Date:   Thu, 14 May 2020 01:36:16 +0200
From:   Borislav Petkov <bp@...e.de>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Arvind Sankar <nivedita@...m.mit.edu>,
        Kalle Valo <kvalo@...eaurora.org>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: gcc-10: kernel stack is corrupted and fails to boot

On Wed, May 13, 2020 at 04:13:53PM -0700, Linus Torvalds wrote:
> The check itself doesn't seem worth it. If your worry is that an empty
> asm() can be optimized away, then don't use an empty asm!

gcc guys said we should use that since the first attempt using

__attribute__((optimize("-fno-stack-protector")))

didn't work because, well, that attribute turned out to be "not suitable in
production code". :)

Full thread here:

https://lore.kernel.org/lkml/20200314164451.346497-1-slyfox@gentoo.org/

> In other words, the only reason for that check seems to be a worry
> that simply isn't worth having.

Yes, that was me asking for a way to check whether any future gccs would
violate that. But if they'd do that, they would break a lot of code
depending on it.

> In fact, I think the check is wrong anyway, since the main thing I can
> see that would do a tailcall despite the empty asm is link-time
> optimizations that that check doesn't even check for!
> 
> So everything I see there just screams "the check is bogus" to me. The
> check doesn't work, and if it were to work it only means that the
> prevent_tail_call_optimization() thing is too fragile.

So I did test it trivially by removing the asm("") and then it would
tailcall optimize. But we didn't think about LTO so hm, that would
probably break it.

> Just put a full memory barrier in there, with an actual "mfence"
> instruction or whatever, so that you know that the check is pointless,
> and so that you know that a link-time optimizer can't turn the
> call+return into a tailcall.

Right, the intention here was to have it arch-agnostic in
include/linux/compiler.h because powerpc might need it too soon:

arch/powerpc/kernel/smp.c:1296: boot_init_stack_canary();

Looking at them, they do have an mb() too so how about this then
instead?

#define prevent_tail_call_optimization()	mb()

Thx.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg