Re: [PATCH v3 0/4] crypto: aegis128 enhancements

On Mon, 30 Nov 2020 at 10:43, Ard Biesheuvel [off-list ref] wrote:

On Mon, 30 Nov 2020 at 10:37, Geert Uytterhoeven [off-list ref] wrote:

quoted

Hi Ard,

On Tue, Nov 17, 2020 at 2:38 PM Ard Biesheuvel [off-list ref] wrote:

quoted

This series supersedes [0] '[PATCH] crypto: aegis128/neon - optimize tail
block handling', which is included as patch #3 here, but hasn't been
modified substantially.

Patch #1 should probably go to -stable, even though aegis128 does not appear
to be widely used.

Patches #2 and #3 improve the SIMD code paths.

Patch #4 enables fuzz testing for the SIMD code by registering the generic
code as a separate driver if the SIMD code path is enabled.

Changes since v2:
- add Ondrej's ack to #1
- fix an issue spotted by Ondrej in #4 where the generic code path would still
  use some of the SIMD helpers

Cc: Ondrej Mosnacek <redacted>
Cc: Eric Biggers <ebiggers@kernel.org>

[0] https://lore.kernel.org/linux-crypto/20201107195516.13952-1-ardb@kernel.org/ (local)

Ard Biesheuvel (4):
  crypto: aegis128 - wipe plaintext and tag if decryption fails
  crypto: aegis128/neon - optimize tail block handling
  crypto: aegis128/neon - move final tag check to SIMD domain

crypto/aegis128-core.c: In function ‘crypto_aegis128_decrypt’:
crypto/aegis128-core.c:454:40: error: passing argument 2 of
‘crypto_aegis128_process_crypt’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
  454 |    crypto_aegis128_process_crypt(NULL, req, &walk,
      |                                        ^~~
      |                                        |
      |                                        struct aead_request *
crypto/aegis128-core.c:335:29: note: expected ‘struct skcipher_walk *’
but argument is of type ‘struct aead_request *’
  335 |       struct skcipher_walk *walk,
      |       ~~~~~~~~~~~~~~~~~~~~~~^~~~
crypto/aegis128-core.c:454:45: error: passing argument 3 of
‘crypto_aegis128_process_crypt’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
  454 |    crypto_aegis128_process_crypt(NULL, req, &walk,
      |                                             ^~~~~
      |                                             |
      |                                             struct skcipher_walk *
crypto/aegis128-core.c:336:14: note: expected ‘void (*)(struct
aegis_state *, u8 *, const u8 *, unsigned int)’ {aka ‘void (*)(struct
aegis_state *, unsigned char *, const unsigned char *, unsigned int)’}
but argument is of type ‘struct skcipher_walk *’
  336 |       void (*crypt)(struct aegis_state *state,
      |       ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  337 |              u8 *dst, const u8 *src,
      |              ~~~~~~~~~~~~~~~~~~~~~~~
  338 |              unsigned int size))
      |              ~~~~~~~~~~~~~~~~~~
crypto/aegis128-core.c:454:4: error: too many arguments to function
‘crypto_aegis128_process_crypt’
  454 |    crypto_aegis128_process_crypt(NULL, req, &walk,
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
crypto/aegis128-core.c:334:5: note: declared here
  334 | int crypto_aegis128_process_crypt(struct aegis_state *state,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[1]: *** [scripts/Makefile.build:283: crypto/aegis128-core.o] Error 1

quoted

  crypto: aegis128 - expose SIMD code path as separate driver

Fixes the above, but causes

ERROR: modpost: "crypto_aegis128_update_simd" [crypto/aegis128.ko] undefined!

as reported by noreply@ellerman.id.au for m68k/defconfig and
m68k/sun3_defconfig.
(neon depends on arm).

Thanks for the report.

It seems like GCC is not optimizing away calls to routines that are
unreachable. Which GCC version are you using?

Also, mind checking whether the below works around this?

diff --git a/crypto/aegis128-core.c b/crypto/aegis128-core.c
index 2b05f79475d3..89dc1c559689 100644
--- a/crypto/aegis128-core.c
+++ b/crypto/aegis128-core.c

@@ -89,7 +89,7 @@ static void crypto_aegis128_update_a(struct

aegis_state *state,
                                     const union aegis_block *msg,
                                     bool do_simd)
 {
-       if (do_simd) {
+       if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD) && do_simd) {
                crypto_aegis128_update_simd(state, msg);
                return;
        }

@@ -101,7 +101,7 @@ static void crypto_aegis128_update_a(struct

aegis_state *state,
 static void crypto_aegis128_update_u(struct aegis_state *state, const
void *msg,
                                     bool do_simd)
 {
-       if (do_simd) {
+       if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD) && do_simd) {
                crypto_aegis128_update_simd(state, msg);
                return;
        }

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel