Re: [PATCH] crypto: move sm3 and sm4 into crypto directory

[Eric Biggers <ebiggers@kernel.org>]

On Sun, Mar 13, 2022 at 09:11:01PM -0600, Jason A. Donenfeld wrote:
> The lib/crypto libraries live in lib because they are used by various
> drivers of the kernel. In contrast, the various helper functions in
> crypto are there because they're used exclusively by the crypto API. The
> SM3 and SM4 helper functions were erroniously moved into lib/crypto/
> instead of crypto/, even though there are no in-kernel users outside of
> the crypto API of those functions. This commit moves them into crypto/.
> 
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> Cc: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  arch/arm64/crypto/Kconfig    |  4 ++--
>  crypto/Kconfig               | 18 ++++++++++++------
>  crypto/Makefile              |  6 ++++--
>  {lib/crypto => crypto}/sm3.c |  0
>  {lib/crypto => crypto}/sm4.c |  0
>  lib/crypto/Kconfig           |  6 ------
>  lib/crypto/Makefile          |  6 ------
>  7 files changed, 18 insertions(+), 22 deletions(-)
>  rename {lib/crypto => crypto}/sm3.c (100%)
>  rename {lib/crypto => crypto}/sm4.c (100%)
> 
> diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
> index 2a965aa0188d..454621a20eaa 100644
> --- a/arch/arm64/crypto/Kconfig
> +++ b/arch/arm64/crypto/Kconfig
> @@ -45,13 +45,13 @@ config CRYPTO_SM3_ARM64_CE
>  	tristate "SM3 digest algorithm (ARMv8.2 Crypto Extensions)"
>  	depends on KERNEL_MODE_NEON
>  	select CRYPTO_HASH
> -	select CRYPTO_LIB_SM3
> +	select CRYPTO_SM3
>  
>  config CRYPTO_SM4_ARM64_CE
>  	tristate "SM4 symmetric cipher (ARMv8.2 Crypto Extensions)"
>  	depends on KERNEL_MODE_NEON
>  	select CRYPTO_ALGAPI
> -	select CRYPTO_LIB_SM4
> +	select CRYPTO_SM4
>  
>  config CRYPTO_GHASH_ARM64_CE
>  	tristate "GHASH/AES-GCM using ARMv8 Crypto Extensions"
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index d6d7e84bb7f8..517525d7d12e 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -274,7 +274,7 @@ config CRYPTO_ECRDSA
>  
>  config CRYPTO_SM2
>  	tristate "SM2 algorithm"
> -	select CRYPTO_LIB_SM3
> +	select CRYPTO_SM3
>  	select CRYPTO_AKCIPHER
>  	select CRYPTO_MANAGER
>  	select MPILIB
> @@ -1005,9 +1005,12 @@ config CRYPTO_SHA3
>  	  http://keccak.noekeon.org/
>  
>  config CRYPTO_SM3
> +	tristate
> +
> +config CRYPTO_SM3_GENERIC
>  	tristate "SM3 digest algorithm"
>  	select CRYPTO_HASH
> -	select CRYPTO_LIB_SM3
> +	select CRYPTO_SM3
>  	help
>  	  SM3 secure hash function as defined by OSCCA GM/T 0004-2012 SM3).
>  	  It is part of the Chinese Commercial Cryptography suite.

This patch generally looks good, but perhaps CRYPTO_SM3 and CRYPTO_SM3_GENERIC
should be merged?  Having separate options provides the ability to enable the
architecture implementation without the generic implementation, so arguably it's
the right thing to do; however, that's not what the other algorithms in crypto/
do.  For example, if you enable CONFIG_CRYPTO_SHA3, you get the generic SHA-3
algorithms as well as the SHA-3 library functions.  I.e., there's no
CONFIG_CRYPTO_SHA3_GENERIC.

IMO, being consistent is more important than providing the ability to omit the
generic implementation.

- Eric