[PATCH v2] crypto: ccp - Fix use-after-free in backlog cmd advancement

[PATCH v2] crypto: ccp - Fix use-after-free in backlog cmd advancement

[Fan Wu]

A CCP_CMD_MAY_BACKLOG command promoted by a queue kthread leaves the
backlog list and is re-queued only later, by a work running on the
system workqueue. Until that work runs, the command is on neither
list, so the flush in ccp*_destroy() cannot reach it: the work then
uses the ccp_device after it has been released and wakes a queue
kthread whose task_struct kthread_stop() has already freed.

The gap between the promotion and the requeue also lets concurrent
submissions claim the space meant for the backlogged command, so it
is not actually reserved and cmd_count can grow past MAX_CMD_QLEN.

Promote the backlogged command in ccp_dequeue_cmd() itself: the slot
freed by the dequeue is transferred to the backlog head under the
same cmd_lock, so the space is reserved atomically, and the command
is put on the normal queue only after its advancement callback
(-EINPROGRESS) has run from the queue kthread, which provides process
context and keeps the backlog completion ahead of execution. The work
struct is no longer needed and the unused member is removed from
ccp_cmd.

Mark the device halting before stopping its queues and suppress the
promotion wake under cmd_lock once teardown starts, so it cannot wake
a queue kthread that teardown has already stopped.

This issue was found by an in-house static analysis tool.

Fixes: 63b945091a07 ("crypto: ccp - CCP device driver and interface support")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---

Link to v1: https://lore.kernel.org/linux-crypto/20260907060415.575656-1-fanwu01@zju.edu.cn/

- transfer the slot freed by the dequeue to the backlogged command
  under cmd_lock, as suggested by Herbert Xu, and drop the work
  struct and the per-device workqueue of v1.
- mark the device halting before stopping its queues and suppress
  the promotion wake under cmd_lock once teardown starts.
 drivers/crypto/ccp/ccp-dev-v3.c |  3 ++
 drivers/crypto/ccp/ccp-dev-v5.c |  3 ++
 drivers/crypto/ccp/ccp-dev.c    | 88 +++++++++++++++++++--------------
 drivers/crypto/ccp/ccp-dev.h    |  3 ++
 include/linux/ccp.h             |  5 +-
 5 files changed, 62 insertions(+), 40 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-dev-v3.c b/drivers/crypto/ccp/ccp-dev-v3.c
index d92de4ad31..c5ab8beb16 100644
--- a/drivers/crypto/ccp/ccp-dev-v3.c
+++ b/drivers/crypto/ccp/ccp-dev-v3.c
@@ -501,6 +501,8 @@ static int ccp_init(struct ccp_device *ccp)
 	ccp_unregister_rng(ccp);
 
 e_kthread:
+	ccp_halt_cmds(ccp);
+
 	for (i = 0; i < ccp->cmd_q_count; i++)
 		if (ccp->cmd_q[i].kthread)
 			kthread_stop(ccp->cmd_q[i].kthread);
@@ -542,6 +544,7 @@ static void ccp_destroy(struct ccp_device *ccp)
 	iowrite32(ccp->qim, ccp->io_regs + IRQ_STATUS_REG);
 
 	/* Stop the queue kthreads */
+	ccp_halt_cmds(ccp);
 	for (i = 0; i < ccp->cmd_q_count; i++)
 		if (ccp->cmd_q[i].kthread)
 			kthread_stop(ccp->cmd_q[i].kthread);
diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c
index dacde9614e..3b09befa45 100644
--- a/drivers/crypto/ccp/ccp-dev-v5.c
+++ b/drivers/crypto/ccp/ccp-dev-v5.c
@@ -989,6 +989,8 @@ static int ccp5_init(struct ccp_device *ccp)
 	ccp_unregister_rng(ccp);
 
 e_kthread:
+	ccp_halt_cmds(ccp);
+
 	for (i = 0; i < ccp->cmd_q_count; i++)
 		if (ccp->cmd_q[i].kthread)
 			kthread_stop(ccp->cmd_q[i].kthread);
@@ -1043,6 +1045,7 @@ static void ccp5_destroy(struct ccp_device *ccp)
 	}
 
 	/* Stop the queue kthreads */
+	ccp_halt_cmds(ccp);
 	for (i = 0; i < ccp->cmd_q_count; i++)
 		if (ccp->cmd_q[i].kthread)
 			kthread_stop(ccp->cmd_q[i].kthread);
diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
index aff3348b83..12c210acac 100644
--- a/drivers/crypto/ccp/ccp-dev.c
+++ b/drivers/crypto/ccp/ccp-dev.c
@@ -177,6 +177,15 @@ void ccp_del_device(struct ccp_device *ccp)
 	write_unlock_irqrestore(&ccp_unit_lock, flags);
 }
 
+/* Prevent promotion from waking an idle queue during teardown. */
+void ccp_halt_cmds(struct ccp_device *ccp)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ccp->cmd_lock, flags);
+	ccp->halting = true;
+	spin_unlock_irqrestore(&ccp->cmd_lock, flags);
+}
 
 
 int ccp_register_rng(struct ccp_device *ccp)
@@ -342,41 +351,13 @@ int ccp_enqueue_cmd(struct ccp_cmd *cmd)
 }
 EXPORT_SYMBOL_GPL(ccp_enqueue_cmd);
 
-static void ccp_do_cmd_backlog(struct work_struct *work)
-{
-	struct ccp_cmd *cmd = container_of(work, struct ccp_cmd, work);
-	struct ccp_device *ccp = cmd->ccp;
-	unsigned long flags;
-	unsigned int i;
-
-	cmd->callback(cmd->data, -EINPROGRESS);
-
-	spin_lock_irqsave(&ccp->cmd_lock, flags);
-
-	ccp->cmd_count++;
-	list_add_tail(&cmd->entry, &ccp->cmd);
-
-	/* Find an idle queue */
-	for (i = 0; i < ccp->cmd_q_count; i++) {
-		if (ccp->cmd_q[i].active)
-			continue;
-
-		break;
-	}
-
-	spin_unlock_irqrestore(&ccp->cmd_lock, flags);
-
-	/* If we found an idle queue, wake it up */
-	if (i < ccp->cmd_q_count)
-		wake_up_process(ccp->cmd_q[i].kthread);
-}
-
 static struct ccp_cmd *ccp_dequeue_cmd(struct ccp_cmd_queue *cmd_q)
 {
 	struct ccp_device *ccp = cmd_q->ccp;
 	struct ccp_cmd *cmd = NULL;
 	struct ccp_cmd *backlog = NULL;
 	unsigned long flags;
+	unsigned int i = ccp->cmd_q_count;
 
 	spin_lock_irqsave(&ccp->cmd_lock, flags);
 
@@ -391,26 +372,61 @@ static struct ccp_cmd *ccp_dequeue_cmd(struct ccp_cmd_queue *cmd_q)
 		return NULL;
 	}
 
-	if (ccp->cmd_count) {
+	if (!list_empty(&ccp->cmd)) {
 		cmd_q->active = 1;
 
 		cmd = list_first_entry(&ccp->cmd, struct ccp_cmd, entry);
 		list_del(&cmd->entry);
 
-		ccp->cmd_count--;
-	}
-
-	if (!list_empty(&ccp->backlog)) {
+		if (!list_empty(&ccp->backlog)) {
+			/* Transfer the freed slot to the backlogged
+			 * command, so that concurrent submissions cannot
+			 * claim the space meant for it.
+			 */
+			backlog = list_first_entry(&ccp->backlog,
+						   struct ccp_cmd, entry);
+			list_del(&backlog->entry);
+		} else {
+			ccp->cmd_count--;
+		}
+	} else if (!list_empty(&ccp->backlog)) {
+		/* No command is available for execution, but a backlogged
+		 * command is stranded: reserve a slot for it.
+		 */
 		backlog = list_first_entry(&ccp->backlog, struct ccp_cmd,
 					   entry);
 		list_del(&backlog->entry);
+
+		ccp->cmd_count++;
 	}
 
 	spin_unlock_irqrestore(&ccp->cmd_lock, flags);
 
 	if (backlog) {
-		INIT_WORK(&backlog->work, ccp_do_cmd_backlog);
-		schedule_work(&backlog->work);
+		/* Notify the advancement out of the backlog before the
+		 * command is made available for execution; the queue
+		 * kthread provides process context, so no work struct
+		 * is needed.
+		 */
+		backlog->callback(backlog->data, -EINPROGRESS);
+
+		spin_lock_irqsave(&ccp->cmd_lock, flags);
+
+		list_add_tail(&backlog->entry, &ccp->cmd);
+
+		/* Find an idle queue */
+		for (i = 0; i < ccp->cmd_q_count; i++) {
+			if (ccp->cmd_q[i].active)
+				continue;
+
+			break;
+		}
+
+		/* Do not wake an idle queue after teardown has started. */
+		if (!ccp->halting && i < ccp->cmd_q_count)
+			wake_up_process(ccp->cmd_q[i].kthread);
+
+		spin_unlock_irqrestore(&ccp->cmd_lock, flags);
 	}
 
 	return cmd;
diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h
index 83350e2d98..ead87c138b 100644
--- a/drivers/crypto/ccp/ccp-dev.h
+++ b/drivers/crypto/ccp/ccp-dev.h
@@ -374,6 +374,8 @@ struct ccp_device {
 	struct list_head cmd;
 	struct list_head backlog;
 
+	bool halting;
+
 	/* The command queues. These represent the queues available on the
 	 * CCP that are available for processing cmds
 	 */
@@ -630,6 +632,7 @@ struct ccp5_desc {
 
 void ccp_add_device(struct ccp_device *ccp);
 void ccp_del_device(struct ccp_device *ccp);
+void ccp_halt_cmds(struct ccp_device *ccp);
 
 extern void ccp_log_error(struct ccp_device *, unsigned int);
 
diff --git a/include/linux/ccp.h b/include/linux/ccp.h
index e6c599243f..0386813361 100644
--- a/include/linux/ccp.h
+++ b/include/linux/ccp.h
@@ -12,7 +12,6 @@
 #define __CCP_H__
 
 #include <linux/scatterlist.h>
-#include <linux/workqueue.h>
 #include <linux/list.h>
 #include <crypto/aes.h>
 #include <crypto/sha1.h>
@@ -637,7 +636,6 @@ enum ccp_engine {
 /**
  * struct ccp_cmd - CCP operation request
  * @entry: list element (ccp driver use only)
- * @work: work element used for callbacks (ccp driver use only)
  * @ccp: CCP device to be run on
  * @ret: operation return code (ccp driver use only)
  * @flags: cmd processing flags
@@ -653,11 +651,10 @@ enum ccp_engine {
  *     operation.
  */
 struct ccp_cmd {
-	/* The list_head, work_struct, ccp and ret variables are for use
+	/* The list_head, ccp and ret variables are for use
 	 * by the CCP driver only.
 	 */
 	struct list_head entry;
-	struct work_struct work;
 	struct ccp_device *ccp;
 	int ret;
 

Re: [PATCH v2] crypto: ccp - Fix use-after-free in backlog cmd advancement

[Markus Elfring]

…
> +++ b/drivers/crypto/ccp/ccp-dev.c
> @@ -177,6 +177,15 @@ void ccp_del_device(struct ccp_device *ccp)
>  	write_unlock_irqrestore(&ccp_unit_lock, flags);
>  }
>  
> +/* Prevent promotion from waking an idle queue during teardown. */
> +void ccp_halt_cmds(struct ccp_device *ccp)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ccp->cmd_lock, flags);
> +	ccp->halting = true;
> +	spin_unlock_irqrestore(&ccp->cmd_lock, flags);
> +}
…

May lock guards be applied in affected function implementations?
https://elixir.bootlin.com/linux/v7.3-rc5/source/include/linux/spinlock.h#L642-L645

Regards,
Markus

Re: [PATCH v2] crypto: ccp - Fix use-after-free in backlog cmd advancement

[Fan Wu]

On Tue, 29 Sep 2026 17:18:29 +0200, Markus Elfring wrote:
> May lock guards be applied in affected function implementations?
> https://elixir.bootlin.com/linux/v7.3-rc5/source/include/linux/spinlock.h#L642-L645

I considered using lock guards, but kept the manual locking here.

ccp_dequeue_cmd() deliberately has two separate cmd_lock sections, with
the -EINPROGRESS callback between them. The callback must run outside
cmd_lock, so the explicit unlock makes that boundary clear.

This is also a stable fix for code dating back to 2013. Stable branches
such as v6.1 do not have the cleanup/guard helpers, while the manual
form backports unchanged.

I can use a guard for the one-statement ccp_halt_cmds() helper if you
prefer, but would keep the two promotion sections explicit.

Thanks,
Fan Wu