From: =?iso-8859-2?Q?Horia_Geant=E3?= <horia.geanta@nxp.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
CC: David Gstir <david@sigma-star.at>, Dan Douglass <dan.douglass@nxp.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        Radu Solea <radu.solea@nxp.com>,
        "richard@sigma-star.at" <richard@sigma-star.at>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver
Thread-Topic: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM
 driver
Thread-Index: AQHS25tCyorrZx6/xka98MRBFLZ7gw==
Date: Fri, 16 Jun 2017 21:01:45 +0000
Message-ID: <VI1PR0401MB25914CC17187DD446802C25F98C10@VI1PR0401MB2591.eurprd04.prod.outlook.com>
References: <20170602122446.2427-1-david@sigma-star.at>
 <VI1PR0401MB25910B19DB95DABF62C00A7598C00@VI1PR0401MB2591.eurprd04.prod.outlook.com>
 <VI1PR0401MB25913AAA76159D27E8EB13DF98C10@VI1PR0401MB2591.eurprd04.prod.outlook.com>
 <20170616075941.GA5346@gondor.apana.org.au>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="iso-8859-2"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Jun 2017 21:01:45.2540
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0401MB2624
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v5GL22Ru015158
Content-Length: 1380
Lines: 32

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.

Thanks,
Horia