linux-kernel - RE: [EXT] [PATCH] crypto: engine - fix crypto

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>] [day] [month] [year] [list]

Date:   Tue, 2 May 2023 07:19:54 +0000
From:   Kshitiz Varshney <kshitiz.varshney@....com>
To:     Olivier Bacon <obacon@...etec.com>,
        Sylvain Ouellet <souellet@...etec.com>
CC:     Varun Sethi <V.Sethi@....com>, Gaurav Jain <gaurav.jain@....com>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Herbert Xu <herbert@...dor.apana.org.au>
Subject: RE: [EXT] [PATCH] crypto: engine - fix crypto_queue backlog handling


Reviewed-by: Kshitiz Varshney <kshitiz.varshney@....com>
Tested-by: Kshitiz Varshney <kshitiz.varshney@....com>

Thanks & Regards,
Kshitiz

 
> -----Original Message-----
> From: Olivier Bacon <olivierb89@...il.com>
> Sent: Thursday, April 20, 2023 8:31 PM
> To: herbert@...dor.apana.org.au
> Cc: linux-crypto@...r.kernel.org; linux-kernel@...r.kernel.org; Olivier Bacon
> <obacon@...etec.com>; Sylvain Ouellet <souellet@...etec.com> 
> Subject: [EXT] [PATCH] crypto: engine - fix crypto_queue backlog handling
> 
> Caution: EXT Email
> 
> CRYPTO_TFM_REQ_MAY_BACKLOG tells the crypto driver that it should
> internally backlog requests until the crypto hw's queue becomes full. At that
> point, crypto_engine backlogs the request and returns -EBUSY. Calling driver
> such as dm-crypt then waits until the
> complete() function is called with a status of -EINPROGRESS before sending a
> new request.
> 
> The problem lies in the call to complete() with a value of -EINPROGRESS that is
> made when a backlog item is present on the queue. The call is done before the
> successful execution of the crypto request. In the case that do_one_request()
> returns < 0 and the retry support is available, the request is put back in the queue.
> This leads upper drivers to send a new request even if the queue is still full.
> 
> The problem can be reproduced by doing a large dd into a crypto dm-crypt
> device. This is pretty easy to see when using Freescale CAAM crypto driver and
> SWIOTLB dma. Since the actual amount of requests that can be hold in the
> queue is unlimited we get IOs error and dma allocation.
> 
> The fix is to call complete with a value of -EINPROGRESS only if the request is
> not enqueued back in crypto_queue. This is done by calling complete() later in
> the code. In order to delay the decision, crypto_queue is modified to correctly
> set the backlog pointer when a request is enqueued back.
> 
> Fixes: 6a89f492f8e5 ("crypto: engine - support for parallel requests based on
> retry mechanism")
> Co-developed-by: Sylvain Ouellet <souellet@...etec.com>
> Signed-off-by: Sylvain Ouellet <souellet@...etec.com>
> Signed-off-by: Olivier Bacon <obacon@...etec.com>
> ---
>  crypto/algapi.c        | 3 +++
>  crypto/crypto_engine.c | 6 +++---
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/crypto/algapi.c b/crypto/algapi.c index d08f864f08be..18a8ca9cafbc
> 100644
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -961,6 +961,9 @@ EXPORT_SYMBOL_GPL(crypto_enqueue_request);
>  void crypto_enqueue_request_head(struct crypto_queue *queue,
>                                  struct crypto_async_request *request)  {
> +       if (unlikely(queue->qlen >= queue->max_qlen))
> +               queue->backlog = queue->backlog->prev;
> +
>         queue->qlen++;
>         list_add(&request->list, &queue->list);  } diff --git a/crypto/crypto_engine.c
> b/crypto/crypto_engine.c index 21f791615114..74fcc0897041 100644
> --- a/crypto/crypto_engine.c
> +++ b/crypto/crypto_engine.c
> @@ -129,9 +129,6 @@ static void crypto_pump_requests(struct crypto_engine
> *engine,
>         if (!engine->retry_support)
>                 engine->cur_req = async_req;
> 
> -       if (backlog)
> -               crypto_request_complete(backlog, -EINPROGRESS);
> -
>         if (engine->busy)
>                 was_busy = true;
>         else
> @@ -217,6 +214,9 @@ static void crypto_pump_requests(struct crypto_engine
> *engine,
>         crypto_request_complete(async_req, ret);
> 
>  retry:
> +       if (backlog)
> +               crypto_request_complete(backlog, -EINPROGRESS);
> +
>         /* If retry mechanism is supported, send new requests to engine */
>         if (engine->retry_support) {
>                 spin_lock_irqsave(&engine->queue_lock, flags);
> --
> 2.40.0