2022-07-25 18:38:20

by Eric Biggers

[permalink] [raw]
Subject: [PATCH v3 0/3] 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).

Changed v2 => v3:
- Added missing MODULE_LICENSE
- Dropped patch "crypto: lib - move crypto_simd_disabled_for_test
into utils"
- Added patch "crypto: lib - remove __HAVE_ARCH_CRYPTO_MEMNEQ"

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

crypto/Kconfig | 2 +-
crypto/algapi.c | 71 -------------------------------
lib/Kconfig | 3 --
lib/Makefile | 1 -
lib/crypto/Kconfig | 8 ++--
lib/crypto/Makefile | 3 ++
lib/{ => crypto}/memneq.c | 7 +---
lib/crypto/utils.c | 88 +++++++++++++++++++++++++++++++++++++++
8 files changed, 99 insertions(+), 84 deletions(-)
rename lib/{ => crypto}/memneq.c (98%)
create mode 100644 lib/crypto/utils.c


base-commit: 9d2bb9a74b2877f100637d6ab5685bcd33c69d44
--
2.37.0


2022-07-25 18:38:30

by Eric Biggers

[permalink] [raw]
Subject: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

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.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/algapi.c | 71 ------------------------------------
lib/crypto/Kconfig | 3 +-
lib/crypto/Makefile | 2 +-
lib/crypto/memneq.c | 2 --
lib/crypto/utils.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 90 insertions(+), 76 deletions(-)
create mode 100644 lib/crypto/utils.c

diff --git a/crypto/algapi.c b/crypto/algapi.c
index d1c99288af3e0d..5c69ff8e8fa5c1 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -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 +
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index b09d9d6546cbc3..7e9683e9f5c636 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -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
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index b956b3bae26aaf..c852f067ab0601 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -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
diff --git a/lib/crypto/memneq.c b/lib/crypto/memneq.c
index f2098318428478..d1e8c86fbb0fcf 100644
--- a/lib/crypto/memneq.c
+++ b/lib/crypto/memneq.c
@@ -175,5 +175,3 @@ noinline unsigned long __crypto_memneq(const void *a, const void *b,
EXPORT_SYMBOL(__crypto_memneq);

#endif /* __HAVE_ARCH_CRYPTO_MEMNEQ */
-
-MODULE_LICENSE("GPL");
diff --git a/lib/crypto/utils.c b/lib/crypto/utils.c
new file mode 100644
index 00000000000000..53230ab1b19576
--- /dev/null
+++ b/lib/crypto/utils.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Crypto library utility functions
+ *
+ * Copyright (c) 2006 Herbert Xu <[email protected]>
+ */
+
+#include <asm/unaligned.h>
+#include <crypto/algapi.h>
+#include <linux/module.h>
+
+/*
+ * 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
+ * crypto_xor() or crypto_xor_cpy() instead.
+ */
+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);
+
+MODULE_LICENSE("GPL");
--
2.37.0

2022-07-25 18:38:46

by Eric Biggers

[permalink] [raw]
Subject: [PATCH v3 3/3] crypto: lib - remove __HAVE_ARCH_CRYPTO_MEMNEQ

From: Eric Biggers <[email protected]>

No architecture actually defines this, so it's unneeded.

Signed-off-by: Eric Biggers <[email protected]>
---
lib/crypto/memneq.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/lib/crypto/memneq.c b/lib/crypto/memneq.c
index d1e8c86fbb0fcf..243d8677cc515d 100644
--- a/lib/crypto/memneq.c
+++ b/lib/crypto/memneq.c
@@ -63,8 +63,6 @@
#include <crypto/algapi.h>
#include <linux/module.h>

-#ifndef __HAVE_ARCH_CRYPTO_MEMNEQ
-
/* Generic path for arbitrary size */
static inline unsigned long
__crypto_memneq_generic(const void *a, const void *b, size_t size)
@@ -173,5 +171,3 @@ noinline unsigned long __crypto_memneq(const void *a, const void *b,
}
}
EXPORT_SYMBOL(__crypto_memneq);
-
-#endif /* __HAVE_ARCH_CRYPTO_MEMNEQ */
--
2.37.0

2022-07-25 18:45:32

by Eric Biggers

[permalink] [raw]
Subject: [PATCH v3 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.

Signed-off-by: Eric Biggers <[email protected]>
---
crypto/Kconfig | 2 +-
lib/Kconfig | 3 ---
lib/Makefile | 1 -
lib/crypto/Kconfig | 5 ++++-
lib/crypto/Makefile | 3 +++
lib/{ => crypto}/memneq.c | 5 ++++-
6 files changed, 12 insertions(+), 7 deletions(-)
rename lib/{ => crypto}/memneq.c (99%)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index e72bf3fc298cc5..7a10441998209d 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -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.

diff --git a/lib/Kconfig b/lib/Kconfig
index eaaad4d85bf24b..6a843639814fbf 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -120,9 +120,6 @@ config INDIRECT_IOMEM_FALLBACK

source "lib/crypto/Kconfig"

-config LIB_MEMNEQ
- bool
-
config CRC_CCITT
tristate "CRC-CCITT functions"
help
diff --git a/lib/Makefile b/lib/Makefile
index 67482f5ec0e899..0557be76c2565f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -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
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 9ff549f63540fa..b09d9d6546cbc3 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -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
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index 919cbb2c220d61..b956b3bae26aaf 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -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
diff --git a/lib/memneq.c b/lib/crypto/memneq.c
similarity index 99%
rename from lib/memneq.c
rename to lib/crypto/memneq.c
index fb11608b1ec1dd..f2098318428478 100644
--- a/lib/memneq.c
+++ b/lib/crypto/memneq.c
@@ -59,8 +59,9 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

-#include <crypto/algapi.h>
#include <asm/unaligned.h>
+#include <crypto/algapi.h>
+#include <linux/module.h>

#ifndef __HAVE_ARCH_CRYPTO_MEMNEQ

@@ -174,3 +175,5 @@ noinline unsigned long __crypto_memneq(const void *a, const void *b,
EXPORT_SYMBOL(__crypto_memneq);

#endif /* __HAVE_ARCH_CRYPTO_MEMNEQ */
+
+MODULE_LICENSE("GPL");
--
2.37.0

2022-07-25 22:07:53

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] crypto: lib - remove __HAVE_ARCH_CRYPTO_MEMNEQ

On Mon, Jul 25, 2022 at 11:36:36AM -0700, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> No architecture actually defines this, so it's unneeded.
>
> Signed-off-by: Eric Biggers <[email protected]>

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

Aside: out of curiosity, I wonder what was originally intended with
this, which magic arch-specific instructions were thought to be
potentially of aid.

2022-07-25 22:13:30

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] crypto: lib - create utils module and move __crypto_memneq into it

On Mon, Jul 25, 2022 at 11:36:34AM -0700, Eric Biggers wrote:
> 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.

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

2022-07-25 22:29:38

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

On Mon, Jul 25, 2022 at 11:36:35AM -0700, Eric Biggers wrote:
> 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.
>
> Signed-off-by: Eric Biggers <[email protected]>

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

With one small question:

> --- /dev/null
> +++ b/lib/crypto/utils.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Crypto library utility functions
> + *
> + * Copyright (c) 2006 Herbert Xu <[email protected]>

Didn't Ard basically write the crypto_xor function in its current form?
I seem to remember some pretty hardcore refactoring he did a while back.

Jason

2022-07-26 00:53:27

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] crypto: lib - remove __HAVE_ARCH_CRYPTO_MEMNEQ

On Tue, Jul 26, 2022 at 12:07:06AM +0200, Jason A. Donenfeld wrote:
> On Mon, Jul 25, 2022 at 11:36:36AM -0700, Eric Biggers wrote:
> > From: Eric Biggers <[email protected]>
> >
> > No architecture actually defines this, so it's unneeded.
> >
> > Signed-off-by: Eric Biggers <[email protected]>
>
> Reviewed-by: Jason A. Donenfeld <[email protected]>
>
> Aside: out of curiosity, I wonder what was originally intended with
> this, which magic arch-specific instructions were thought to be
> potentially of aid.

The original commit (6bf37e5aa90f) says:

Similarly to kernel library string functions, leave an option for future
even further optimized architecture specific assembler implementations.

But so far no one has bothered. It's already optimized to use 'long' accesses,
so I wouldn't expect it to get much faster with assembly.

- Eric

2022-07-26 01:06:47

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

On Tue, Jul 26, 2022 at 12:09:33AM +0200, Jason A. Donenfeld wrote:
> On Mon, Jul 25, 2022 at 11:36:35AM -0700, Eric Biggers wrote:
> > 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.
> >
> > Signed-off-by: Eric Biggers <[email protected]>
>
> Reviewed-by: Jason A. Donenfeld <[email protected]>
>
> With one small question:
>
> > --- /dev/null
> > +++ b/lib/crypto/utils.c
> > @@ -0,0 +1,88 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Crypto library utility functions
> > + *
> > + * Copyright (c) 2006 Herbert Xu <[email protected]>
>
> Didn't Ard basically write the crypto_xor function in its current form?
> I seem to remember some pretty hardcore refactoring he did a while back.
>
> Jason

I think that's fair to say, based on git blame. I just copied the copyright
statement from the top of crypto/algapi.c.

- Eric

2022-07-26 13:49:42

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

On Mon, 25 Jul 2022 at 15:09, Jason A. Donenfeld <[email protected]> wrote:
>
> On Mon, Jul 25, 2022 at 11:36:35AM -0700, Eric Biggers wrote:
> > 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.
> >
> > Signed-off-by: Eric Biggers <[email protected]>
>
> Reviewed-by: Jason A. Donenfeld <[email protected]>
>
> With one small question:
>
> > --- /dev/null
> > +++ b/lib/crypto/utils.c
> > @@ -0,0 +1,88 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Crypto library utility functions
> > + *
> > + * Copyright (c) 2006 Herbert Xu <[email protected]>
>
> Didn't Ard basically write the crypto_xor function in its current form?
> I seem to remember some pretty hardcore refactoring he did a while back.
>

Hi Jason,

Thanks for pointing this out. When I made those changes, I don't think
an authorship assertion for copyright purposes was appropriate for the
entire .c file (the FSF have some guidelines for this IIRC). It would
also be strange for me or Eric to suddenly introduce a (c) Linaro (or
ARM, not sure who my employer was at the time) at this point, so I
think we can just leave this as proposed by Eric.

2022-08-19 11:03:04

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] crypto: lib - create utils module

Eric Biggers <[email protected]> wrote:
> 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).
>
> Changed v2 => v3:
> - Added missing MODULE_LICENSE
> - Dropped patch "crypto: lib - move crypto_simd_disabled_for_test
> into utils"
> - Added patch "crypto: lib - remove __HAVE_ARCH_CRYPTO_MEMNEQ"
>
> Eric Biggers (3):
> crypto: lib - create utils module and move __crypto_memneq into it
> crypto: lib - move __crypto_xor into utils
> crypto: lib - remove __HAVE_ARCH_CRYPTO_MEMNEQ
>
> crypto/Kconfig | 2 +-
> crypto/algapi.c | 71 -------------------------------
> lib/Kconfig | 3 --
> lib/Makefile | 1 -
> lib/crypto/Kconfig | 8 ++--
> lib/crypto/Makefile | 3 ++
> lib/{ => crypto}/memneq.c | 7 +---
> lib/crypto/utils.c | 88 +++++++++++++++++++++++++++++++++++++++
> 8 files changed, 99 insertions(+), 84 deletions(-)
> rename lib/{ => crypto}/memneq.c (98%)
> create mode 100644 lib/crypto/utils.c
>
>
> base-commit: 9d2bb9a74b2877f100637d6ab5685bcd33c69d44

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

2022-08-26 04:49:52

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

Herbert, Jason, and Justin:

On Mon, Jul 25, 2022 at 11:36:35AM -0700, Eric Biggers wrote:
> diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
> index b09d9d6546cbc3..7e9683e9f5c636 100644
> --- a/lib/crypto/Kconfig
> +++ b/lib/crypto/Kconfig
> @@ -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

Just a heads up; the unnecessary selection of XOR_BLOCKS here (which got
backported to v5.10.120 and v5.15.45 by "lib/crypto: add prompts back to crypto
libraries") can be considered an actual bug, as it increases boot time on
systems that didn't have it selected before. This is because the code enabled
by XOR_BLOCKS (crypto/xor.c) runs a benchmark, which takes some time. It
doesn't take *that* long, but it got noticed as a regression nonetheless, and it
needs to be fixed. My patch series happens to have fixed this, but I didn't
have it mind that it was a real bug fix.

Herbert, any chance that you could send my patch series to Linus without waiting
for the next merge window, so that it can be backported?

- Eric

2022-08-26 04:50:29

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

On Thu, Aug 25, 2022 at 09:44:38PM -0700, Eric Biggers wrote:
.
> Herbert, any chance that you could send my patch series to Linus without waiting
> for the next merge window, so that it can be backported?

Sure, which patch fixes this?

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

2022-08-26 04:51:52

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

On Fri, Aug 26, 2022 at 12:46:25PM +0800, Herbert Xu wrote:
> On Thu, Aug 25, 2022 at 09:44:38PM -0700, Eric Biggers wrote:
> .
> > Herbert, any chance that you could send my patch series to Linus without waiting
> > for the next merge window, so that it can be backported?
>
> Sure, which patch fixes this?
>

The one I'm commenting on (patch 2 of 3). But patch 1 is needed as a
prerequisite.

- Eric

2022-08-26 04:53:20

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

On Thu, Aug 25, 2022 at 09:48:25PM -0700, Eric Biggers wrote:
>
> The one I'm commenting on (patch 2 of 3). But patch 1 is needed as a
> prerequisite.

Oh so it's just dropping the select? Can you please send me a
separate patch for the crypto tree with a Fixes header? I'll
deal with the merge conflict.

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

2022-08-26 05:10:37

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] crypto: lib - move __crypto_xor into utils

On Fri, Aug 26, 2022 at 12:52:53PM +0800, Herbert Xu wrote:
> On Thu, Aug 25, 2022 at 09:48:25PM -0700, Eric Biggers wrote:
> >
> > The one I'm commenting on (patch 2 of 3). But patch 1 is needed as a
> > prerequisite.
>
> Oh so it's just dropping the select? Can you please send me a
> separate patch for the crypto tree with a Fixes header? I'll
> deal with the merge conflict.
>

Done: https://lore.kernel.org/r/[email protected]

- Eric