Skip to contentSkip to conversation

crypto: xor - move __crypto_xor into lib/

PATCH In review Eric Biggers 3 participants 3 messages Jul 9, 2022 – Jul 15, 2022

Subsystems (3)
Library code Crypto API Crypto library
lore
Share & tools
lore

Eric Biggers

author maintainer
#0 lore raw

PATCH 1/1 crypto: xor - move __crypto_xor into lib/

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 lib/xor.c, alongside lib/memneq.c where __crypto_memneq was recently moved.

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.

Signed-off-by Eric Biggers <[email protected]>
---
 crypto/Kconfig     |  1 +
 crypto/algapi.c    | 71 --------------------------------------
 lib/Kconfig        |  3 ++
 lib/Makefile       |  1 +
 lib/crypto/Kconfig |  3 +-
 lib/xor.c          | 85 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 91 insertions(+), 73 deletions(-)
 create mode 100644 lib/xor.c
crypto/Kconfig +1 -0
index 59489a300cd100..9145fdfa8a5bbd 100644
@@ -40,6 +40,7 @@ config CRYPTO_ALGAPI
 	  This option provides the API for cryptographic algorithms.
 
 config CRYPTO_ALGAPI2
+	select LIB_XOR
 	tristate
 
 config CRYPTO_AEAD
crypto/algapi.c +0 -71

77 lines not on this page · raw message

lib/Kconfig +3 -0
index eaaad4d85bf24b..4d2b09487442ea 100644
@@ -123,6 +123,9 @@ source "lib/crypto/Kconfig"
 config LIB_MEMNEQ
 	bool
 
+config LIB_XOR
+	bool
+
 config CRC_CCITT
 	tristate "CRC-CCITT functions"
 	help
lib/Makefile +1 -0
index f99bf61f8bbc67..ceeb3f913f1b57 100644
@@ -252,6 +252,7 @@ obj-$(CONFIG_SIGNATURE) += digsig.o
 
 lib-$(CONFIG_CLZ_TAB) += clz_tab.o
 lib-$(CONFIG_LIB_MEMNEQ) += memneq.o
+lib-$(CONFIG_LIB_XOR) += xor.o
 
 obj-$(CONFIG_GENERIC_STRNCPY_FROM_USER) += strncpy_from_user.o
 obj-$(CONFIG_GENERIC_STRNLEN_USER) += strnlen_user.o
lib/crypto/Kconfig +1 -2
index 2082af43d51fbe..8432e0e2a9ee0d 100644
@@ -33,7 +33,7 @@ config CRYPTO_ARCH_HAVE_LIB_CHACHA
 
 config CRYPTO_LIB_CHACHA_GENERIC
 	tristate
-	select XOR_BLOCKS
+	select LIB_XOR
 	help
 	  This symbol can be depended upon by arch implementations of the
 	  ChaCha library interface that require the generic code as a

@@ -43,7 +43,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/xor.c new file +85 -0

85 lines not on this page · raw message

base-commit: 79e6e2f3f3ff345947075341781e900e4f70db81

Signature · 1 line
2.37.0
maintainer Reviewed-by on v1 1/1
#1 lore raw

Hi Eric,

Quoted 9 lines · 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 lib/xor.c, alongside lib/memneq.c where __crypto_memneq was recently moved.

Quoted 3 lines

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.

Oh thank goodness. Excellent! Indeed untangling this knot will be a large improvement.

Reviewed-by Jason A. Donenfeld <[email protected]>

Jason

maintainer
#2 lore raw
In reply to Eric Biggers
Quoted 15 lines · 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 lib/xor.c, alongside lib/memneq.c where __crypto_memneq was recently moved.

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.

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

---

Quoted 8 lines
crypto/Kconfig     |  1 +
crypto/algapi.c    | 71 --------------------------------------
lib/Kconfig        |  3 ++
lib/Makefile       |  1 +
lib/crypto/Kconfig |  3 +-
lib/xor.c          | 85 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 91 insertions(+), 73 deletions(-)

create mode 100644 lib/xor.c

Now that there are two of these things (xor and memneq), please aggregate them into a module (either as separate files linked together or as a single file) and turn it into a tristate.

They should also be moved into lib/crypto together with their main users as lib is way too crowded as it is.

We could then revisit that simd variable and move it in too.

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