[PATCH] crypto: mxs-dcp - Fix softirq locking and completion

[Tomas Paukrt]

The request queues are protected by plain spin_lock(), but requests are
enqueued from softirq context (e.g. by ESP on packet receive) as well as
from process context, while the channel threads dequeue them in process
context with bottom halves enabled. A softirq that runs on the same CPU
while the lock is held either deadlocks on SMP, or corrupts the queue on
UP where the spinlock does not provide any protection.

In addition, the channel threads call the completion callbacks with
bottom halves enabled, although the crypto API requires them to be
called with bottom halves disabled. With IPsec, esp_input_done() then
runs xfrm_input() and gro_cells_receive() in plain process context,
which races with the NET_RX softirq on the per-CPU GRO cell queue and on
the xfrm state, and defers the NAPI poll until the next interrupt.

This was observed with strongSwan on i.MX6ULL: the throughput drops
quickly and eventually no packets are decrypted at all.

Fix this by taking the queue locks with spin_lock_bh() and by calling
the completion callbacks with bottom halves disabled.

Fixes: 15b59e7c3733 ("crypto: mxs - Add Freescale MXS DCP driver")
Signed-off-by: Tomas Paukrt <tomaspaukrt@xxxxxxxx>
---
 drivers/crypto/mxs-dcp.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index e1ff6ac8b213..b6749eb8f6f4 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -420,10 +420,10 @@ static int dcp_chan_thread_aes(void *data)
 	while (!kthread_should_stop()) {
 		set_current_state(TASK_INTERRUPTIBLE);

-		spin_lock(&sdcp->lock[chan]);
+		spin_lock_bh(&sdcp->lock[chan]);
 		backlog = crypto_get_backlog(&sdcp->queue[chan]);
 		arq = crypto_dequeue_request(&sdcp->queue[chan]);
-		spin_unlock(&sdcp->lock[chan]);
+		spin_unlock_bh(&sdcp->lock[chan]);

 		if (!backlog && !arq) {
 			schedule();
@@ -432,12 +432,17 @@ static int dcp_chan_thread_aes(void *data)

 		set_current_state(TASK_RUNNING);

-		if (backlog)
+		if (backlog) {
+			local_bh_disable();
 			crypto_request_complete(backlog, -EINPROGRESS);
+			local_bh_enable();
+		}

 		if (arq) {
 			ret = mxs_dcp_aes_block_crypt(arq);
+			local_bh_disable();
 			crypto_request_complete(arq, ret);
+			local_bh_enable();
 		}
 	}

@@ -480,9 +485,9 @@ static int mxs_dcp_aes_enqueue(struct skcipher_request *req, int enc, int ecb)
 	rctx->ecb = ecb;
 	actx->chan = DCP_CHAN_CRYPTO;

-	spin_lock(&sdcp->lock[actx->chan]);
+	spin_lock_bh(&sdcp->lock[actx->chan]);
 	ret = crypto_enqueue_request(&sdcp->queue[actx->chan], &req->base);
-	spin_unlock(&sdcp->lock[actx->chan]);
+	spin_unlock_bh(&sdcp->lock[actx->chan]);

 	wake_up_process(sdcp->thread[actx->chan]);

@@ -750,10 +755,10 @@ static int dcp_chan_thread_sha(void *data)
 	while (!kthread_should_stop()) {
 		set_current_state(TASK_INTERRUPTIBLE);

-		spin_lock(&sdcp->lock[chan]);
+		spin_lock_bh(&sdcp->lock[chan]);
 		backlog = crypto_get_backlog(&sdcp->queue[chan]);
 		arq = crypto_dequeue_request(&sdcp->queue[chan]);
-		spin_unlock(&sdcp->lock[chan]);
+		spin_unlock_bh(&sdcp->lock[chan]);

 		if (!backlog && !arq) {
 			schedule();
@@ -762,12 +767,17 @@ static int dcp_chan_thread_sha(void *data)

 		set_current_state(TASK_RUNNING);

-		if (backlog)
+		if (backlog) {
+			local_bh_disable();
 			crypto_request_complete(backlog, -EINPROGRESS);
+			local_bh_enable();
+		}

 		if (arq) {
 			ret = dcp_sha_req_to_buf(arq);
+			local_bh_disable();
 			crypto_request_complete(arq, ret);
+			local_bh_enable();
 		}
 	}

@@ -827,9 +837,9 @@ static int dcp_sha_update_fx(struct ahash_request *req, int fini)
 		rctx->init = 1;
 	}

-	spin_lock(&sdcp->lock[actx->chan]);
+	spin_lock_bh(&sdcp->lock[actx->chan]);
 	ret = crypto_enqueue_request(&sdcp->queue[actx->chan], &req->base);
-	spin_unlock(&sdcp->lock[actx->chan]);
+	spin_unlock_bh(&sdcp->lock[actx->chan]);

 	wake_up_process(sdcp->thread[actx->chan]);
 	mutex_unlock(&actx->mutex);
--
2.43.0