Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210512184439.8778-1-ardb@kernel.org> <20210512184439.8778-3-ardb@kernel.org>
 <YJw1KLD8bCLTd+Oc@gmail.com>
In-Reply-To: <YJw1KLD8bCLTd+Oc@gmail.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Wed, 12 May 2021 23:24:51 +0200
Message-ID: <CAMj1kXFg8Viy-+h1Nsd1dXsc2KdwtDvdv4e1YeEKMnoLRhszow@mail.gmail.com>
Subject: Re: [PATCH v3 2/7] crypto: aead - disallow en/decrypt for non-task or
 non-softirq context
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Will Deacon <will@kernel.org>,
        Android Kernel Team <kernel-team@android.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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