linux-kernel - Re: [PATCH v2] dm crypt: add flags to optionally bypass dm-crypt workqueues

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:   Tue, 30 Jun 2020 03:45:48 +0000
From:   Damien Le Moal <Damien.LeMoal@....com>
To:     Herbert Xu <herbert@...dor.apana.org.au>
CC:     Ignat Korchagin <ignat@...udflare.com>,
        "agk@...hat.com" <agk@...hat.com>,
        "snitzer@...hat.com" <snitzer@...hat.com>,
        "dm-devel@...hat.com" <dm-devel@...hat.com>,
        "dm-crypt@...ut.de" <dm-crypt@...ut.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "ebiggers@...nel.org" <ebiggers@...nel.org>,
        "mpatocka@...hat.com" <mpatocka@...hat.com>,
        "kernel-team@...udflare.com" <kernel-team@...udflare.com>
Subject: Re: [PATCH v2] dm crypt: add flags to optionally bypass dm-crypt
 workqueues

On 2020/06/30 12:09, Herbert Xu wrote:
> On Tue, Jun 30, 2020 at 02:51:17AM +0000, Damien Le Moal wrote:
>>
>>> @@ -1463,12 +1465,12 @@ static void crypt_alloc_req_skcipher(struct crypt_config *cc,
>>>  	 * requests if driver request queue is full.
>>>  	 */
>>>  	skcipher_request_set_callback(ctx->r.req,
>>> -	    CRYPTO_TFM_REQ_MAY_BACKLOG,
>>> +	    nobacklog ? 0 : CRYPTO_TFM_REQ_MAY_BACKLOG,
>>>  	    kcryptd_async_done, dmreq_of_req(cc, ctx->r.req));
>>
>> Will not specifying CRYPTO_TFM_REQ_MAY_BACKLOG always cause the crypto API to
>> return -EBUSY ? From the comment above the skcipher_request_set_callback(), it
>> seems that this will be the case only if the skcipher diver queue is full. So in
>> other word, keeping the kcryptd_async_done() callback and executing the skcipher
>> request through crypt_convert() and crypt_convert_block_skcipher() may still end
>> up being an asynchronous operation. Can you confirm this and is it what you
>> intended to implement ?
> 
> The purpose of MAY_BACKLOG is to make the crypto request reliable.
> It has nothing to do with whether the request will be synchronous
> or not.
> 
> Without the backlog flag, if the hardware queue is full the request
> will simply be dropped, which is appropriate in the network stack
> with IPsec where congestion can be dealt with at the source.
> 
> Block layer on the other hand should always use the backlog flag
> and stop sending more requests to the crypto API until the congestion
> goes away.

OK. Thanks for the information. So it sounds like this patch still needs some
more fixes.

> 
> Cheers,
> 


-- 
Damien Le Moal
Western Digital Research