linux-kernel - Re: [v3 PATCH] crypto: lib/Kconfig

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: <134f64aa-65bd-4de0-9ac6-52326e35d6d6@app.fastmail.com>
Date: Thu, 27 Feb 2025 09:32:51 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Herbert Xu" <herbert@...dor.apana.org.au>,
 "Arnd Bergmann" <arnd@...nel.org>
Cc: "Will Deacon" <will@...nel.org>, "David S . Miller" <davem@...emloft.net>,
 "Catalin Marinas" <catalin.marinas@....com>,
 "Thomas Bogendoerfer" <tsbogend@...ha.franken.de>,
 "Harald Freudenberger" <freude@...ux.ibm.com>,
 "Holger Dengler" <dengler@...ux.ibm.com>,
 "Heiko Carstens" <hca@...ux.ibm.com>, "Vasily Gorbik" <gor@...ux.ibm.com>,
 "Alexander Gordeev" <agordeev@...ux.ibm.com>,
 "Christian Borntraeger" <borntraeger@...ux.ibm.com>,
 "Sven Schnelle" <svens@...ux.ibm.com>,
 "Martin K. Petersen" <martin.petersen@...cle.com>,
 "Ard Biesheuvel" <ardb@...nel.org>, "Eric Biggers" <ebiggers@...gle.com>,
 "James E . J . Bottomley" <James.Bottomley@...senpartnership.com>,
 "Jarkko Sakkinen" <jarkko@...nel.org>, linux-crypto@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-mips@...r.kernel.org, linux-s390@...r.kernel.org
Subject: Re: [v3 PATCH] crypto: lib/Kconfig - Hide arch options from user

On Thu, Feb 27, 2025, at 08:48, Herbert Xu wrote:
> The ARCH_MAY_HAVE patch missed arm64, mips and s390.  But it may
> also lead to arch options being enabled but ineffective because
> of modular/built-in conflicts.
>
> As the primary user of all these options wireguard is selecting
> the arch options anyway, make the same selections at the lib/crypto
> option level and hide the arch options from the user.
>
> Instead of selecting them centrally from lib/crypto, simply set
> the default of each arch option as suggested by Eric Biggers.
>
> Change the Crypto API generic algorithms to select the top-level
> lib/crypto options instead of the generic one as otherwise there
> is no way to enable the arch options (Eric Biggers).  Introduce a
> set of INTERNAL options to work around dependency cycles on the
> CONFIG_CRYPTO symbol.
>
> Fixes: 1047e21aecdf ("crypto: lib/Kconfig - Fix lib built-in failure 
> when arch is modular")
> Reported-by: kernel test robot <lkp@...el.com>
> Reported-by: Arnd Bergmann <arnd@...nel.org>
> Closes: 
> https://lore.kernel.org/oe-kbuild-all/202502232152.JC84YDLp-lkp@intel.com/
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>

This looks like a good approach. Once it works correctly,
it should be possible to clean up the 'select' statements
in wireguard as well and just 'select CRYPTO_LIB_CHACHA' etc.

> @@ -45,9 +47,10 @@ config CRYPTO_NHPOLY1305_NEON
>  	  - NEON (Advanced SIMD) extensions
> 
>  config CRYPTO_POLY1305_ARM
> -	tristate "Hash functions: Poly1305 (NEON)"
> +	tristate
>  	select CRYPTO_HASH
> -	select CRYPTO_ARCH_MAY_HAVE_LIB_POLY1305
> +	select CRYPTO_ARCH_HAVE_LIB_POLY1305
> +	default CRYPTO_LIB_POLY1305_INTERNAL
>  	help
>  	  Poly1305 authenticator algorithm (RFC7539)
> 
> @@ -212,9 +215,10 @@ config CRYPTO_AES_ARM_CE
>  	  - ARMv8 Crypto Extensions
> 
>  config CRYPTO_CHACHA20_NEON
> -	tristate "Ciphers: ChaCha20, XChaCha20, XChaCha12 (NEON)"
> +	tristate
>  	select CRYPTO_SKCIPHER
> -	select CRYPTO_ARCH_MAY_HAVE_LIB_CHACHA
> +	select CRYPTO_ARCH_HAVE_LIB_CHACHA
> +	default CRYPTO_LIB_CHACHA_INTERNAL

I think the more common style is to put the 'default'
lines before 'select'.

It appears that the two above are missing a
'depends on KERNEL_MODE_NEON' line. There is still
a runtime check that prevents it from being used on
non-neon machines, but I think you should add these
lines here since it's no longer possible to turn
them off individually when building a kernel for a
non-NEON target.

> +config CRYPTO_LIB_CHACHA_INTERNAL
> +	tristate
> +	select CRYPTO_LIB_CHACHA_GENERIC if CRYPTO_ARCH_HAVE_LIB_CHACHA=n
> +
>  config CRYPTO_LIB_CHACHA
>  	tristate "ChaCha library interface"
> -	select CRYPTO_LIB_CHACHA_GENERIC if CRYPTO_ARCH_HAVE_LIB_CHACHA=n
> +	select CRYPTO
> +	select CRYPTO_LIB_CHACHA_INTERNAL
>  	help
>  	  Enable the ChaCha library interface. This interface may be fulfilled
>  	  by either the generic implementation or an arch-specific one, if one

I'm not sure why we need the extra "_INTERNAL" symbols, but I
may be missing something here. What problem does this solve
for you?

      Arnd