[RFC] x86: Disable traditional FPU instructions too

[RFC] x86: Disable traditional FPU instructions too

[Borislav Petkov]

Hey guys,

should we do the below? It looks like we don't disable the generation of
*all* FPU instructions on x86_64 (commit message below has the rationale
why).

We do -msoft-float on 32-bit only and Micha says that -msoft-float and
-mno-80387 are the same and the gcc manpage says:

           On machines where a function returns floating-point results in the 80387
           register stack, some floating-point opcodes may be emitted even if
           -msoft-float is used.

and right after, it has also

       -mno-fp-ret-in-387
           Do not use the FPU registers for return values of functions.

           The usual calling convention has functions return values of types "float"
           and "double" in an FPU register, even if there is no FPU.  The idea is that
           the operating system should emulate an FPU.

           The option -mno-fp-ret-in-387 causes such values to be returned in ordinary
           CPU registers instead.

Btw, there's this -mno-fp-regs switch too which forces passing of FP
results of functions in integer registers...

So maybe we should do this:

---
From: Borislav Petkov <bp@suse.de>
Subject: [PATCH] x86: Disable generation of traditional x87 instructions

We recently had the case where wrongly used constant caused the
generation of x87 instructions in kernel code unknowingly, wreaking all
kinds of havoc. Disable the generation of those too. This will save
people a lot of time when trying to debug such issues by erroring out
of the build and not manifesting itself in all kinds of spectacular and
funny ways at runtime.

Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Michael Matz <matz@suse.de>
Cc: Michal Hocko <mhocko@suse.cz>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 13b22e0f681d..dbd31182669c 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -61,7 +61,7 @@ else
         KBUILD_CFLAGS += -m64
 
         # Don't autogenerate MMX or SSE instructions
-        KBUILD_CFLAGS += -mno-mmx -mno-sse
+        KBUILD_CFLAGS += -mno-mmx -mno-sse -mno-80387 -mno-fp-ret-in-387
 
 	# Use -mpreferred-stack-boundary=3 if supported.
 	KBUILD_CFLAGS += $(call cc-option,-mpreferred-stack-boundary=3)
-- 
1.8.5.2.192.g7794a68


-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [RFC] x86: Disable traditional FPU instructions too

[H. Peter Anvin]

On 01/27/2014 08:37 AM, Borislav Petkov wrote:
> Hey guys,
> 
> should we do the below? It looks like we don't disable the generation of
> *all* FPU instructions on x86_64 (commit message below has the rationale
> why).
> 
> We do -msoft-float on 32-bit only and Micha says that -msoft-float and
> -mno-80387 are the same and the gcc manpage says:
> 
>            On machines where a function returns floating-point results in the 80387
>            register stack, some floating-point opcodes may be emitted even if
>            -msoft-float is used.
> 
> and right after, it has also
> 
>        -mno-fp-ret-in-387
>            Do not use the FPU registers for return values of functions.
> 
>            The usual calling convention has functions return values of types "float"
>            and "double" in an FPU register, even if there is no FPU.  The idea is that
>            the operating system should emulate an FPU.
> 
>            The option -mno-fp-ret-in-387 causes such values to be returned in ordinary
>            CPU registers instead.
> 
> Btw, there's this -mno-fp-regs switch too which forces passing of FP
> results of functions in integer registers...
> 

I don't think it'd hurt... although I think the above pretty much
requires that the code contain actual floating-point types to ever be
generated.  The issue with MMX/SSE is that an autovectorizing compiler
could decide to use them for *integer* code.

-mno-fp-ret-in-387 in particular will only ever apply if a function
return type is a floating-point type.

	-hpa

Re: [RFC] x86: Disable traditional FPU instructions too

[Jiri Kosina]

On Mon, 27 Jan 2014, H. Peter Anvin wrote:

> I don't think it'd hurt... although I think the above pretty much
> requires that the code contain actual floating-point types to ever be
> generated.  

Unfortunately using float-typed constants (such as 1EexpL) are enough for 
this to happen.

-- 
Jiri Kosina
SUSE Labs

Re: [RFC] x86: Disable traditional FPU instructions too

[Andi Kleen]

Jiri Kosina <jkosina@suse.cz> writes:

> On Mon, 27 Jan 2014, H. Peter Anvin wrote:
>
>> I don't think it'd hurt... although I think the above pretty much
>> requires that the code contain actual floating-point types to ever be
>> generated.  
>
> Unfortunately using float-typed constants (such as 1EexpL) are enough for 
> this to happen.

checkpatch would seem to be the right place to check for this?

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [RFC] x86: Disable traditional FPU instructions too

[H. Peter Anvin]

On 01/27/2014 09:01 AM, Andi Kleen wrote:
> Jiri Kosina <jkosina@suse.cz> writes:
> 
>> On Mon, 27 Jan 2014, H. Peter Anvin wrote:
>>
>>> I don't think it'd hurt... although I think the above pretty much
>>> requires that the code contain actual floating-point types to ever be
>>> generated.  
>>
>> Unfortunately using float-typed constants (such as 1EexpL) are enough for 
>> this to happen.
> 
> checkpatch would seem to be the right place to check for this?
> 

Ideally gcc should have a -no-float option...

	-hpa

Re: [RFC] x86: Disable traditional FPU instructions too

[Borislav Petkov]

On Mon, Jan 27, 2014 at 09:16:20AM -0800, H. Peter Anvin wrote:
> > checkpatch would seem to be the right place to check for this?

checkpatch is not mandatory.

> Ideally gcc should have a -no-float option...

Yeah.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [RFC] x86: Disable traditional FPU instructions too

[Borislav Petkov]

On Mon, Jan 27, 2014 at 08:41:09AM -0800, H. Peter Anvin wrote:
> I don't think it'd hurt... although I think the above pretty much
> requires that the code contain actual floating-point types to ever be
> generated.

Yes, that is supposed to catch usage of the 'E' floating constant, for
example, in definitions. It is a build-time assertion, if you want. :)

> The issue with MMX/SSE is that an autovectorizing compiler could
> decide to use them for *integer* code.

Right, and we don't want the FPU state handling if it can be helped.

> -mno-fp-ret-in-387 in particular will only ever apply if a function
> return type is a floating-point type.

Yeah, this is for when using -msoft-float == -mno-80387 which still can
generate FPU insns according to the manpage:

           On machines where a function returns floating-point results in the 80387
           register stack, some floating-point opcodes may be emitted even if
           -msoft-float is used.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [RFC] x86: Disable traditional FPU instructions too

[Michal Hocko]

[CCing Ralf for MIPS.]

On Mon 27-01-14 17:45:44, Borislav Petkov wrote:
> On Mon, Jan 27, 2014 at 08:41:09AM -0800, H. Peter Anvin wrote:
> > I don't think it'd hurt... although I think the above pretty much
> > requires that the code contain actual floating-point types to ever be
> > generated.
> 
> Yes, that is supposed to catch usage of the 'E' floating constant, for
> example, in definitions. It is a build-time assertion, if you want. :)

And git grep says that mips abuses this notation:
arch/mips/include/asm/kvm_host.h:#define MS_TO_NS(x) (x * 1E6L)

I do not have cross-compiler set up to check whether this is a real
issue because include/media/rc-core.h defines the same macro correctly
so the real code might end up using the right variant.
-- 
Michal Hocko
SUSE Labs

Re: [RFC] x86: Disable traditional FPU instructions too

[H. Peter Anvin]

If that is intentional it needs a big huge freaking comment recording why it is not a bug.  I would expect that to generate a FP multiply.

On January 28, 2014 1:37:01 AM PST, Michal Hocko <mhocko@suse.cz> wrote:
>[CCing Ralf for MIPS.]
>
>On Mon 27-01-14 17:45:44, Borislav Petkov wrote:
>> On Mon, Jan 27, 2014 at 08:41:09AM -0800, H. Peter Anvin wrote:
>> > I don't think it'd hurt... although I think the above pretty much
>> > requires that the code contain actual floating-point types to ever
>be
>> > generated.
>> 
>> Yes, that is supposed to catch usage of the 'E' floating constant,
>for
>> example, in definitions. It is a build-time assertion, if you want.
>:)
>
>And git grep says that mips abuses this notation:
>arch/mips/include/asm/kvm_host.h:#define MS_TO_NS(x) (x * 1E6L)
>
>I do not have cross-compiler set up to check whether this is a real
>issue because include/media/rc-core.h defines the same macro correctly
>so the real code might end up using the right variant.

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

Re: [RFC] x86: Disable traditional FPU instructions too

[Michal Hocko]

On Tue 28-01-14 02:25:09, H. Peter Anvin wrote:
> If that is intentional it needs a big huge freaking comment recording
> why it is not a bug. 

Agreed.

> I would expect that to generate a FP multiply.

Exactly! And actually the very same construct in a module provided to
reproduce an issue was a trigger to the discussed patch.

> On January 28, 2014 1:37:01 AM PST, Michal Hocko <mhocko@suse.cz> wrote:
> >[CCing Ralf for MIPS.]
> >
> >On Mon 27-01-14 17:45:44, Borislav Petkov wrote:
> >> On Mon, Jan 27, 2014 at 08:41:09AM -0800, H. Peter Anvin wrote:
> >> > I don't think it'd hurt... although I think the above pretty much
> >> > requires that the code contain actual floating-point types to ever
> >be
> >> > generated.
> >> 
> >> Yes, that is supposed to catch usage of the 'E' floating constant,
> >for
> >> example, in definitions. It is a build-time assertion, if you want.
> >:)
> >
> >And git grep says that mips abuses this notation:
> >arch/mips/include/asm/kvm_host.h:#define MS_TO_NS(x) (x * 1E6L)
> >
> >I do not have cross-compiler set up to check whether this is a real
> >issue because include/media/rc-core.h defines the same macro correctly
> >so the real code might end up using the right variant.
> 
> -- 
> Sent from my mobile phone.  Please pardon brevity and lack of formatting.

-- 
Michal Hocko
SUSE Labs