crypto: engine - fix crypto_queue backlog handling
PATCH 1/1 crypto: engine - fix crypto_queue backlog handling
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.
--- crypto/algapi.c | 3 +++ crypto/crypto_engine.c | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-)
crypto/algapi.c +3 -0
@@ -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); }
crypto/crypto_engine.c +3 -3
@@ -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);
Signature · 1 line
2.40.0
Herbert Xu
maintainerQuoted 26 lines · Olivier Bacon
CRYPTO_TFM_REQ_MAY_BACKLOGtells the crypto driver that it should internally backlog requests until the crypto hw's queue becomes full. At that point,crypto_enginebacklogs the request and returns-EBUSY. Calling driver such as dm-crypt then waits until thecomplete()function is called with a status of-EINPROGRESSbefore sending a new request.The problem lies in the call to
complete()with a value of-EINPROGRESSthat 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 thatdo_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
-EINPROGRESSonly if the request is not enqueued back incrypto_queue. This is done by callingcomplete()later in the code. In order to delay the decision,crypto_queueis modified to correctly set the backlog pointer when a request is enqueued back.
Quoted 8 lines
Fixes 6a89f492f8e5 ("crypto: engine - support for parallel requests based on retry mechanism")Co-developed-by Sylvain Ouellet <[email protected]>Signed-off-by Sylvain Ouellet <[email protected]>Signed-off-by Olivier Bacon <[email protected]>--- crypto/algapi.c | 3 +++ crypto/crypto_engine.c | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-)
Patch applied. Thanks.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
A message that is not in the archive (sent off-list or not yet received): AM9PR04MB82116818BF48EFE951BB1276E8679@AM9PR04MB8211.eurprd04.prod.outlook.com
Thanks & Regards,
Kshitiz
Quoted 82 lines
-----Original Message-----
From: Olivier Bacon <[email protected]>
Sent: Thursday, April 20, 2023 8:31 PM
To: [email protected]
Cc: [email protected]; [email protected]; Olivier Bacon <[email protected]>; Sylvain Ouellet <[email protected]>
Subject: [EXT] [PATCH] crypto: engine - fixcrypto_queuebacklog handlingCaution: EXT Email
CRYPTO_TFM_REQ_MAY_BACKLOGtells the crypto driver that it should internally backlog requests until the crypto hw's queue becomes full. At that point,crypto_enginebacklogs the request and returns-EBUSY. Calling driver such as dm-crypt then waits until thecomplete()function is called with a status of-EINPROGRESSbefore sending a new request.The problem lies in the call to
complete()with a value of-EINPROGRESSthat 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 thatdo_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
-EINPROGRESSonly if the request is not enqueued back incrypto_queue. This is done by callingcomplete()later in the code. In order to delay the decision,crypto_queueis modified to correctly set the backlog pointer when a request is enqueued back.Fixes 6a89f492f8e5 ("crypto: engine - support for parallel requests based onretry mechanism")
Co-developed-by Sylvain Ouellet <[email protected]>Signed-off-by Sylvain Ouellet <[email protected]>Signed-off-by Olivier Bacon <[email protected]>--- crypto/algapi.c | 3 +++ crypto/crypto_engine.c | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-)crypto/algapi.c index d08f864f08be..18a8ca9cafbc renamed from crypto/algapi.c +0
-0100644
crypto/algapi.c +3
-0@@ -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.cindex21f791615114..74fcc0897041100644crypto/crypto_engine.c +0
-3@@ -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
crypto/crypto_engine.c +3
-0@@ -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