linux-kernel - Re: [PATCH 2/2] selftests/fpu: Add an FPU selftest

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: <CAKwvOdnt+8RR=1JPjDNaVY8T1K7wmqFjSGM7XNUUGRb=t1hiWQ@mail.gmail.com>
Date:   Tue, 23 Jun 2020 12:07:13 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
        jpa@...nelbug.mail.kapsi.fi, Dave Hansen <dave.hansen@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] selftests/fpu: Add an FPU selftest

On Tue, Jun 23, 2020 at 11:47 AM Borislav Petkov <bp@...en8.de> wrote:
>
> On Tue, Jun 23, 2020 at 11:22:53AM -0700, Andy Lutomirski wrote:
> > See that same atrocious bug report.  It's the insane interaction
> > between -mno-sse2 and -mpreferred-stack-boundary.  So you need to
> > cc-option them both?  Or just stick with a compiler version check, I
> > guess.

Yeah, IIRC, this was fixed in GCC 7.1. See also
c868868f6b6a5272350781f9a19b3a5ba1c00b02
00db297106e81770e7c4319014a67896053b5a22 (this one for details about GCC 7.1)
e8a170ff9a3576730e43c0dbdd27b7cd3dc56848

>
> Yes, it was the interaction. This below seems to work. Note the "-msse"
> in the first argument of cc-option which causes the compiler to error
> out with
>
> /dev/null:1:0: error: -mpreferred-stack-boundary=3 is not between 4 and 12
>
> Adding Nick for the clang side.
>
> @Nick: I'm simply going to add -msse2 with cc-option.

You already have a conditional below for CC_IS_GCC; just add an else
and unconditionally add -msse2.  You *should* use -msse2 for GCC 7.1+
IMO.

Note that Clang has -mstack-alignment=8 whereas GCC has
-mpreferred-stack-boundary=3.  (Clang is a value in bytes, GCC is 2^X
bytes)

I recommend a version check for GCC < 7.1, or simply disabling the
self test if the version of GCC used is older than 7.1.  Mixing stack
alignments is a recipe for GPF with SSE when the caller has a smaller
stack alignment than the callee and is also unspecified behavior.  It
might work today, but as soon as someone perturbs the stack of the
call chain, you'll likely see a GPF due to non-16B aligned stack when
using SSE with stack operands in the caller.

>
> Anyway, lemme test this thing a bit more.
>
> Thx.
>
> ---
>
> # CFLAGS for compiling floating point code inside the kernel. x86/Makefile turns
> # off the generation of FPU/SSE* instructions for kernel proper but FPU_FLAGS
> # get appended last to CFLAGS and thus override those previous compiler options.
> #
> FPU_CFLAGS := -mhard-float -msse
> FPU_CFLAGS += $(call cc-option,-msse2,)
> ifdef CONFIG_CC_IS_GCC
> # Stack alignment mismatch, proceed with caution.
> # GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3
> # (8B stack alignment).

^ looks familiar ;)

> # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53383
> FPU_CFLAGS += $(call cc-option,-msse -mpreferred-stack-boundary=3,-mpreferred-stack-boundary=4)
> endif
>
> obj-$(CONFIG_TEST_FPU) += test_fpu.o
> CFLAGS_test_fpu.o += $(FPU_CFLAGS)
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette



-- 
Thanks,
~Nick Desaulniers