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 <[email protected]>
---
crypto/aead.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/crypto/aead.c b/crypto/aead.c
index 16991095270d..141c9369b02a 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -88,7 +88,11 @@ int crypto_aead_encrypt(struct aead_request *req)
int ret;
crypto_stats_get(alg);
- if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
+ if (!(alg->cra_flags & CRYPTO_ALG_ASYNC) &&
+ WARN_ONCE(!in_task() && !in_serving_softirq(),
+ "synchronous call from invalid context\n"))
+ ret = -EBUSY;
+ else if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
ret = -ENOKEY;
else
ret = crypto_aead_alg(aead)->encrypt(req);
@@ -105,7 +109,11 @@ int crypto_aead_decrypt(struct aead_request *req)
int ret;
crypto_stats_get(alg);
- if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
+ if (!(alg->cra_flags & CRYPTO_ALG_ASYNC) &&
+ WARN_ONCE(!in_task() && !in_serving_softirq(),
+ "synchronous call from invalid context\n"))
+ ret = -EBUSY;
+ else if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
ret = -ENOKEY;
else if (req->cryptlen < crypto_aead_authsize(aead))
ret = -EINVAL;
--
2.20.1
On Wed, May 19, 2021 at 01:22:34PM +0200, Ard Biesheuvel wrote:
>
> crypto_stats_get(alg);
> - if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
> + if (!(alg->cra_flags & CRYPTO_ALG_ASYNC) &&
> + WARN_ONCE(!in_task() && !in_serving_softirq(),
> + "synchronous call from invalid context\n"))
> + ret = -EBUSY;
I don't think we've ever supported crypto in hard IRQ contexts.
So this should be done regardless of ASYNC.
Then again, do we really need this since the assumption has
always been that the crypto API can only be invoked in user or
softirq context?
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, May 19, 2021 at 01:36:37PM +0200, Ard Biesheuvel wrote:
>
> So if we do need to check this, we should check it here. If we don't,
> then we can drop these patches.
Historically other things would break in nasty ways if you tried
to do crypto in hard IRQ contexts, e.g., overwritten kmap slots
back when we had individual slots for each context, but I don't
think we've ever found anyone crazy enough to do that to warrant
a run-time check.
I'd just leave it out for now.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, 19 May 2021 at 13:29, Herbert Xu <[email protected]> wrote:
>
> On Wed, May 19, 2021 at 01:22:34PM +0200, Ard Biesheuvel wrote:
> >
> > crypto_stats_get(alg);
> > - if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
> > + if (!(alg->cra_flags & CRYPTO_ALG_ASYNC) &&
> > + WARN_ONCE(!in_task() && !in_serving_softirq(),
> > + "synchronous call from invalid context\n"))
> > + ret = -EBUSY;
>
> I don't think we've ever supported crypto in hard IRQ contexts.
> So this should be done regardless of ASYNC.
>
OK.
> Then again, do we really need this since the assumption has
> always been that the crypto API can only be invoked in user or
> softirq context?
>
With this series applied, some of the arm64 accelerated s/w
implementations will no longer work correctly when this rule is
violated, and so it would be nice to have a sanity check somewhere.
And policing rules like these is best done in generic code, right?
So if we do need to check this, we should check it here. If we don't,
then we can drop these patches.
On Wed, 19 May 2021 at 13:51, Herbert Xu <[email protected]> wrote:
>
> On Wed, May 19, 2021 at 01:36:37PM +0200, Ard Biesheuvel wrote:
> >
> > So if we do need to check this, we should check it here. If we don't,
> > then we can drop these patches.
>
> Historically other things would break in nasty ways if you tried
> to do crypto in hard IRQ contexts, e.g., overwritten kmap slots
> back when we had individual slots for each context, but I don't
> think we've ever found anyone crazy enough to do that to warrant
> a run-time check.
>
> I'd just leave it out for now.
>
Fair enough. Would you like me to resend the series with these patches
left out Or are you ok to just take the remaining ones (assuming there
are no issues reported with those)?