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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 16 Jun 2017 21:01:45 +0000
From:   Horia Geantă <horia.geanta@....com>
To:     Herbert Xu <herbert@...dor.apana.org.au>
CC:     David Gstir <david@...ma-star.at>,
        Dan Douglass <dan.douglass@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Radu Solea <radu.solea@....com>,
        "richard@...ma-star.at" <richard@...ma-star.at>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 0/2] crypto: caam - fix cts(cbc(aes)) with CAAM driver

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