| Message ID | 20190124163347.12653-1-ard.biesheuvel@linaro.org |
|---|---|
| Headers | show |
| Series | crypto: arm64/aes-ccm - various bug fixes | expand |
On Thu, Jan 24, 2019 at 05:33:44PM +0100, Ard Biesheuvel wrote: > Fix a couple of bugs found by Eric's new testing code, and another > issue found by inspection. > > Ard Biesheuvel (3): > crypto: arm64/aes-ccm - fix logical bug in AAD MAC handling > crypto: arm64/aes-ccm - fix bugs in non-NEON fallback routine > crypto: arm64/aes-ccm - don't use an atomic walk needlessly > > arch/arm64/crypto/aes-ce-ccm-core.S | 5 +++-- > arch/arm64/crypto/aes-ce-ccm-glue.c | 8 +++----- > 2 files changed, 6 insertions(+), 7 deletions(-) > Thanks Ard. For all three patches: Reviewed-by: Eric Biggers <ebiggers@kernel.org> However, can you add Cc stable to the first two? BTW: I notice the !may_use_simd() case is still not covered by the self-tests. Do you happen to have any suggestion for how to do it? If there is a way to make may_use_simd() temporarily return false, I can easily put a flag in 'struct testvec_config' that makes the self-tests sometimes use that context to execute the actual encryption/decryption/hashing. It looks like on arm64, wrapping the operation in local_irq_{disable,enable}() or kernel_fpu_{begin,end}() would be sufficient? But x86 it doesn't look as easy as x86 needs a context where 'in_interrupt() && !interrupted_user_mode() && !interrupted_kernel_fpu_idle()'. Likewise arm32 requires 'in_interrupt()'. It would be worth doing just for arm64, but ideally it would work on most/all architectures. - Eric
On Thu, Jan 24, 2019 at 10:42:18PM -0800, Eric Biggers wrote: > > Reviewed-by: Eric Biggers <ebiggers@kernel.org> > > However, can you add Cc stable to the first two? I will add them when I apply the patches. Cheers, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, 25 Jan 2019 at 07:47, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Thu, Jan 24, 2019 at 10:42:18PM -0800, Eric Biggers wrote: > > > > Reviewed-by: Eric Biggers <ebiggers@kernel.org> > > > > However, can you add Cc stable to the first two? > > I will add them when I apply the patches. > Thanks. Also, Greg and Sasha tend to pick anything with a fixes tag these days.
On Fri, 25 Jan 2019 at 07:42, Eric Biggers <ebiggers@kernel.org> wrote: > > On Thu, Jan 24, 2019 at 05:33:44PM +0100, Ard Biesheuvel wrote: > > Fix a couple of bugs found by Eric's new testing code, and another > > issue found by inspection. > > > > Ard Biesheuvel (3): > > crypto: arm64/aes-ccm - fix logical bug in AAD MAC handling > > crypto: arm64/aes-ccm - fix bugs in non-NEON fallback routine > > crypto: arm64/aes-ccm - don't use an atomic walk needlessly > > > > arch/arm64/crypto/aes-ce-ccm-core.S | 5 +++-- > > arch/arm64/crypto/aes-ce-ccm-glue.c | 8 +++----- > > 2 files changed, 6 insertions(+), 7 deletions(-) > > > > Thanks Ard. For all three patches: > > Reviewed-by: Eric Biggers <ebiggers@kernel.org> > > However, can you add Cc stable to the first two? > > BTW: I notice the !may_use_simd() case is still not covered by the self-tests. > Do you happen to have any suggestion for how to do it? If there is a way to > make may_use_simd() temporarily return false, I can easily put a flag in 'struct > testvec_config' that makes the self-tests sometimes use that context to execute > the actual encryption/decryption/hashing. > > It looks like on arm64, wrapping the operation in local_irq_{disable,enable}() > or kernel_fpu_{begin,end}() would be sufficient? But x86 it doesn't look as > easy as x86 needs a context where 'in_interrupt() && !interrupted_user_mode() && > !interrupted_kernel_fpu_idle()'. Likewise arm32 requires 'in_interrupt()'. > > It would be worth doing just for arm64, but ideally it would work on most/all > architectures. > I think the best way to address this is to [conditionally] wrap may_use_simd() into something cryptoapi specific, which is under the control of the test framework.
On Thu, Jan 24, 2019 at 05:33:44PM +0100, Ard Biesheuvel wrote: > Fix a couple of bugs found by Eric's new testing code, and another > issue found by inspection. > > Ard Biesheuvel (3): > crypto: arm64/aes-ccm - fix logical bug in AAD MAC handling > crypto: arm64/aes-ccm - fix bugs in non-NEON fallback routine > crypto: arm64/aes-ccm - don't use an atomic walk needlessly > > arch/arm64/crypto/aes-ce-ccm-core.S | 5 +++-- > arch/arm64/crypto/aes-ce-ccm-glue.c | 8 +++----- > 2 files changed, 6 insertions(+), 7 deletions(-) All applied. Thanks. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt