Skip to contentSkip to conversation

crypto: mxs-dcp - Fix softirq locking and completion

PATCH New Tomas Paukrt 1 participant 1 message Nov 9

Subsystems (2)
Arm i.MX and Layerscape Crypto API
lore
Share & tools
lore

Tomas Paukrt

author
#0 lore raw

PATCH 1/1 crypto: mxs-dcp - Fix softirq locking and completion

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 <[email protected]>
---
 drivers/crypto/mxs-dcp.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)
drivers/crypto/mxs-dcp.c +20 -10
index e1ff6ac8b213..b6749eb8f6f4 100644
@@ -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