linux-kernel - RE: [PATCH] Kconfig: Explicitly disable asm goto w/ outputs on gcc-11 (and earlier)

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: 
 <DM6PR02MB40580E317C24E87B319CD748B84B2@DM6PR02MB4058.namprd02.prod.outlook.com>
Date: Fri, 9 Feb 2024 19:26:43 +0000
From: "Andrew Pinski (QUIC)" <quic_apinski@...cinc.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
        Sean Christopherson
	<seanjc@...gle.com>
CC: "Andrew Pinski (QUIC)" <quic_apinski@...cinc.com>,
        Nick Desaulniers
	<ndesaulniers@...gle.com>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Peter
 Zijlstra <peterz@...radead.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>
Subject: RE: [PATCH] Kconfig: Explicitly disable asm goto w/ outputs on gcc-11
 (and earlier)

> -----Original Message-----
> From: Linus Torvalds <torvalds@...ux-foundation.org>
> Sent: Friday, February 9, 2024 10:56 AM
> To: Sean Christopherson <seanjc@...gle.com>
> Cc: Andrew Pinski (QUIC) <quic_apinski@...cinc.com>; Nick Desaulniers
> <ndesaulniers@...gle.com>; linux-kernel@...r.kernel.org; Masahiro Yamada
> <masahiroy@...nel.org>; Peter Zijlstra <peterz@...radead.org>;
> kvm@...r.kernel.org
> Subject: Re: [PATCH] Kconfig: Explicitly disable asm goto w/ outputs on gcc-11
> (and earlier)
> 
> On Fri, 9 Feb 2024 at 10:43, Sean Christopherson <seanjc@...gle.com>
> wrote:
> >
> > > We've removed that workaround in commit 43c249ea0b1e ("compiler-
> gcc.h:
> > > remove ancient workaround for gcc PR 58670"), I'm wondering if maybe
> > > that removal was a bit optimistic.
> >
> > FWIW, reverting that does restore correct behavior on gcc-11.
> 
> Hmm. I suspect we'll just have to revert that commit then - although probably
> in some form where it only affects the "this has outputs"
> case.
> 
> Which is much less common than the non-output "asm goto" case.
> 
> It does cause gcc to generate fairly horrific code (as noted in the commit), but
> it's almost certainly still better code than what the non-asm-goto case results
> in.
> 
> We have very few uses of CC_HAS_ASM_GOTO_OUTPUT (and the related
> CC_HAS_ASM_GOTO_TIED_OUTPUT), but unsafe_get_user() in particular
> generates horrid code without it.
> 
> But it would be really good to understand *what* that secondary bug is, and
> the fix for it. Just to make sure that gcc is really fixed, and there isn't some
> pending bug that we just keep hiding with that extra empty volatile asm.
> 
> Andrew?
> 

Let me first state inline-asm without output has always been volatile; `asm goto` or not (this has been true since 2.95.3 or even before).
The issue is with an output inline-asm is no longer volatile. But the documentation of GCC stated all `asm goto` was volatile. 
So the side effect of making all `asm goto` as volatile does have an effect on the ones without an output since they were already treated internally and treated similarly to the inline-asm without an output.
The fix that was done to GCC was now treat all `asm goto` as volatile instead of just the ones without output.
So basically what would happen is internally GCC would remove the `asm goto` with an output if the output was unused. This would cause havoc in the internal state of GCC because the CFG would not be up to date to what the rest of the IR was. 

Oh and there was a bug in GCC 12.1.0-12.3.0, and GCC 13.1.0 releases which might be the reason why folks are hitting issues even with the volatile added back, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110420 was the fix there. 

Note there was/is a new different  for `asm goto` was fixed in the past few weeks where some of the optimizations was not ready to handle them (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110422); this was just fixed on all of the open release branches but NOT in any release yet; it will be in 11.5.0, 12.4.0, 13.3.0 and 14.1.0.

>                    Linus