Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver

From	David Gstir <david@sigma-star.at>
Newsgroups	linux.kernel
Subject	Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver
Date	2017-06-17 11:10 +0200
Message-ID	<tThTY-4Th-5@gated-at.bofh.it> (permalink)
References	<tNTSh-11x-3@gated-at.bofh.it> <tSEpA-3Kz-9@gated-at.bofh.it> <tSUkG-5AJ-21@gated-at.bofh.it> <tSUum-5Te-15@gated-at.bofh.it> <tT6Fc-5za-17@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

Horia,

> On 16 Jun 2017, at 23:01, Horia Geantă <horia.geanta@nxp.com> wrote:
> 
> On 6/16/2017 11:00 AM, Herbert Xu wrote:
>> On Fri, Jun 16, 2017 at 07:57:00AM +0000, Horia Geantă wrote:
>>> 
>>> Commit 0605c41cc53ca ("crypto: cts - Convert to skcipher") appends
>>> CRYPTO_TFM_REQ_MAY_BACKLOG to the original crypto request flags for the
>>> last block - when calling cts_cbc_encrypt().
>>> Is it really needed?
>> 
>> Yes, because at this point we cannot tell the sender to back off.
>> 
>>> For cts(cbc(aes)) with cbc(aes) offloaded in HW, i.e. running in async
>>> mode, we get the below stack for CAAM driver.
>>> Driver is told that it can sleep (CRYPTO_TFM_REQ_MAY_BACKLOG flag), so
>>> it uses GFP_KERNEL to allocate memory. However, this is incorrect, since
>>> driver runs in atomic context (softirq).
>> 
>> This is wrong.  Whether you can sleep or not is determined by
>> MAY_SLEEP, not MAY_BACKLOG.  MAY_BACKLOG only indicates that this
>> request must be queued, even if the queue is full.
>> 
> Indeed, CAAM driver incorrectly decides to use GFP_KERNEL for allocation
> when MAY_BACKLOG flag is set. This seems to be a long-standing issue, I
> will send a fix (separately).
> 
> Still I think we have a problem.
> David reported that the user is fscrypt. Looking into fscrypt code, I
> see that besides MAY_BACKLOG, MAY_SLEEP flag is also set. So we end up
> in the situation I described earlier: the last block is encrypted in
> atomic context and with MAY_SLEEP set.

Fixing the MAY_BACKLOG issue in the CAAM driver should get rid of the problem
altogether since the CTS code clears the MAY_SLEEP flag when encrypting the
last block [1].

David

[1] http://elixir.free-electrons.com/linux/v4.12-rc5/source/crypto/cts.c#L124

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver Horia Geantă <horia.geanta@nxp.com> - 2017-06-15 17:00 +0200
  Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver Horia Geantă <horia.geanta@nxp.com> - 2017-06-16 10:00 +0200
    Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver Herbert Xu <herbert@gondor.apana.org.au> - 2017-06-16 10:10 +0200
      Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver Horia Geantă <horia.geanta@nxp.com> - 2017-06-16 23:10 +0200
        Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver David Gstir <david@sigma-star.at> - 2017-06-17 11:10 +0200

csiph-web