linux-kernel - Re: [GIT PULL] Crypto Fixes for 5.6

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]

Date:   Thu, 12 Mar 2020 09:40:54 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Herbert Xu <herbert@...dor.apana.org.au>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>
Subject: Re: [GIT PULL] Crypto Fixes for 5.6

On Thu, Mar 12, 2020 at 4:57 AM Herbert Xu <herbert@...dor.apana.org.au> wrote:
>
> This push fixes a build problem with x86/curve25519.

Pulled.

I do have a comment, though: this fix matches the existing pattern of
checking for assembler support, but that existing pattern is
absolutely horrible.

Would some enterprising individual please look at making the
CONFIG_AS_xyz flags use the _real_ config subsystem rather than ad-hoc
Makefile rules?

IOW, instead of having

  adx_instr := $(call as-instr,adox %r10$(comma)%r10,-DCONFIG_AS_ADX=1)
  ..
  adx_supported := $(call as-instr,adox %r10$(comma)%r10,yes,no)

in the makefiles, and silently changing how the Kconfig variables work
depending on those flags, make that DCONFIG_AS_ADX be a real config
variable:

   config AS_ADX
           def_bool $(success,$(srctree)/scripts/as-instr.sh "adox %r10,%r10")

or something like that?

And then we can make that CRYPTO_CURVE25519_X86 config variable simply have a

        depends on AS_ADX

in it, and the Kconfig system just takes care of these dependencies on its own.

Anyway, the crypto change isn't _wrong_, but it does point out an ugly
little horror in how the crypto layer silently basically changes the
configuration depending on other things.

For an example of why this is problematic: it means that if somebody
sends you their config file, the actual configuration you get may be
*completely* different from what they actually had, depending on
tools.

Added Masahiro to the cc, since he's used to the 'def_bool' model, and
also is familiar with our existing 'as-instr' Makefile macro.

So this is basically me throwing out a "I wish somebody would look at
this". Not meant as a criticism of the commit in question.

            Linus