Skip to contentSkip to conversation

crypto: lib - create utils module

PATCH Superseded Eric Biggers 3 participants 8 messages Jul 16, 2022 – Jul 21, 2022

Subsystems (3)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Eric Biggers

author maintainer
#0 lore raw

v2 cover crypto: lib - create utils module

Create a utils module in lib/crypto/, and move __crypto_memneq, __crypto_xor, and crypto_simd_disabled_for_test into it.

This supersedes "crypto: xor - move __crypto_xor into lib/"
(https://lore.kernel.org/linux-crypto/[email protected]/T/#u).

Eric Biggers (3):
crypto: lib - create utils module and move __crypto_memneq into it
crypto: lib - move __crypto_xor into utils
crypto: lib - move crypto_simd_disabled_for_test into utils

 crypto/Kconfig            |  2 +-
 crypto/algapi.c           | 77 ---------------------------------
 lib/Kconfig               |  3 --
 lib/Makefile              |  1 -
 lib/crypto/Kconfig        |  8 ++--
 lib/crypto/Makefile       |  3 ++
 lib/{ => crypto}/memneq.c |  0
 lib/crypto/utils.c        | 91 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 100 insertions(+), 85 deletions(-)
 rename lib/{ => crypto}/memneq.c (100%)
 create mode 100644 lib/crypto/utils.c

base-commit: ec8f7f4821d5e70d71601519bc2325b311324a96

Signature · 1 line
2.37.0
author maintainer
#1 lore raw

PATCH v2 1/3 crypto: lib - create utils module and move __crypto_memneq into it

From: Eric Biggers <[email protected]>

As requested at
https://lore.kernel.org/r/[email protected], move __crypto_memneq into lib/crypto/ and put it under a new tristate. The tristate is CRYPTO_LIB_UTILS, and it builds a module libcryptoutils. As more crypto library utilities are being added, this creates a single place for them to go without cluttering up the main lib directory.

The module's main file will be lib/crypto/utils.c. However, leave memneq.c as its own file because of its nonstandard license.

---
 crypto/Kconfig            | 2 +-
 lib/Kconfig               | 3 ---
 lib/Makefile              | 1 -
 lib/crypto/Kconfig        | 5 ++++-
 lib/crypto/Makefile       | 3 +++
 lib/{ => crypto}/memneq.c | 0
 6 files changed, 8 insertions(+), 6 deletions(-)
 rename lib/{ => crypto}/memneq.c (100%)
crypto/Kconfig +1 -1
index 75c71d9a5ffb7e..dc7c33f4ee6d55 100644
@@ -15,7 +15,7 @@ source "crypto/async_tx/Kconfig"
 #
 menuconfig CRYPTO
 	tristate "Cryptographic API"
-	select LIB_MEMNEQ
+	select CRYPTO_LIB_UTILS
 	help
 	  This option provides the core Cryptographic API.
 
lib/Kconfig +0 -3
index eaaad4d85bf24b..6a843639814fbf 100644
@@ -120,9 +120,6 @@ config INDIRECT_IOMEM_FALLBACK
 
 source "lib/crypto/Kconfig"
 
-config LIB_MEMNEQ
-	bool
-
 config CRC_CCITT
 	tristate "CRC-CCITT functions"
 	help
lib/Makefile +0 -1
index 67482f5ec0e899..0557be76c2565f 100644
@@ -251,7 +251,6 @@ obj-$(CONFIG_DIMLIB) += dim/
 obj-$(CONFIG_SIGNATURE) += digsig.o
 
 lib-$(CONFIG_CLZ_TAB) += clz_tab.o
-lib-$(CONFIG_LIB_MEMNEQ) += memneq.o
 
 obj-$(CONFIG_GENERIC_STRNCPY_FROM_USER) += strncpy_from_user.o
 obj-$(CONFIG_GENERIC_STRNLEN_USER) += strnlen_user.o
lib/crypto/Kconfig +4 -1
index 9ff549f63540fa..b09d9d6546cbc3 100644
@@ -2,6 +2,9 @@
 
 menu "Crypto library routines"
 
+config CRYPTO_LIB_UTILS
+	tristate
+
 config CRYPTO_LIB_AES
 	tristate
 
@@ -71,7 +74,7 @@ config CRYPTO_LIB_CURVE25519
 	tristate "Curve25519 scalar multiplication library"
 	depends on CRYPTO_ARCH_HAVE_LIB_CURVE25519 || !CRYPTO_ARCH_HAVE_LIB_CURVE25519
 	select CRYPTO_LIB_CURVE25519_GENERIC if CRYPTO_ARCH_HAVE_LIB_CURVE25519=n
-	select LIB_MEMNEQ
+	select CRYPTO_LIB_UTILS
 	help
 	  Enable the Curve25519 library interface. This interface may be
 	  fulfilled by either the generic implementation or an arch-specific
lib/crypto/Makefile +3 -0
index 919cbb2c220d61..b956b3bae26aaf 100644
@@ -1,5 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
+obj-$(CONFIG_CRYPTO_LIB_UTILS)			+= libcryptoutils.o
+libcryptoutils-y				:= memneq.o
+
 # chacha is used by the /dev/random driver which is always builtin
 obj-y						+= chacha.o
 obj-$(CONFIG_CRYPTO_LIB_CHACHA_GENERIC)		+= libchacha.o
lib/crypto/memneq.c renamed from lib/memneq.c +0 -0
similarity index 100%
rename from lib/memneq.c
rename to lib/crypto/memneq.c
Signature · 1 line
2.37.0
author maintainer
#2 lore raw

PATCH v2 2/3 crypto: lib - move __crypto_xor into utils

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

CRYPTO_LIB_CHACHA depends on CRYPTO for __crypto_xor, defined in crypto/algapi.c. This is a layering violation because the dependencies should only go in the other direction (crypto/ => lib/crypto/). Also the correct dependency would be CRYPTO_ALGAPI, not CRYPTO. Fix this by moving __crypto_xor into the utils module in lib/crypto/.

Note that CRYPTO_LIB_CHACHA_GENERIC selected XOR_BLOCKS, which is unrelated and unnecessary. It was perhaps thought that XOR_BLOCKS was needed for __crypto_xor, but that's not the case.

---
 crypto/algapi.c     | 71 -------------------------------------
 lib/crypto/Kconfig  |  3 +-
 lib/crypto/Makefile |  2 +-
 lib/crypto/utils.c  | 85 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 87 insertions(+), 74 deletions(-)
 create mode 100644 lib/crypto/utils.c
crypto/algapi.c +0 -71
index d1c99288af3e0d..5c69ff8e8fa5c1 100644
@@ -997,77 +997,6 @@ void crypto_inc(u8 *a, unsigned int size)
 }
 EXPORT_SYMBOL_GPL(crypto_inc);
 
-void __crypto_xor(u8 *dst, const u8 *src1, const u8 *src2, unsigned int len)
-{
-	int relalign = 0;
-
-	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) {
-		int size = sizeof(unsigned long);
-		int d = (((unsigned long)dst ^ (unsigned long)src1) |
-			 ((unsigned long)dst ^ (unsigned long)src2)) &
-			(size - 1);
-
-		relalign = d ? 1 << __ffs(d) : size;
-
-		/*
-		 * If we care about alignment, process as many bytes as
-		 * needed to advance dst and src to values whose alignments
-		 * equal their relative alignment. This will allow us to
-		 * process the remainder of the input using optimal strides.
-		 */
-		while (((unsigned long)dst & (relalign - 1)) && len > 0) {
-			*dst++ = *src1++ ^ *src2++;
-			len--;
-		}
-	}
-
-	while (IS_ENABLED(CONFIG_64BIT) && len >= 8 && !(relalign & 7)) {
-		if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) {
-			u64 l = get_unaligned((u64 *)src1) ^
-				get_unaligned((u64 *)src2);
-			put_unaligned(l, (u64 *)dst);
-		} else {
-			*(u64 *)dst = *(u64 *)src1 ^ *(u64 *)src2;
-		}
-		dst += 8;
-		src1 += 8;
-		src2 += 8;
-		len -= 8;
-	}
-
-	while (len >= 4 && !(relalign & 3)) {
-		if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) {
-			u32 l = get_unaligned((u32 *)src1) ^
-				get_unaligned((u32 *)src2);
-			put_unaligned(l, (u32 *)dst);
-		} else {
-			*(u32 *)dst = *(u32 *)src1 ^ *(u32 *)src2;
-		}
-		dst += 4;
-		src1 += 4;
-		src2 += 4;
-		len -= 4;
-	}
-
-	while (len >= 2 && !(relalign & 1)) {
-		if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) {
-			u16 l = get_unaligned((u16 *)src1) ^
-				get_unaligned((u16 *)src2);
-			put_unaligned(l, (u16 *)dst);
-		} else {
-			*(u16 *)dst = *(u16 *)src1 ^ *(u16 *)src2;
-		}
-		dst += 2;
-		src1 += 2;
-		src2 += 2;
-		len -= 2;
-	}
-
-	while (len--)
-		*dst++ = *src1++ ^ *src2++;
-}
-EXPORT_SYMBOL_GPL(__crypto_xor);
-
 unsigned int crypto_alg_extsize(struct crypto_alg *alg)
 {
 	return alg->cra_ctxsize +
lib/crypto/Kconfig +1 -2
index b09d9d6546cbc3..7e9683e9f5c636 100644
@@ -36,7 +36,7 @@ config CRYPTO_ARCH_HAVE_LIB_CHACHA
 
 config CRYPTO_LIB_CHACHA_GENERIC
 	tristate
-	select XOR_BLOCKS
+	select CRYPTO_LIB_UTILS
 	help
 	  This symbol can be depended upon by arch implementations of the
 	  ChaCha library interface that require the generic code as a
@@ -46,7 +46,6 @@ config CRYPTO_LIB_CHACHA_GENERIC
 
 config CRYPTO_LIB_CHACHA
 	tristate "ChaCha library interface"
-	depends on CRYPTO
 	depends on CRYPTO_ARCH_HAVE_LIB_CHACHA || !CRYPTO_ARCH_HAVE_LIB_CHACHA
 	select CRYPTO_LIB_CHACHA_GENERIC if CRYPTO_ARCH_HAVE_LIB_CHACHA=n
 	help
lib/crypto/Makefile +1 -1
index b956b3bae26aaf..c852f067ab0601 100644
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_CRYPTO_LIB_UTILS)			+= libcryptoutils.o
-libcryptoutils-y				:= memneq.o
+libcryptoutils-y				:= memneq.o utils.o
 
 # chacha is used by the /dev/random driver which is always builtin
 obj-y						+= chacha.o
lib/crypto/utils.c new file +85 -0

85 lines not on this page · raw message

Signature · 1 line
2.37.0
author maintainer
#3 lore raw

PATCH v2 3/3 crypto: lib - move crypto_simd_disabled_for_test into utils

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Move the definition of crypto_simd_disabled_for_test into lib/crypto/utils.c so that it can be accessed by library code.

This is needed when code that is shared between a traditional crypto API implementation and a library implementation is built-in, but CRYPTO_ALGAPI=m. The x86 blake2s previously was an example of this
(https://lore.kernel.org/linux-crypto/[email protected]/T/#u).
Although that case was resolved by removing the blake2s shash support, this problem could easily come back in the future, so let's address it.

---
 crypto/algapi.c    | 6 ------
 lib/crypto/utils.c | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)
crypto/algapi.c +0 -6
index 5c69ff8e8fa5c1..9377dae75b3def 100644
@@ -6,7 +6,6 @@
  */
 
 #include <crypto/algapi.h>
-#include <crypto/internal/simd.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/fips.h>
@@ -22,11 +21,6 @@
 
 static LIST_HEAD(crypto_template_list);
 
-#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
-DEFINE_PER_CPU(bool, crypto_simd_disabled_for_test);
-EXPORT_PER_CPU_SYMBOL_GPL(crypto_simd_disabled_for_test);
-#endif
-
 static inline void crypto_check_module_sig(struct module *mod)
 {
 	if (fips_enabled && mod && !module_sig_ok(mod))
lib/crypto/utils.c +6 -0
index f20bdb2ae88771..e7d99f150d4e61 100644
@@ -6,8 +6,14 @@
  */
 
 #include <crypto/algapi.h>
+#include <crypto/internal/simd.h>
 #include <asm/unaligned.h>
 
+#ifdef CONFIG_CRYPTO_MANAGER_EXTRA_TESTS
+DEFINE_PER_CPU(bool, crypto_simd_disabled_for_test);
+EXPORT_PER_CPU_SYMBOL_GPL(crypto_simd_disabled_for_test);
+#endif
+
 /*
  * XOR @len bytes from @src1 and @src2 together, writing the result to @dst
  * (which may alias one of the sources).  Don't call this directly; call
Signature · 1 line
2.37.0
#4 lore raw

Re: [PATCH v2 3/3] crypto: lib - move crypto_simd_disabled_for_test into utils

Hi Eric,

Quoted 11 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Move the definition of crypto_simd_disabled_for_test into lib/crypto/utils.c so that it can be accessed by library code.

This is needed when code that is shared between a traditional crypto API implementation and a library implementation is built-in, but CRYPTO_ALGAPI=m. The x86 blake2s previously was an example of this
(https://lore.kernel.org/linux-crypto/[email protected]/T/#u).
Although that case was resolved by removing the blake2s shash support, this problem could easily come back in the future, so let's address it.

I'm not sure I see the reason in general for a utility library rather than doing these piecemeal like the rest of lib functions. Why is crypto special here? But in particular to this patch: nothing is actually using crypto_simd_disabled_for_test in lib/crypto, right? So is this necessary?

Jason

author maintainer
#5 lore raw

Re: [PATCH v2 3/3] crypto: lib - move crypto_simd_disabled_for_test into utils

Quoted 15 lines · Jason A. Donenfeld

Hi Eric,

Quoted 11 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Move the definition of crypto_simd_disabled_for_test into lib/crypto/utils.c so that it can be accessed by library code.

This is needed when code that is shared between a traditional crypto API implementation and a library implementation is built-in, but CRYPTO_ALGAPI=m. The x86 blake2s previously was an example of this
(https://lore.kernel.org/linux-crypto/[email protected]/T/#u).
Although that case was resolved by removing the blake2s shash support, this problem could easily come back in the future, so let's address it.

Quoted 5 lines

I'm not sure I see the reason in general for a utility library rather than doing these piecemeal like the rest of lib functions. Why is crypto special here? But in particular to this patch: nothing is actually using crypto_simd_disabled_for_test in lib/crypto, right? So is this necessary?

Well, this is what Herbert wanted:
https://lore.kernel.org/r/[email protected]. It's subjective, but for now I think I prefer this approach too, since the utility functions are so small and are widely used. A whole module is overkill for just a few lines of code.

The commit message answers your second and third questions.

- Eric

author maintainer
#6 lore raw

Re: [PATCH v2 3/3] crypto: lib - move crypto_simd_disabled_for_test into utils

Quoted 22 lines · Eric Biggers
Quoted 15 lines · Jason A. Donenfeld

Hi Eric,

Quoted 11 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Move the definition of crypto_simd_disabled_for_test into lib/crypto/utils.c so that it can be accessed by library code.

This is needed when code that is shared between a traditional crypto API implementation and a library implementation is built-in, but CRYPTO_ALGAPI=m. The x86 blake2s previously was an example of this
(https://lore.kernel.org/linux-crypto/[email protected]/T/#u).
Although that case was resolved by removing the blake2s shash support, this problem could easily come back in the future, so let's address it.

Quoted 5 lines

I'm not sure I see the reason in general for a utility library rather than doing these piecemeal like the rest of lib functions. Why is crypto special here? But in particular to this patch: nothing is actually using crypto_simd_disabled_for_test in lib/crypto, right? So is this necessary?

Quoted 8 lines

Well, this is what Herbert wanted:
https://lore.kernel.org/r/[email protected]. It's subjective, but for now I think I prefer this approach too, since the utility functions are so small and are widely used. A whole module is overkill for just a few lines of code.

The commit message answers your second and third questions.

Herbert, any thoughts on this?

Note: I forgot to put a MODULE_LICENSE in the new module, so I'll need to resend this patchset even if there are no other issues.

- Eric

maintainer
#7 lore raw

Re: [PATCH v2 3/3] crypto: lib - move crypto_simd_disabled_for_test into utils

Hi Eric:

Quoted 2 lines · Eric Biggers

Herbert, any thoughts on this?

I'm happy to drop the simd patch for now as there are no users under lib/crypto.

Quoted 2 lines

Note: I forgot to put a MODULE_LICENSE in the new module, so I'll need to resend this patchset even if there are no other issues.

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