Skip to contentSkip to conversation

crypto: aegis128 - avoid spurious references crypto_aegis128_update_simd

PATCH In review Ard Biesheuvel 3 participants 5 messages Nov 30, 2020 – Dec 4, 2020 Crypto API

lore
Share & tools
lore

Ard Biesheuvel

author
#0 lore raw

PATCH 1/1 crypto: aegis128 - avoid spurious references crypto_aegis128_update_simd

Geert reports that builds where CONFIG_CRYPTO_AEGIS128_SIMD is not set may still emit references to crypto_aegis128_update_simd(), which cannot be satisfied and therefore break the build. These references only exist in functions that can be optimized away, but apparently, the compiler is not always able to prove this.

So add some explicit checks for CONFIG_CRYPTO_AEGIS128_SIMD to help the compiler figure this out.

Tested-by Geert Uytterhoeven <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 crypto/aegis128-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
crypto/aegis128-core.c +2 -2
index 2b05f79475d3..89dc1c559689 100644
@@ -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;
 	}
Signature · 1 line
2.17.1
#1 lore raw

Hi Ard,

Quoted 5 lines · Ard Biesheuvel

Geert reports that builds where CONFIG_CRYPTO_AEGIS128_SIMD is not set may still emit references to crypto_aegis128_update_simd(), which cannot be satisfied and therefore break the build. These references only exist in functions that can be optimized away, but apparently, the compiler is not always able to prove this.

The code is not unreachable. Both crypto_aegis128_encrypt_simd() and crypto_aegis128_decrypt_simd() call crypto_aegis128_process_ad(..., true);

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author
#2 lore raw
Quoted 4 lines · Geert Uytterhoeven

Hi Ard,

On Mon, Nov 30, 2020 at 1:26 PM Ard Biesheuvel <[email protected]> wrote:

Quoted 9 lines
Quoted 5 lines

Geert reports that builds where CONFIG_CRYPTO_AEGIS128_SIMD is not set may still emit references to crypto_aegis128_update_simd(), which cannot be satisfied and therefore break the build. These references only exist in functions that can be optimized away, but apparently, the compiler is not always able to prove this.

The code is not unreachable. Both crypto_aegis128_encrypt_simd() and crypto_aegis128_decrypt_simd() call crypto_aegis128_process_ad(..., true);

Those functions themselves can be optimized away too, as well as struct aead_alg crypto_aegis128_alg_simd, which is the only thing that refers to those functions, and is itself only referenced inside a 'if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD))' conditional block. This is why it works fine most of the time.

#3 lore raw

Hi Ard,

Quoted 12 lines · Ard Biesheuvel
Quoted 10 lines · Geert Uytterhoeven
Quoted 5 lines · Ard Biesheuvel

Geert reports that builds where CONFIG_CRYPTO_AEGIS128_SIMD is not set may still emit references to crypto_aegis128_update_simd(), which cannot be satisfied and therefore break the build. These references only exist in functions that can be optimized away, but apparently, the compiler is not always able to prove this.

The code is not unreachable. Both crypto_aegis128_encrypt_simd() and crypto_aegis128_decrypt_simd() call crypto_aegis128_process_ad(..., true);

Quoted 5 lines

Those functions themselves can be optimized away too, as well as struct aead_alg crypto_aegis128_alg_simd, which is the only thing that refers to those functions, and is itself only referenced inside a 'if (IS_ENABLED(CONFIG_CRYPTO_AEGIS128_SIMD))' conditional block. This is why it works fine most of the time.

I stand corrected: I missed the conditional registration of crypto_aegis128_alg_simd.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
maintainer
#4 lore raw
In reply to Ard Biesheuvel
Quoted 6 lines · Ard Biesheuvel

Geert reports that builds where CONFIG_CRYPTO_AEGIS128_SIMD is not set may still emit references to crypto_aegis128_update_simd(), which cannot be satisfied and therefore break the build. These references only exist in functions that can be optimized away, but apparently, the compiler is not always able to prove this.

Quoted 8 lines

So add some explicit checks for CONFIG_CRYPTO_AEGIS128_SIMD to help the compiler figure this out.

Tested-by Geert Uytterhoeven <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 crypto/aegis128-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch applied. 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