| Message ID | 20201130122620.16640-1-ardb@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: aegis128 - avoid spurious references crypto_aegis128_update_simd | expand |
Hi Ard, On Mon, Nov 30, 2020 at 1:26 PM Ard Biesheuvel <ardb@kernel.org> wrote: > 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
On Mon, 30 Nov 2020 at 13:42, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Ard, > > On Mon, Nov 30, 2020 at 1:26 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > 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.
Hi Ard, On Mon, Nov 30, 2020 at 1:47 PM Ard Biesheuvel <ardb@kernel.org> wrote: > On Mon, 30 Nov 2020 at 13:42, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Mon, Nov 30, 2020 at 1:26 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > > 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. I stand corrected: I missed the conditional registration of crypto_aegis128_alg_simd. Gr{oetje,eeting}s, Geert
On Mon, Nov 30, 2020 at 01:26:20PM +0100, Ard Biesheuvel wrote: > 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 <geert@linux-m68k.org> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > crypto/aegis128-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Patch applied. Thanks.
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; }