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: <CAKwvOdkTYqpphqr2cvL2pDuK33aYzxXsLDQ6+_PSUk6qxUuQ7A@mail.gmail.com>
Date:   Tue, 23 Jun 2020 13:58:15 -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 12:13 PM Borislav Petkov <bp@...en8.de> wrote:
>
> On Tue, Jun 23, 2020 at 12:07:13PM -0700, Nick Desaulniers wrote:
> > 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.
>
> Why if one can write it more compact with cc-option?
>
> FPU_CFLAGS += $(call cc-option,-msse2,)

This will always be true though, right? For both compilers.  So why
test for it via cc-option when you can just do:

FPU_CFLAGS += -msse2

Though we might only want -msse2 if CC_IS_CLAG...I don't remember now
if GCC will select SSE instructions that require 16B operands at
-msse2, which will be problematic for the -mpreferred-stack-boundary=4
case.

My point was more so about avoiding needless cc-option checks when
they're tautological.

> > 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.
>
> See Andy's suggestion upthread.

Thread for other travelers:
https://lore.kernel.org/lkml/CALCETrXXzt8WZMs3dsReCJ5wdF3zhxFmUtGnmdCgV7_exFUKKQ@mail.gmail.com/

When Andy says "consider dropping the problematic GCC version" I
wonder if it was meant *just for this selftest* as I suggested, or
outright (which is untenable IMO, as it's a large jump to GCC 7.1+).

> And I agree too that using cc-option is better than simply tying it to a
> compiler version.

I agree that will differentiate better than a version check; but it's
still dangerous IMO to mix and match stack alignments.

> Who knows what compiler has what backported. In such
> cases a version number means nothing.

I'm not sure I agree, but I'll take feature detection any day over
version detection.

>
> > ^ looks familiar ;)
>
> It has been pasted around the kernel, I came to realize today. :-)

Guilty, your honor. :P

Maybe the feature test should be copy+pasta'd to those other places in
the kernel, rather than the version check?
-- 
Thanks,
~Nick Desaulniers