[PATCH 0/2] crypto: Enable fuzz testing for generic crc32/crc32c

[PATCH 0/2] crypto: Enable fuzz testing for generic crc32/crc32c

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

crc32-generic and crc32c-generic are built around the architecture
library code for CRC-32, and the lack of distinct drivers for this arch
code means they are lacking test coverage.

Fix this by providing another, truly generic driver built on top of the
generic C code when fuzz testing is enabled.

Ard Biesheuvel (2):
  crypto/crc32: Provide crc32-base alias to enable fuzz testing
  crypto/crc32c: Provide crc32c-base alias to enable fuzz testing

 crypto/crc32_generic.c  | 73 ++++++++++++++------
 crypto/crc32c_generic.c | 72 +++++++++++++------
 crypto/testmgr.c        |  2 +
 3 files changed, 103 insertions(+), 44 deletions(-)

-- 
2.47.0.rc1.288.g06298d1525-goog

[PATCH 1/2] crypto/crc32: Provide crc32-base alias to enable fuzz testing

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

crc32-generic is backed by the architecture's CRC-32 library, which may
offer a variety of implementations depending on the capabilities of the
platform. These are not covered by the crypto subsystem's fuzz testing
capabilities because crc32-generic is the reference driver that the
fuzzing logic uses as a source of truth.

Fix this by proving a crc32-base implementation which is always based on
the generic C implementation, and wire it up as the reference
implementation for the fuzz tester.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 crypto/crc32_generic.c | 73 ++++++++++++++------
 crypto/testmgr.c       |  1 +
 2 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/crypto/crc32_generic.c b/crypto/crc32_generic.c
index d1251663ed66..73948ce96ac7 100644
--- a/crypto/crc32_generic.c
+++ b/crypto/crc32_generic.c
@@ -63,6 +63,17 @@ static int crc32_update(struct shash_desc *desc, const u8 *data,
 	return 0;
 }
 
+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+static int crc32_update_base(struct shash_desc *desc, const u8 *data,
+			     unsigned int len)
+{
+	u32 *crcp = shash_desc_ctx(desc);
+
+	*crcp = crc32_le_base(*crcp, data, len);
+	return 0;
+}
+#endif
+
 /* No final XOR 0xFFFFFFFF, like crc32_le */
 static int __crc32_finup(u32 *crcp, const u8 *data, unsigned int len,
 			 u8 *out)
@@ -91,35 +102,53 @@ static int crc32_digest(struct shash_desc *desc, const u8 *data,
 	return __crc32_finup(crypto_shash_ctx(desc->tfm), data, len,
 			     out);
 }
-static struct shash_alg alg = {
-	.setkey		= crc32_setkey,
-	.init		= crc32_init,
-	.update		= crc32_update,
-	.final		= crc32_final,
-	.finup		= crc32_finup,
-	.digest		= crc32_digest,
-	.descsize	= sizeof(u32),
-	.digestsize	= CHKSUM_DIGEST_SIZE,
-	.base		= {
-		.cra_name		= "crc32",
-		.cra_driver_name	= "crc32-generic",
-		.cra_priority		= 100,
-		.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
-		.cra_blocksize		= CHKSUM_BLOCK_SIZE,
-		.cra_ctxsize		= sizeof(u32),
-		.cra_module		= THIS_MODULE,
-		.cra_init		= crc32_cra_init,
-	}
-};
+static struct shash_alg algs[] = {{
+	.setkey			= crc32_setkey,
+	.init			= crc32_init,
+	.update			= crc32_update,
+	.final			= crc32_final,
+	.finup			= crc32_finup,
+	.digest			= crc32_digest,
+	.descsize		= sizeof(u32),
+	.digestsize		= CHKSUM_DIGEST_SIZE,
+
+	.base.cra_name		= "crc32",
+	.base.cra_driver_name	= "crc32-generic",
+	.base.cra_priority	= 100,
+	.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
+	.base.cra_blocksize	= CHKSUM_BLOCK_SIZE,
+	.base.cra_ctxsize	= sizeof(u32),
+	.base.cra_module	= THIS_MODULE,
+	.base.cra_init		= crc32_cra_init,
+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+}, {
+	.setkey			= crc32_setkey,
+	.init			= crc32_init,
+	.update			= crc32_update_base,
+	.final			= crc32_final,
+	.digest			= crc32_digest,
+	.descsize		= sizeof(u32),
+	.digestsize		= CHKSUM_DIGEST_SIZE,
+
+	.base.cra_name		= "crc32",
+	.base.cra_driver_name	= "crc32-base",
+	.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
+	.base.cra_blocksize	= CHKSUM_BLOCK_SIZE,
+	.base.cra_ctxsize	= sizeof(u32),
+	.base.cra_module	= THIS_MODULE,
+	.base.cra_init		= crc32_cra_init,
+#endif
+}};
 
 static int __init crc32_mod_init(void)
 {
-	return crypto_register_shash(&alg);
+	return crypto_register_shashes(algs, ARRAY_SIZE(algs));
 }
 
 static void __exit crc32_mod_fini(void)
 {
-	crypto_unregister_shash(&alg);
+	crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
+
 }
 
 subsys_initcall(crc32_mod_init);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index ee8da628e9da..e0d6bfcc13e6 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4692,6 +4692,7 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.test = alg_test_null,
 	}, {
 		.alg = "crc32",
+		.generic_driver = "crc32-base",
 		.test = alg_test_hash,
 		.fips_allowed = 1,
 		.suite = {
-- 
2.47.0.rc1.288.g06298d1525-goog

[PATCH 2/2] crypto/crc32c: Provide crc32c-base alias to enable fuzz testing

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

crc32-generic is backed by the architecture's CRC-32c library, which may
offer a variety of implementations depending on the capabilities of the
platform. These are not covered by the crypto subsystem's fuzz testing
capabilities because crc32c-generic is the reference driver that the
fuzzing logic uses as a source of truth.

Fix this by proving a crc32c-base implementation which is always based
on the generic C implementation, and wire it up as the reference
implementation for the fuzz tester.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 crypto/crc32c_generic.c | 72 ++++++++++++++------
 crypto/testmgr.c        |  1 +
 2 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/crypto/crc32c_generic.c b/crypto/crc32c_generic.c
index a8c90b3f4c6c..0a7543dbc515 100644
--- a/crypto/crc32c_generic.c
+++ b/crypto/crc32c_generic.c
@@ -80,6 +80,17 @@ static int chksum_setkey(struct crypto_shash *tfm, const u8 *key,
 	return 0;
 }
 
+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+static int chksum_update_base(struct shash_desc *desc, const u8 *data,
+			      unsigned int length)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	ctx->crc = __crc32c_le_base(ctx->crc, data, length);
+	return 0;
+}
+#endif
+
 static int chksum_update(struct shash_desc *desc, const u8 *data,
 			 unsigned int length)
 {
@@ -127,35 +138,52 @@ static int crc32c_cra_init(struct crypto_tfm *tfm)
 	return 0;
 }
 
-static struct shash_alg alg = {
-	.digestsize		=	CHKSUM_DIGEST_SIZE,
-	.setkey			=	chksum_setkey,
-	.init		=	chksum_init,
-	.update		=	chksum_update,
-	.final		=	chksum_final,
-	.finup		=	chksum_finup,
-	.digest		=	chksum_digest,
-	.descsize		=	sizeof(struct chksum_desc_ctx),
-	.base			=	{
-		.cra_name		=	"crc32c",
-		.cra_driver_name	=	"crc32c-generic",
-		.cra_priority		=	100,
-		.cra_flags		=	CRYPTO_ALG_OPTIONAL_KEY,
-		.cra_blocksize		=	CHKSUM_BLOCK_SIZE,
-		.cra_ctxsize		=	sizeof(struct chksum_ctx),
-		.cra_module		=	THIS_MODULE,
-		.cra_init		=	crc32c_cra_init,
-	}
-};
+static struct shash_alg algs[] = {{
+	.digestsize		= CHKSUM_DIGEST_SIZE,
+	.setkey			= chksum_setkey,
+	.init			= chksum_init,
+	.update			= chksum_update,
+	.final			= chksum_final,
+	.finup			= chksum_finup,
+	.digest			= chksum_digest,
+	.descsize		= sizeof(struct chksum_desc_ctx),
+
+	.base.cra_name		= "crc32c",
+	.base.cra_driver_name	= "crc32c-generic",
+	.base.cra_priority	= 100,
+	.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
+	.base.cra_blocksize	= CHKSUM_BLOCK_SIZE,
+	.base.cra_ctxsize	= sizeof(struct chksum_ctx),
+	.base.cra_module	= THIS_MODULE,
+	.base.cra_init		= crc32c_cra_init,
+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+}, {
+	.digestsize		= CHKSUM_DIGEST_SIZE,
+	.setkey			= chksum_setkey,
+	.init			= chksum_init,
+	.update			= chksum_update_base,
+	.final			= chksum_final,
+	.digest			= chksum_digest,
+	.descsize		= sizeof(struct chksum_desc_ctx),
+
+	.base.cra_name		= "crc32c",
+	.base.cra_driver_name	= "crc32c-base",
+	.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
+	.base.cra_blocksize	= CHKSUM_BLOCK_SIZE,
+	.base.cra_ctxsize	= sizeof(struct chksum_ctx),
+	.base.cra_module	= THIS_MODULE,
+	.base.cra_init		= crc32c_cra_init,
+#endif
+}};
 
 static int __init crc32c_mod_init(void)
 {
-	return crypto_register_shash(&alg);
+	return crypto_register_shashes(algs, ARRAY_SIZE(algs));
 }
 
 static void __exit crc32c_mod_fini(void)
 {
-	crypto_unregister_shash(&alg);
+	crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
 }
 
 subsys_initcall(crc32c_mod_init);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index e0d6bfcc13e6..c3f1cfe9e17e 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4700,6 +4700,7 @@ static const struct alg_test_desc alg_test_descs[] = {
 		}
 	}, {
 		.alg = "crc32c",
+		.generic_driver = "crc32c-base",
 		.test = alg_test_crc32c,
 		.fips_allowed = 1,
 		.suite = {
-- 
2.47.0.rc1.288.g06298d1525-goog

Re: [PATCH 0/2] crypto: Enable fuzz testing for generic crc32/crc32c

[Eric Biggers]

On Tue, Oct 15, 2024 at 04:15:15PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> crc32-generic and crc32c-generic are built around the architecture
> library code for CRC-32, and the lack of distinct drivers for this arch
> code means they are lacking test coverage.
> 
> Fix this by providing another, truly generic driver built on top of the
> generic C code when fuzz testing is enabled.

Wouldn't it make more sense to make crc32-generic actually be the generic
implementation, and add crc32-arm64 and crc32-riscv?  Likewise for crc32c.  That
is the usual way that the algorithms get wired up.

Even if we take a shortcut and don't do that, the naming could at least be more
consistent, e.g. rename the existing crc32-generic to crc32-lib and add
crc32-generic alongside that.

- Eric

Re: [PATCH 1/2] crypto/crc32: Provide crc32-base alias to enable fuzz testing

[Eric Biggers]

On Tue, Oct 15, 2024 at 04:15:16PM +0200, Ard Biesheuvel wrote:
> +#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
> +}, {
> +	.setkey			= crc32_setkey,
> +	.init			= crc32_init,
> +	.update			= crc32_update_base,
> +	.final			= crc32_final,
> +	.digest			= crc32_digest,

crc32_digest() uses the lib implementation.

- Eric

Re: [PATCH 0/2] crypto: Enable fuzz testing for generic crc32/crc32c

[Herbert Xu]

On Tue, Oct 15, 2024 at 07:20:51PM -0700, Eric Biggers wrote:
>
> Wouldn't it make more sense to make crc32-generic actually be the generic
> implementation, and add crc32-arm64 and crc32-riscv?  Likewise for crc32c.  That
> is the usual way that the algorithms get wired up.

Agreed.  The library interface can expose the optimal algorithm,
but the Crypto API should not expose the library interface and
should instead hook directly to the C implementation.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/2] crypto/crc32: Provide crc32-base alias to enable fuzz testing

[kernel test robot]

Hi Ard,

kernel test robot noticed the following build errors:

[auto build test ERROR on herbert-cryptodev-2.6/master]
[also build test ERROR on herbert-crypto-2.6/master linus/master v6.12-rc3 next-20241018]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ard-Biesheuvel/crypto-crc32-Provide-crc32-base-alias-to-enable-fuzz-testing/20241015-221858
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link:    https://lore.kernel.org/r/20241015141514.3000757-5-ardb%2Bgit%40google.com
patch subject: [PATCH 1/2] crypto/crc32: Provide crc32-base alias to enable fuzz testing
config: i386-buildonly-randconfig-002-20241018 (https://download.01.org/0day-ci/archive/20241019/202410190753.2wu0ZufT-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410190753.2wu0ZufT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410190753.2wu0ZufT-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in lib/zlib_inflate/zlib_inflate.o
>> ERROR: modpost: "crc32_le_base" [crypto/crc32_generic.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] crypto/crc32c: Provide crc32c-base alias to enable fuzz testing

[kernel test robot]

Hi Ard,

kernel test robot noticed the following build errors:

[auto build test ERROR on herbert-cryptodev-2.6/master]
[also build test ERROR on herbert-crypto-2.6/master linus/master v6.12-rc3 next-20241018]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ard-Biesheuvel/crypto-crc32-Provide-crc32-base-alias-to-enable-fuzz-testing/20241015-221858
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link:    https://lore.kernel.org/r/20241015141514.3000757-6-ardb%2Bgit%40google.com
patch subject: [PATCH 2/2] crypto/crc32c: Provide crc32c-base alias to enable fuzz testing
config: hexagon-randconfig-001-20241018 (https://download.01.org/0day-ci/archive/20241019/202410191558.TPVuZtME-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project bfe84f7085d82d06d61c632a7bad1e692fd159e4)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241019/202410191558.TPVuZtME-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410191558.TPVuZtME-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__crc32c_le_base" [crypto/crc32c_generic.ko] undefined!

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki