crypto/crc32c: register only one shash_alg

PATCH In review Eric Biggers 2 participants 2 messages May 30, 2025 – May 31, 2025 Crypto API

lore

Share & tools

lore

Eric Biggers

author

2025-05-30 16:12 UTC #0 lore raw

PATCH 1/1 crypto/crc32c: register only one shash_alg

From: Eric Biggers <[email protected]>

Stop unnecessarily registering a "crc32c-generic" shash_alg when a "crc32c-$(ARCH)" shash_alg is registered too.

While every algorithm does need to have a generic implementation to ensure uniformity of support across platforms, that doesn't mean that we need to make the generic implementation available through crypto_shash when an optimized implementation is also available.

Registering the generic shash_alg did allow users of the crypto_shash or crypto_ahash APIs to request the generic implementation specifically, instead of an optimized one. However, the only known use case for that was the differential fuzz tests in crypto/testmgr.c. Equivalent test coverage is now provided by crc_kunit.

Signed-off-by Eric Biggers <[email protected]>

---

I'm planning to take this through the crc tree.

 crypto/crc32c.c | 70 +++++++------------------------------------------
 1 file changed, 10 insertions(+), 60 deletions(-)

crypto/crc32c.c +10 ~~-60~~

index e5377898414a2..e160695682f16 100644
@@ -83,19 +83,10 @@ static int chksum_setkey(struct crypto_shash *tfm, const u8 *key,
 static int chksum_update(struct shash_desc *desc, const u8 *data,
 			 unsigned int length)
 {
 	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
 
-	ctx->crc = crc32c_base(ctx->crc, data, length);
-	return 0;
-}
-
-static int chksum_update_arch(struct shash_desc *desc, const u8 *data,
-			      unsigned int length)
-{
-	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
-
 	ctx->crc = crc32c(ctx->crc, data, length);
 	return 0;
 }
 
 static int chksum_final(struct shash_desc *desc, u8 *out)

@@ -105,17 +96,10 @@ static int chksum_final(struct shash_desc *desc, u8 *out)
 	put_unaligned_le32(~ctx->crc, out);
 	return 0;
 }
 
 static int __chksum_finup(u32 *crcp, const u8 *data, unsigned int len, u8 *out)
-{
-	put_unaligned_le32(~crc32c_base(*crcp, data, len), out);
-	return 0;
-}
-
-static int __chksum_finup_arch(u32 *crcp, const u8 *data, unsigned int len,
-			       u8 *out)
 {
 	put_unaligned_le32(~crc32c(*crcp, data, len), out);
 	return 0;
 }

@@ -125,98 +109,64 @@ static int chksum_finup(struct shash_desc *desc, const u8 *data,
 	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
 
 	return __chksum_finup(&ctx->crc, data, len, out);
 }
 
-static int chksum_finup_arch(struct shash_desc *desc, const u8 *data,
-			     unsigned int len, u8 *out)
-{
-	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
-
-	return __chksum_finup_arch(&ctx->crc, data, len, out);
-}
-
 static int chksum_digest(struct shash_desc *desc, const u8 *data,
 			 unsigned int length, u8 *out)
 {
 	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
 
 	return __chksum_finup(&mctx->key, data, length, out);
 }
 
-static int chksum_digest_arch(struct shash_desc *desc, const u8 *data,
-			      unsigned int length, u8 *out)
-{
-	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
-
-	return __chksum_finup_arch(&mctx->key, data, length, out);
-}
-
 static int crc32c_cra_init(struct crypto_tfm *tfm)
 {
 	struct chksum_ctx *mctx = crypto_tfm_ctx(tfm);
 
 	mctx->key = ~0;
 	return 0;
 }
 
-static struct shash_alg algs[] = {{
+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",
-	.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,
-}, {
-	.digestsize		= CHKSUM_DIGEST_SIZE,
-	.setkey			= chksum_setkey,
-	.init			= chksum_init,
-	.update			= chksum_update_arch,
-	.final			= chksum_final,
-	.finup			= chksum_finup_arch,
-	.digest			= chksum_digest_arch,
-	.descsize		= sizeof(struct chksum_desc_ctx),
-
-	.base.cra_name		= "crc32c",
-	.base.cra_driver_name	= "crc32c-" __stringify(ARCH),
-	.base.cra_priority	= 150,
-	.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,
-}};
-
-static int num_algs;
+};
 
 static int __init crc32c_mod_init(void)
 {
-	/* register the arch flavor only if it differs from the generic one */
-	num_algs = 1 + ((crc32_optimizations() & CRC32C_OPTIMIZATION) != 0);
+	const char *driver_name =
+		(crc32_optimizations() & CRC32C_OPTIMIZATION) ?
+			"crc32c-" __stringify(ARCH) :
+			"crc32c-generic";
+
+	strscpy(alg.base.cra_driver_name, driver_name, CRYPTO_MAX_ALG_NAME);
 
-	return crypto_register_shashes(algs, num_algs);
+	return crypto_register_shash(&alg);
 }
 
 static void __exit crc32c_mod_fini(void)
 {
-	crypto_unregister_shashes(algs, num_algs);
+	crypto_unregister_shash(&alg);
 }
 
 module_init(crc32c_mod_init);
 module_exit(crc32c_mod_fini);
 
 MODULE_AUTHOR("Clay Haapala <[email protected]>");
 MODULE_DESCRIPTION("CRC32c (Castagnoli) calculations wrapper for lib/crc32c");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS_CRYPTO("crc32c");
-MODULE_ALIAS_CRYPTO("crc32c-generic");

base-commit: f66bc387efbee59978e076ce9bf123ac353b389c

Signature · 1 line

2.49.0

Herbert Xu

maintainer

2025-05-31 07:10 UTC #1 lore raw

Quoted 16 lines · Eric Biggers

From: Eric Biggers <[email protected]>
Stop unnecessarily registering a "crc32c-generic" shash_alg when a "crc32c-$(ARCH)" shash_alg is registered too.
While every algorithm does need to have a generic implementation to ensure uniformity of support across platforms, that doesn't mean that we need to make the generic implementation available through crypto_shash when an optimized implementation is also available.
Registering the generic shash_alg did allow users of the crypto_shash or crypto_ahash APIs to request the generic implementation specifically, instead of an optimized one. However, the only known use case for that was the differential fuzz tests in crypto/testmgr.c. Equivalent test coverage is now provided by crc_kunit.

Quoted 7 lines

Signed-off-by Eric Biggers <[email protected]>
---
I'm planning to take this through the crc tree.
crypto/crc32c.c | 70 +++++++------------------------------------------
1 file changed, 10 insertions(+), 60 deletions(-)

Ditto with "crc32". Please remove drivers/crypto/stm32/stm32-crc32.c first before getting rid of "crc32c-generic".

Thanks,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt