[PATCH] crypto: hisilicon/sec - Fix use-after-free and unchecked kfifo_put in sec_send_request()

[Arbab Haider <arbabhaider649@gmail.com>]

The capacity check in sec_alg_skcipher_crypto() only verified that the
hardware queue could accept all elements or that the software queue had
enough space, but failed to account for the case where the software
queue already had pending entries. When the software queue is non-empty,
all new entries must go to the software queue to preserve cipher
chaining ordering, even if the hardware queue has available capacity.

This could lead to two issues:

1. Elements silently dropped from the software queue if
   kfifo_put() returns 0 (queue full), causing the request to never
   complete.

2. Use-after-free if sec_queue_send() returns -EAGAIN after some
   elements were already queued to hardware or software — the error
   path in sec_send_request() returns -EBUSY, and the caller's
   err_free_elements path frees all elements, including those
   referenced by the hardware queue or software queue.

Fix by:

- Checking the return value of kfifo_put() and returning -ENOSPC on
  failure, with a WARN_ON_ONCE since this should not happen with the
  corrected capacity check.

- Catching all errors from sec_queue_send() (not just -EAGAIN) with a
  WARN_ON_ONCE, as this path is unreachable with proper capacity
  accounting.

- Replacing the capacity check with correct logic that accounts for
  the three possible queueing scenarios:
  * Software queue has entries or hardware queue is non-empty: all
    entries go to the software queue → verify software queue capacity
    for all steps.
  * Both queues empty: first entry goes to hardware, rest to software
    → verify hardware can take 1 and software can take steps - 1.
  * No software queue configured: all to hardware → verify hardware
    capacity for all steps.

Fixes: 915e4e8413da ("crypto: hisilicon - SEC security accelerator driver")
Signed-off-by: Arbab Haider <arbabhaider649@gmail.com>
---
 drivers/crypto/hisilicon/sec/sec_algs.c | 51 +++++++++++++++++--------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/crypto/hisilicon/sec/sec_algs.c b/drivers/crypto/hisilicon/sec/sec_algs.c
index 85eecbb40e7e..6cc08e9a07d0 100644
--- a/drivers/crypto/hisilicon/sec/sec_algs.c
+++ b/drivers/crypto/hisilicon/sec/sec_algs.c
@@ -402,14 +402,17 @@ static int sec_send_request(struct sec_request *sec_req, struct sec_queue *queue
 		    (kfifo_is_empty(&queue->softqueue) &&
 		     sec_queue_empty(queue))) {
 			ret = sec_queue_send(queue, &el->req, sec_req);
-			if (ret == -EAGAIN) {
-				/* Wait unti we can send then try again */
-				/* DEAD if here - should not happen */
+			if (ret) {
+				WARN_ON_ONCE(ret == -EAGAIN);
 				ret = -EBUSY;
 				goto err_unlock;
 			}
 		} else {
-			kfifo_put(&queue->softqueue, el);
+			if (!kfifo_put(&queue->softqueue, el)) {
+				WARN_ON_ONCE(1);
+				ret = -ENOSPC;
+				goto err_unlock;
+			}
 		}
 	}
 err_unlock:
@@ -790,28 +793,44 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq,
 
 	/*
 	 * Only attempt to queue if the whole lot can fit in the queue -
-	 * we can't successfully cleanup after a partial queing so this
+	 * we can't successfully cleanup after a partial queuing so this
 	 * must succeed or fail atomically.
 	 *
-	 * Big hammer test of both software and hardware queues - could be
-	 * more refined but this is unlikely to happen so no need.
+	 * When the softqueue has pending entries, all new entries must
+	 * go to the softqueue to preserve ordering.  When both queues
+	 * are empty the first entry goes to hardware and the rest to
+	 * the softqueue (if enabled).
 	 */
 
 	/* Grab a big lock for a long time to avoid concurrency issues */
 	spin_lock_bh(&queue->queuelock);
 
 	/*
-	 * Can go on to queue if we have space in either:
-	 * 1) The hardware queue and no software queue
-	 * 2) The software queue
-	 * AND there is nothing in the backlog.  If there is backlog we
-	 * have to only queue to the backlog queue and return busy.
+	 * Can go on to queue if we have space for everything upfront.
+	 * If there is backlog we must queue to the backlog and return busy.
 	 */
-	if ((!sec_queue_can_enqueue(queue, steps) &&
-	     (!queue->havesoftqueue ||
-	      kfifo_avail(&queue->softqueue) > steps)) ||
-	    !list_empty(&ctx->backlog)) {
+	if (!list_empty(&ctx->backlog)) {
 		ret = -EBUSY;
+		goto backlog;
+	}
+
+	if (!queue->havesoftqueue) {
+		if (!sec_queue_can_enqueue(queue, steps))
+			ret = -EBUSY;
+	} else if (!kfifo_is_empty(&queue->softqueue) ||
+		   !sec_queue_empty(queue)) {
+		/* All entries must go to the softqueue */
+		if (kfifo_avail(&queue->softqueue) <= steps)
+			ret = -EBUSY;
+	} else {
+		/* First to hardware, rest to softqueue */
+		if (!sec_queue_can_enqueue(queue, 1) ||
+		    kfifo_avail(&queue->softqueue) < steps - 1)
+			ret = -EBUSY;
+	}
+
+	if (ret) {
+backlog:
 		if ((skreq->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {
 			list_add_tail(&sec_req->backlog_head, &ctx->backlog);
 			spin_unlock_bh(&queue->queuelock);
-- 
2.53.0