| Message ID | 20220102204203.521148-1-Jason@zx2c4.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v6] lib/crypto: blake2s: include as built-in | expand |
On Sun, Jan 02, 2022 at 09:42:03PM +0100, Jason A. Donenfeld wrote: > In preparation for using blake2s in the RNG, we change the way that it > is wired-in to the build system. Instead of using ifdefs to select the > right symbol, we use weak symbols. And because ARM doesn't need the > generic implementation, we make the generic one default only if an arch > library doesn't need it already, and then have arch libraries that do > need it opt-in. > > Acked-by: Ard Biesheuvel <ardb@kernel.org> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: linux-kbuild@vger.kernel.org > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: linux-crypto@vger.kernel.org > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > Herbert - As mentioned with the vPrev, I intend to take this via the > crng/random.git tree, since it forms a dependency and I'd like to send a > pull early in 5.17 cycle. At this point I think we should push this through crypto. The changes are too invasive with respect to the crypto Kconfig files. > diff --git a/crypto/Kconfig b/crypto/Kconfig > index 285f82647d2b..b7a2e50dcbc8 100644 > --- a/crypto/Kconfig > +++ b/crypto/Kconfig > @@ -702,7 +702,7 @@ config CRYPTO_BLAKE2S > See https://blake2.net for further information. > > config CRYPTO_BLAKE2S_X86 > - tristate "BLAKE2s digest algorithm (x86 accelerated version)" > + bool "BLAKE2s digest algorithm (x86 accelerated version)" > depends on X86 && 64BIT > select CRYPTO_LIB_BLAKE2S_GENERIC > select CRYPTO_ARCH_HAVE_LIB_BLAKE2S This will break when CRYPTO is disabled because the x86 crypto glue code depends on the crypto subsystem. Cheers,
On 1/3/22, Herbert Xu <herbert@gondor.apana.org.au> wrote: > At this point I think we should push this through crypto. The > changes are too invasive with respect to the crypto Kconfig files. Ugh, can we please not? That will really make things much harder and more annoying for me. I have an early pull planned, and you'll quickly be able to rebase on top of it. It also doesn't appear to conflict with anything you have queued up. Please, I would really appreciate some straight forward linearity here, and I don't think my taking it will negatively impact the flow. > >> diff --git a/crypto/Kconfig b/crypto/Kconfig >> index 285f82647d2b..b7a2e50dcbc8 100644 >> --- a/crypto/Kconfig >> +++ b/crypto/Kconfig >> @@ -702,7 +702,7 @@ config CRYPTO_BLAKE2S >> See https://blake2.net for further information. >> >> config CRYPTO_BLAKE2S_X86 >> - tristate "BLAKE2s digest algorithm (x86 accelerated version)" >> + bool "BLAKE2s digest algorithm (x86 accelerated version)" >> depends on X86 && 64BIT >> select CRYPTO_LIB_BLAKE2S_GENERIC >> select CRYPTO_ARCH_HAVE_LIB_BLAKE2S > > This will break when CRYPTO is disabled because the x86 crypto > glue code depends on the crypto subsystem. That snippet is inside an 'if CRYPTO' block, so it can't be selected without CRYPTO being enabled.
On Mon, Jan 03, 2022 at 04:45:10AM +0100, Jason A. Donenfeld wrote: > > Ugh, can we please not? That will really make things much harder and > more annoying for me. I have an early pull planned, and you'll quickly > be able to rebase on top of it. It also doesn't appear to conflict > with anything you have queued up. Please, I would really appreciate > some straight forward linearity here, and I don't think my taking it > will negatively impact the flow. Your patches as they stand will break the crypto tree. So that's why they should not go in without the proper changes. > That snippet is inside an 'if CRYPTO' block, so it can't be selected > without CRYPTO being enabled. No CONFIG_CRYPTO is not the issue. This depends on specific bits of the Crypto API such as CRYPTO_HASH. Simply selecting it is also not acceptable because you will be forcing all of the Crypto API into vmlinux even though none of it is required by /dev/random. Cheers,
Hi Herbert, On Mon, Jan 3, 2022 at 5:07 AM Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Mon, Jan 03, 2022 at 04:45:10AM +0100, Jason A. Donenfeld wrote: > > > > Ugh, can we please not? That will really make things much harder and > > more annoying for me. I have an early pull planned, and you'll quickly > > be able to rebase on top of it. It also doesn't appear to conflict > > with anything you have queued up. Please, I would really appreciate > > some straight forward linearity here, and I don't think my taking it > > will negatively impact the flow. > > Your patches as they stand will break the crypto tree. So > that's why they should not go in without the proper changes. Okay, I'll try to fix them up so that they don't break the crypto tree, and given below, I think I should be able to do this with fewer changes to some of the Kconfig, which will hopefully address your concerns and enable me to take this patch so that things are a bit more straightforward. > > > That snippet is inside an 'if CRYPTO' block, so it can't be selected > > without CRYPTO being enabled. > > No CONFIG_CRYPTO is not the issue. This depends on specific > bits of the Crypto API such as CRYPTO_HASH. Simply selecting > it is also not acceptable because you will be forcing all of the > Crypto API into vmlinux even though none of it is required by > /dev/random. Thanks, I think I see what your concern is now. I'll take a stab at addressing that. Jason
diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig index 2b575792363e..f1bcf804b8b5 100644 --- a/arch/arm/crypto/Kconfig +++ b/arch/arm/crypto/Kconfig @@ -63,7 +63,7 @@ config CRYPTO_SHA512_ARM using optimized ARM assembler and NEON, when available. config CRYPTO_BLAKE2S_ARM - tristate "BLAKE2s digest algorithm (ARM)" + bool "BLAKE2s digest algorithm (ARM)" select CRYPTO_ARCH_HAVE_LIB_BLAKE2S help BLAKE2s digest algorithm optimized with ARM scalar instructions. This diff --git a/arch/arm/crypto/blake2s-core.S b/arch/arm/crypto/blake2s-core.S index 86345751bbf3..df40e46601f1 100644 --- a/arch/arm/crypto/blake2s-core.S +++ b/arch/arm/crypto/blake2s-core.S @@ -167,8 +167,8 @@ .endm // -// void blake2s_compress_arch(struct blake2s_state *state, -// const u8 *block, size_t nblocks, u32 inc); +// void blake2s_compress(struct blake2s_state *state, +// const u8 *block, size_t nblocks, u32 inc); // // Only the first three fields of struct blake2s_state are used: // u32 h[8]; (inout) @@ -176,7 +176,7 @@ // u32 f[2]; (in) // .align 5 -ENTRY(blake2s_compress_arch) +ENTRY(blake2s_compress) push {r0-r2,r4-r11,lr} // keep this an even number .Lnext_block: @@ -303,4 +303,4 @@ ENTRY(blake2s_compress_arch) str r3, [r12], #4 bne 1b b .Lcopy_block_done -ENDPROC(blake2s_compress_arch) +ENDPROC(blake2s_compress) diff --git a/arch/arm/crypto/blake2s-glue.c b/arch/arm/crypto/blake2s-glue.c index f2cc1e5fc9ec..09d3a0cabd2c 100644 --- a/arch/arm/crypto/blake2s-glue.c +++ b/arch/arm/crypto/blake2s-glue.c @@ -11,17 +11,17 @@ #include <linux/module.h> /* defined in blake2s-core.S */ -EXPORT_SYMBOL(blake2s_compress_arch); +EXPORT_SYMBOL(blake2s_compress); static int crypto_blake2s_update_arm(struct shash_desc *desc, const u8 *in, unsigned int inlen) { - return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch); + return crypto_blake2s_update(desc, in, inlen, blake2s_compress); } static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out) { - return crypto_blake2s_final(desc, out, blake2s_compress_arch); + return crypto_blake2s_final(desc, out, blake2s_compress); } #define BLAKE2S_ALG(name, driver_name, digest_size) \ diff --git a/arch/x86/crypto/blake2s-glue.c b/arch/x86/crypto/blake2s-glue.c index a40365ab301e..ef91a3167d27 100644 --- a/arch/x86/crypto/blake2s-glue.c +++ b/arch/x86/crypto/blake2s-glue.c @@ -28,9 +28,8 @@ asmlinkage void blake2s_compress_avx512(struct blake2s_state *state, static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_ssse3); static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_avx512); -void blake2s_compress_arch(struct blake2s_state *state, - const u8 *block, size_t nblocks, - const u32 inc) +void blake2s_compress(struct blake2s_state *state, const u8 *block, + size_t nblocks, const u32 inc) { /* SIMD disables preemption, so relax after processing each page. */ BUILD_BUG_ON(SZ_4K / BLAKE2S_BLOCK_SIZE < 8); @@ -56,17 +55,17 @@ void blake2s_compress_arch(struct blake2s_state *state, block += blocks * BLAKE2S_BLOCK_SIZE; } while (nblocks); } -EXPORT_SYMBOL(blake2s_compress_arch); +EXPORT_SYMBOL(blake2s_compress); static int crypto_blake2s_update_x86(struct shash_desc *desc, const u8 *in, unsigned int inlen) { - return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch); + return crypto_blake2s_update(desc, in, inlen, blake2s_compress); } static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out) { - return crypto_blake2s_final(desc, out, blake2s_compress_arch); + return crypto_blake2s_final(desc, out, blake2s_compress); } #define BLAKE2S_ALG(name, driver_name, digest_size) \ diff --git a/crypto/Kconfig b/crypto/Kconfig index 285f82647d2b..b7a2e50dcbc8 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -702,7 +702,7 @@ config CRYPTO_BLAKE2S See https://blake2.net for further information. config CRYPTO_BLAKE2S_X86 - tristate "BLAKE2s digest algorithm (x86 accelerated version)" + bool "BLAKE2s digest algorithm (x86 accelerated version)" depends on X86 && 64BIT select CRYPTO_LIB_BLAKE2S_GENERIC select CRYPTO_ARCH_HAVE_LIB_BLAKE2S @@ -1919,9 +1919,10 @@ config CRYPTO_STATS config CRYPTO_HASH_INFO bool -source "lib/crypto/Kconfig" source "drivers/crypto/Kconfig" source "crypto/asymmetric_keys/Kconfig" source "certs/Kconfig" endif # if CRYPTO + +source "lib/crypto/Kconfig" diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 6cccc3dc00bc..b2a4f998c180 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -81,7 +81,6 @@ config WIREGUARD select CRYPTO select CRYPTO_LIB_CURVE25519 select CRYPTO_LIB_CHACHA20POLY1305 - select CRYPTO_LIB_BLAKE2S select CRYPTO_CHACHA20_X86_64 if X86 && 64BIT select CRYPTO_POLY1305_X86_64 if X86 && 64BIT select CRYPTO_BLAKE2S_X86 if X86 && 64BIT diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h index 8e50d487500f..d39cfa0d333e 100644 --- a/include/crypto/internal/blake2s.h +++ b/include/crypto/internal/blake2s.h @@ -11,11 +11,11 @@ #include <crypto/internal/hash.h> #include <linux/string.h> -void blake2s_compress_generic(struct blake2s_state *state,const u8 *block, +void blake2s_compress_generic(struct blake2s_state *state, const u8 *block, size_t nblocks, const u32 inc); -void blake2s_compress_arch(struct blake2s_state *state,const u8 *block, - size_t nblocks, const u32 inc); +void blake2s_compress(struct blake2s_state *state, const u8 *block, + size_t nblocks, const u32 inc); bool blake2s_selftest(void); diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig index 545ccbddf6a1..0f27976b5038 100644 --- a/lib/crypto/Kconfig +++ b/lib/crypto/Kconfig @@ -9,14 +9,14 @@ config CRYPTO_LIB_ARC4 tristate config CRYPTO_ARCH_HAVE_LIB_BLAKE2S - tristate + bool help Declares whether the architecture provides an arch-specific accelerated implementation of the Blake2s library interface, either builtin or as a module. config CRYPTO_LIB_BLAKE2S_GENERIC - tristate + def_bool !CRYPTO_ARCH_HAVE_LIB_BLAKE2S help This symbol can be depended upon by arch implementations of the Blake2s library interface that require the generic code as a @@ -24,15 +24,6 @@ config CRYPTO_LIB_BLAKE2S_GENERIC implementation is enabled, this implementation serves the users of CRYPTO_LIB_BLAKE2S. -config CRYPTO_LIB_BLAKE2S - tristate "BLAKE2s hash function library" - depends on CRYPTO_ARCH_HAVE_LIB_BLAKE2S || !CRYPTO_ARCH_HAVE_LIB_BLAKE2S - select CRYPTO_LIB_BLAKE2S_GENERIC if CRYPTO_ARCH_HAVE_LIB_BLAKE2S=n - help - Enable the Blake2s library interface. This interface may be fulfilled - by either the generic implementation or an arch-specific one, if one - is available and enabled. - config CRYPTO_ARCH_HAVE_LIB_CHACHA tristate help diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile index 73205ed269ba..ed43a41f2dcc 100644 --- a/lib/crypto/Makefile +++ b/lib/crypto/Makefile @@ -10,11 +10,10 @@ libaes-y := aes.o obj-$(CONFIG_CRYPTO_LIB_ARC4) += libarc4.o libarc4-y := arc4.o -obj-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC) += libblake2s-generic.o -libblake2s-generic-y += blake2s-generic.o - -obj-$(CONFIG_CRYPTO_LIB_BLAKE2S) += libblake2s.o -libblake2s-y += blake2s.o +# blake2s is used by the /dev/random driver which is always builtin +obj-y += libblake2s.o +libblake2s-y := blake2s.o +libblake2s-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC) += blake2s-generic.o obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305) += libchacha20poly1305.o libchacha20poly1305-y += chacha20poly1305.o diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c index 04ff8df24513..75ccb3e633e6 100644 --- a/lib/crypto/blake2s-generic.c +++ b/lib/crypto/blake2s-generic.c @@ -37,7 +37,11 @@ static inline void blake2s_increment_counter(struct blake2s_state *state, state->t[1] += (state->t[0] < inc); } -void blake2s_compress_generic(struct blake2s_state *state,const u8 *block, +void blake2s_compress(struct blake2s_state *state, const u8 *block, + size_t nblocks, const u32 inc) + __weak __alias(blake2s_compress_generic); + +void blake2s_compress_generic(struct blake2s_state *state, const u8 *block, size_t nblocks, const u32 inc) { u32 m[16]; diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c index 4055aa593ec4..93f2ae051370 100644 --- a/lib/crypto/blake2s.c +++ b/lib/crypto/blake2s.c @@ -16,12 +16,6 @@ #include <linux/init.h> #include <linux/bug.h> -#if IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S) -# define blake2s_compress blake2s_compress_arch -#else -# define blake2s_compress blake2s_compress_generic -#endif - void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen) { __blake2s_update(state, in, inlen, blake2s_compress);