| Message ID | 20210512184439.8778-1-ardb@kernel.org |
|---|---|
| Headers | show |
| Series | running kernel mode SIMD with softirqs disabled | expand |
On Wed, May 12, 2021 at 08:44:34PM +0200, Ard Biesheuvel wrote: > In order to ensure that kernel mode SIMD routines will not need a scalar > fallback if they run with softirqs disabled, disallow any use of the > AEAD encrypt and decrypt routines from outside of task or softirq context. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > crypto/aead.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/crypto/aead.c b/crypto/aead.c > index 16991095270d..b5304b3d3314 100644 > --- a/crypto/aead.c > +++ b/crypto/aead.c > @@ -87,6 +87,11 @@ int crypto_aead_encrypt(struct aead_request *req) > unsigned int cryptlen = req->cryptlen; > int ret; > > + if (!(alg->cra_flags & CRYPTO_ALG_ASYNC) && > + WARN_ONCE(!in_task() && !in_serving_softirq(), > + "synchronous call from invalid context\n")) > + return -EBUSY; > + > crypto_stats_get(alg); > if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY) > ret = -ENOKEY; > @@ -104,6 +109,11 @@ int crypto_aead_decrypt(struct aead_request *req) > unsigned int cryptlen = req->cryptlen; > int ret; > > + if (!(alg->cra_flags & CRYPTO_ALG_ASYNC) && > + WARN_ONCE(!in_task() && !in_serving_softirq(), > + "synchronous call from invalid context\n")) > + return -EBUSY; > + > crypto_stats_get(alg); > if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY) > ret = -ENOKEY; This probably should go after crypto_stats_get() so that the error gets counted in the stats (if stats are enabled) -- analogous to how the ENOKEY error is counted. Likewise for the skcipher patch. - Eric
On Wed, May 12, 2021 at 08:44:32PM +0200, Ard Biesheuvel wrote: > This is a follow-up to [0], but given that the arm64 architectural > pieces have been merged for arm64, the only remaining changes are crypto > specific. Therefore, the audience has been reduced to those people who > are likely to care about these specifics. > > Patch #1 addresses an issue in the skcipher walker which doesn't handle > zero sized AEAD inputs entirely consistently, which is uncovered by the > change in patch #7. > > Patches #2 and #3 add some sanity checks to the public AEAD and skcipher > APIs to limit their availibility to either task or softirq context > (which is the only way in which they are currently being used). Adding > this restriction permits the arm64 crypto code to get rid of all scalar > fallbacks, given that on this architecture, softirqs are no longer > served while the SIMD unit is being used in kernel mode, which means > that the scalar fallbacks are never needed. These are removed in the > remaining 4 patches. > > [0] https://lore.kernel.org/linux-arm-kernel/20210302090118.30666-1-ardb@kernel.org/ Did you check whether any updates to the self-tests in testmgr.c are warranted? Specifically, is disabling the use of SIMD for testing still something that makes sense? - Eric
On Wed, 12 May 2021 at 22:06, Eric Biggers <ebiggers@kernel.org> wrote: > > On Wed, May 12, 2021 at 08:44:34PM +0200, Ard Biesheuvel wrote: > > In order to ensure that kernel mode SIMD routines will not need a scalar > > fallback if they run with softirqs disabled, disallow any use of the > > AEAD encrypt and decrypt routines from outside of task or softirq context. > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > crypto/aead.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/crypto/aead.c b/crypto/aead.c > > index 16991095270d..b5304b3d3314 100644 > > --- a/crypto/aead.c > > +++ b/crypto/aead.c > > @@ -87,6 +87,11 @@ int crypto_aead_encrypt(struct aead_request *req) > > unsigned int cryptlen = req->cryptlen; > > int ret; > > > > + if (!(alg->cra_flags & CRYPTO_ALG_ASYNC) && > > + WARN_ONCE(!in_task() && !in_serving_softirq(), > > + "synchronous call from invalid context\n")) > > + return -EBUSY; > > + > > crypto_stats_get(alg); > > if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY) > > ret = -ENOKEY; > > @@ -104,6 +109,11 @@ int crypto_aead_decrypt(struct aead_request *req) > > unsigned int cryptlen = req->cryptlen; > > int ret; > > > > + if (!(alg->cra_flags & CRYPTO_ALG_ASYNC) && > > + WARN_ONCE(!in_task() && !in_serving_softirq(), > > + "synchronous call from invalid context\n")) > > + return -EBUSY; > > + > > crypto_stats_get(alg); > > if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY) > > ret = -ENOKEY; > > This probably should go after crypto_stats_get() so that the error gets counted > in the stats (if stats are enabled) -- analogous to how the ENOKEY error is > counted. > > Likewise for the skcipher patch. > Good point, I'll fix that
On Wed, 12 May 2021 at 22:11, Eric Biggers <ebiggers@kernel.org> wrote: > > On Wed, May 12, 2021 at 08:44:32PM +0200, Ard Biesheuvel wrote: > > This is a follow-up to [0], but given that the arm64 architectural > > pieces have been merged for arm64, the only remaining changes are crypto > > specific. Therefore, the audience has been reduced to those people who > > are likely to care about these specifics. > > > > Patch #1 addresses an issue in the skcipher walker which doesn't handle > > zero sized AEAD inputs entirely consistently, which is uncovered by the > > change in patch #7. > > > > Patches #2 and #3 add some sanity checks to the public AEAD and skcipher > > APIs to limit their availibility to either task or softirq context > > (which is the only way in which they are currently being used). Adding > > this restriction permits the arm64 crypto code to get rid of all scalar > > fallbacks, given that on this architecture, softirqs are no longer > > served while the SIMD unit is being used in kernel mode, which means > > that the scalar fallbacks are never needed. These are removed in the > > remaining 4 patches. > > > > [0] https://lore.kernel.org/linux-arm-kernel/20210302090118.30666-1-ardb@kernel.org/ > > Did you check whether any updates to the self-tests in testmgr.c are warranted? > Specifically, is disabling the use of SIMD for testing still something that > makes sense? > The situation is not ideal, but I am not sure what we can do about this: the scalar fallbacks are gone, which means that the SIMD unit will be used in the test even if testmgr attempts to disable it. But keeping the scalar fallbacks just for the test suite makes no sense either. So I don't think we should change anything, other than perhaps document this somewhere (any suggestions on a place to put that) Note that the library routines, as well as shashes (which are sometimes exposed via library routines, e.g., CRC-T10DIF and CRC-32, and maybe others) are different, which is why their scalar fallbacks are retained. There, we need the testmgr to override SIMD availability to ensure that combinations of the SIMD and scalar code are tested.
This is a follow-up to [0], but given that the arm64 architectural pieces have been merged for arm64, the only remaining changes are crypto specific. Therefore, the audience has been reduced to those people who are likely to care about these specifics. Patch #1 addresses an issue in the skcipher walker which doesn't handle zero sized AEAD inputs entirely consistently, which is uncovered by the change in patch #7. Patches #2 and #3 add some sanity checks to the public AEAD and skcipher APIs to limit their availibility to either task or softirq context (which is the only way in which they are currently being used). Adding this restriction permits the arm64 crypto code to get rid of all scalar fallbacks, given that on this architecture, softirqs are no longer served while the SIMD unit is being used in kernel mode, which means that the scalar fallbacks are never needed. These are removed in the remaining 4 patches. [0] https://lore.kernel.org/linux-arm-kernel/20210302090118.30666-1-ardb@kernel.org/ Ard Biesheuvel (7): crypto: handle zero sized AEAD inputs correctly crypto: aead - disallow en/decrypt for non-task or non-softirq context crypto: skcipher - disallow en/decrypt for non-task or non-softirq context crypto: arm64/gcm-aes-ce - remove non-SIMD fallback path crypto: arm64/aes-neonbs - stop using SIMD helper for skciphers crypto: arm64/aes-ce - stop using SIMD helper for skciphers crypto: arm64/aes-ccm - remove non-SIMD fallback path arch/arm64/crypto/Kconfig | 6 - arch/arm64/crypto/aes-ce-ccm-core.S | 1 + arch/arm64/crypto/aes-ce-ccm-glue.c | 183 +++++------------ arch/arm64/crypto/aes-glue.c | 102 ++-------- arch/arm64/crypto/aes-neonbs-glue.c | 122 +----------- arch/arm64/crypto/ghash-ce-glue.c | 209 +++++--------------- crypto/aead.c | 10 + crypto/skcipher.c | 12 ++ 8 files changed, 148 insertions(+), 497 deletions(-)