[PATCH v2 0/2] crypto: Enable fuzz testing for arch code

[PATCH v2 0/2] crypto: Enable fuzz testing for arch code

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

Follow-up to [0].

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 exposing the arch library code as a separate driver (with a
higher priority) if it is different from the generic C code. Update the
crc32-generic drivers to always use the generic C code.

Changes since [0]:
- make generic drivers truly generic, and expose the arch code as a
  separate driver

[0] https://lore.kernel.org/all/20241015141514.3000757-4-ardb+git@google.com/T/#u

Ard Biesheuvel (2):
  crypto/crc32: Provide crc32-arch driver for accelerated library code
  crypto/crc32c: Provide crc32c-arch driver for accelerated library code

 crypto/Makefile         |  2 +
 crypto/crc32_generic.c  | 94 +++++++++++++++-----
 crypto/crc32c_generic.c | 94 +++++++++++++++-----
 lib/crc32.c             |  4 +
 4 files changed, 148 insertions(+), 46 deletions(-)

-- 
2.47.0.rc1.288.g06298d1525-goog

[PATCH v2 1/2] crypto/crc32: Provide crc32-arch driver for accelerated library code

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

crc32-generic is currently backed by the architecture's CRC-32 library
code, 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 providing a crc32-arch implementation which is based on the
arch library code if available, and modify crc32-generic so it is
always based on the generic C implementation. If the arch has no CRC-32
library code, this change does nothing.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 crypto/Makefile        |  1 +
 crypto/crc32_generic.c | 94 +++++++++++++++-----
 lib/crc32.c            |  2 +
 3 files changed, 73 insertions(+), 24 deletions(-)

diff --git a/crypto/Makefile b/crypto/Makefile
index 4c99e5d376f6..2d4a3c0659fa 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -152,6 +152,7 @@ obj-$(CONFIG_CRYPTO_DEFLATE) += deflate.o
 obj-$(CONFIG_CRYPTO_MICHAEL_MIC) += michael_mic.o
 obj-$(CONFIG_CRYPTO_CRC32C) += crc32c_generic.o
 obj-$(CONFIG_CRYPTO_CRC32) += crc32_generic.o
+CFLAGS_crc32_generic.o += -DARCH=$(ARCH)
 obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif_common.o crct10dif_generic.o
 obj-$(CONFIG_CRYPTO_CRC64_ROCKSOFT) += crc64_rocksoft_generic.o
 obj-$(CONFIG_CRYPTO_AUTHENC) += authenc.o authencesn.o
diff --git a/crypto/crc32_generic.c b/crypto/crc32_generic.c
index d1251663ed66..6a55d206fab3 100644
--- a/crypto/crc32_generic.c
+++ b/crypto/crc32_generic.c
@@ -59,6 +59,15 @@ static int crc32_update(struct shash_desc *desc, const u8 *data,
 {
 	u32 *crcp = shash_desc_ctx(desc);
 
+	*crcp = crc32_le_base(*crcp, data, len);
+	return 0;
+}
+
+static int crc32_update_arch(struct shash_desc *desc, const u8 *data,
+			     unsigned int len)
+{
+	u32 *crcp = shash_desc_ctx(desc);
+
 	*crcp = crc32_le(*crcp, data, len);
 	return 0;
 }
@@ -66,6 +75,13 @@ static int crc32_update(struct shash_desc *desc, const u8 *data,
 /* No final XOR 0xFFFFFFFF, like crc32_le */
 static int __crc32_finup(u32 *crcp, const u8 *data, unsigned int len,
 			 u8 *out)
+{
+	put_unaligned_le32(crc32_le_base(*crcp, data, len), out);
+	return 0;
+}
+
+static int __crc32_finup_arch(u32 *crcp, const u8 *data, unsigned int len,
+			      u8 *out)
 {
 	put_unaligned_le32(crc32_le(*crcp, data, len), out);
 	return 0;
@@ -77,6 +93,12 @@ static int crc32_finup(struct shash_desc *desc, const u8 *data,
 	return __crc32_finup(shash_desc_ctx(desc), data, len, out);
 }
 
+static int crc32_finup_arch(struct shash_desc *desc, const u8 *data,
+		       unsigned int len, u8 *out)
+{
+	return __crc32_finup_arch(shash_desc_ctx(desc), data, len, out);
+}
+
 static int crc32_final(struct shash_desc *desc, u8 *out)
 {
 	u32 *crcp = shash_desc_ctx(desc);
@@ -88,38 +110,62 @@ static int crc32_final(struct shash_desc *desc, u8 *out)
 static int crc32_digest(struct shash_desc *desc, const u8 *data,
 			unsigned int len, u8 *out)
 {
-	return __crc32_finup(crypto_shash_ctx(desc->tfm), data, len,
-			     out);
+	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 int crc32_digest_arch(struct shash_desc *desc, const u8 *data,
+			     unsigned int len, u8 *out)
+{
+	return __crc32_finup_arch(crypto_shash_ctx(desc->tfm), data, len, out);
+}
+
+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,
+}, {
+	.setkey			= crc32_setkey,
+	.init			= crc32_init,
+	.update			= crc32_update_arch,
+	.final			= crc32_final,
+	.finup			= crc32_finup_arch,
+	.digest			= crc32_digest_arch,
+	.descsize		= sizeof(u32),
+	.digestsize		= CHKSUM_DIGEST_SIZE,
+
+	.base.cra_name		= "crc32",
+	.base.cra_driver_name	= "crc32-" __stringify(ARCH),
+	.base.cra_priority	= 150,
+	.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,
+}};
 
 static int __init crc32_mod_init(void)
 {
-	return crypto_register_shash(&alg);
+	/* register the arch flavor only if it differs from the generic one */
+	return crypto_register_shashes(algs, 1 + (&crc32_le != &crc32_le_base));
 }
 
 static void __exit crc32_mod_fini(void)
 {
-	crypto_unregister_shash(&alg);
+	crypto_unregister_shashes(algs, 1 + (&crc32_le != &crc32_le_base));
 }
 
 subsys_initcall(crc32_mod_init);
diff --git a/lib/crc32.c b/lib/crc32.c
index 5649847d0a8d..a54ba87b7073 100644
--- a/lib/crc32.c
+++ b/lib/crc32.c
@@ -205,6 +205,8 @@ EXPORT_SYMBOL(crc32_le);
 EXPORT_SYMBOL(__crc32c_le);
 
 u32 __pure crc32_le_base(u32, unsigned char const *, size_t) __alias(crc32_le);
+EXPORT_SYMBOL(crc32_le_base);
+
 u32 __pure __crc32c_le_base(u32, unsigned char const *, size_t) __alias(__crc32c_le);
 u32 __pure crc32_be_base(u32, unsigned char const *, size_t) __alias(crc32_be);
 
-- 
2.47.0.rc1.288.g06298d1525-goog

[PATCH v2 2/2] crypto/crc32c: Provide crc32c-arch driver for accelerated library code

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

crc32c-generic is currently backed by the architecture's CRC-32c library
code, 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 providing a crc32c-arch implementation which is based on the
arch library code if available, and modify crc32c-generic so it is
always based on the generic C implementation. If the arch has no CRC-32c
library code, this change does nothing.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 crypto/Makefile         |  1 +
 crypto/crc32c_generic.c | 94 +++++++++++++++-----
 lib/crc32.c             |  2 +
 3 files changed, 75 insertions(+), 22 deletions(-)

diff --git a/crypto/Makefile b/crypto/Makefile
index 2d4a3c0659fa..a1ce3fa5298d 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -152,6 +152,7 @@ obj-$(CONFIG_CRYPTO_DEFLATE) += deflate.o
 obj-$(CONFIG_CRYPTO_MICHAEL_MIC) += michael_mic.o
 obj-$(CONFIG_CRYPTO_CRC32C) += crc32c_generic.o
 obj-$(CONFIG_CRYPTO_CRC32) += crc32_generic.o
+CFLAGS_crc32c_generic.o += -DARCH=$(ARCH)
 CFLAGS_crc32_generic.o += -DARCH=$(ARCH)
 obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif_common.o crct10dif_generic.o
 obj-$(CONFIG_CRYPTO_CRC64_ROCKSOFT) += crc64_rocksoft_generic.o
diff --git a/crypto/crc32c_generic.c b/crypto/crc32c_generic.c
index a8c90b3f4c6c..7c2357c30fdf 100644
--- a/crypto/crc32c_generic.c
+++ b/crypto/crc32c_generic.c
@@ -85,6 +85,15 @@ static int chksum_update(struct shash_desc *desc, const u8 *data,
 {
 	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
 
+	ctx->crc = __crc32c_le_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_le(ctx->crc, data, length);
 	return 0;
 }
@@ -98,6 +107,13 @@ static int chksum_final(struct shash_desc *desc, u8 *out)
 }
 
 static int __chksum_finup(u32 *crcp, const u8 *data, unsigned int len, u8 *out)
+{
+	put_unaligned_le32(~__crc32c_le_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_le(*crcp, data, len), out);
 	return 0;
@@ -111,6 +127,14 @@ static int chksum_finup(struct shash_desc *desc, const u8 *data,
 	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)
 {
@@ -119,6 +143,14 @@ static int chksum_digest(struct shash_desc *desc, const u8 *data,
 	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);
@@ -127,35 +159,53 @@ 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,
+}, {
+	.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 __init crc32c_mod_init(void)
 {
-	return crypto_register_shash(&alg);
+	/* register the arch flavor only if it differs from the generic one */
+	return crypto_register_shashes(algs, 1 + (&__crc32c_le != &__crc32c_le_base));
 }
 
 static void __exit crc32c_mod_fini(void)
 {
-	crypto_unregister_shash(&alg);
+	crypto_unregister_shashes(algs, 1 + (&__crc32c_le != &__crc32c_le_base));
 }
 
 subsys_initcall(crc32c_mod_init);
diff --git a/lib/crc32.c b/lib/crc32.c
index a54ba87b7073..ff587fee3893 100644
--- a/lib/crc32.c
+++ b/lib/crc32.c
@@ -208,6 +208,8 @@ u32 __pure crc32_le_base(u32, unsigned char const *, size_t) __alias(crc32_le);
 EXPORT_SYMBOL(crc32_le_base);
 
 u32 __pure __crc32c_le_base(u32, unsigned char const *, size_t) __alias(__crc32c_le);
+EXPORT_SYMBOL(__crc32c_le_base);
+
 u32 __pure crc32_be_base(u32, unsigned char const *, size_t) __alias(crc32_be);
 
 /*
-- 
2.47.0.rc1.288.g06298d1525-goog

Re: [PATCH v2 0/2] crypto: Enable fuzz testing for arch code

[Eric Biggers]

On Wed, Oct 16, 2024 at 08:57:23PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Follow-up to [0].
> 
> 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 exposing the arch library code as a separate driver (with a
> higher priority) if it is different from the generic C code. Update the
> crc32-generic drivers to always use the generic C code.
> 
> Changes since [0]:
> - make generic drivers truly generic, and expose the arch code as a
>   separate driver
> 
> [0] https://lore.kernel.org/all/20241015141514.3000757-4-ardb+git@google.com/T/#u
> 
> Ard Biesheuvel (2):
>   crypto/crc32: Provide crc32-arch driver for accelerated library code
>   crypto/crc32c: Provide crc32c-arch driver for accelerated library code
> 
>  crypto/Makefile         |  2 +
>  crypto/crc32_generic.c  | 94 +++++++++++++++-----
>  crypto/crc32c_generic.c | 94 +++++++++++++++-----
>  lib/crc32.c             |  4 +
>  4 files changed, 148 insertions(+), 46 deletions(-)

Reviewed-by: Eric Biggers <ebiggers@google.com>

- Eric

Re: [PATCH v2 0/2] crypto: Enable fuzz testing for arch code

[Herbert Xu]

On Wed, Oct 16, 2024 at 08:57:23PM +0200, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> Follow-up to [0].
> 
> 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 exposing the arch library code as a separate driver (with a
> higher priority) if it is different from the generic C code. Update the
> crc32-generic drivers to always use the generic C code.
> 
> Changes since [0]:
> - make generic drivers truly generic, and expose the arch code as a
>   separate driver
> 
> [0] https://lore.kernel.org/all/20241015141514.3000757-4-ardb+git@google.com/T/#u
> 
> Ard Biesheuvel (2):
>   crypto/crc32: Provide crc32-arch driver for accelerated library code
>   crypto/crc32c: Provide crc32c-arch driver for accelerated library code
> 
>  crypto/Makefile         |  2 +
>  crypto/crc32_generic.c  | 94 +++++++++++++++-----
>  crypto/crc32c_generic.c | 94 +++++++++++++++-----
>  lib/crc32.c             |  4 +
>  4 files changed, 148 insertions(+), 46 deletions(-)
> 
> -- 
> 2.47.0.rc1.288.g06298d1525-goog

All applied.  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