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:   Fri, 13 Mar 2020 14:27:29 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        "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

Hi Linus,

On Fri, Mar 13, 2020 at 1:41 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> 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.


Thanks for the heads-up.

In fact, as-instr is already used in Kconfig.
arch/arm64/Kconfig: line 1396


arm / arm64 are simple cases because
32, 64-bit is separated by directory.

There is one thing we need to be careful about.
The x86 GCC is usually biarch.
So, when evaluating 64-bit assembly code
with a default 32-bit compiler,
-m64 must be passed.

I will keep this conversion in my mind.

Thanks.

> 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



-- 
Best Regards
Masahiro Yamada