crypto: ccp - Fix use-after-free in backlog cmd advancement

PATCH Superseded Fan Wu 2 participants 2 messages Sep 7 – Sep 18

Subsystems (3)

Stable branch Crypto API AMD cryptographic coprocessor (CCP) driver

lore

Share & tools

lore

Fan Wu

author

2026-09-07 06:05 UTC #0 lore raw

PATCH 1/1 crypto: ccp - Fix use-after-free in backlog cmd advancement

CCP_CMD_MAY_BACKLOG commands are removed from ccp->backlog by a queue kthread and advanced asynchronously by ccp_do_cmd_backlog(). The promoted command is no longer on either command list.

During device removal, ccp*_destroy() stops the queue kthreads and flushes the lists, but does not wait for a promoted work on system_wq. That work can subsequently access the devm-allocated ccp_device after it has been released, and wake a queue kthread whose task_struct kthread_stop() has already released.

Use a per-device workqueue for backlog advancement and destroy it after stopping all queue kthreads, so every promoted work has completed before the command lists are flushed. The queue kthreads are the only source of backlog works, so no new work can be queued once they are stopped. Individual cancellation is not possible because a promoted command is no longer reachable from either list.

Mark the device as halting before teardown. A draining backlog work then completes its command with -ENODEV instead of re-queuing it. Keep the halting check and the kthread wake under cmd_lock so teardown cannot stop the selected kthread between them.

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

Fixes 63b945091a07 ("crypto: ccp - CCP device driver and interface support")

Cc [email protected]

Assisted-by Codex:gpt-5.6

Co-developed-by Song Li <[email protected]>

Signed-off-by Song Li <[email protected]>

Signed-off-by Fan Wu <[email protected]>

---
 drivers/crypto/ccp/ccp-dev-v3.c | 19 +++++++++++++++++++
 drivers/crypto/ccp/ccp-dev-v5.c | 18 ++++++++++++++++++
 drivers/crypto/ccp/ccp-dev.c    | 33 ++++++++++++++++++++++++++++-----
 drivers/crypto/ccp/ccp-dev.h    |  4 ++++
 4 files changed, 69 insertions(+), 5 deletions(-)

drivers/crypto/ccp/ccp-dev-v3.c +19 -0

index fe69053b2394..b84ac117ea25 100644
@@ -460,6 +460,14 @@ static int ccp_init(struct ccp_device *ccp)
 		tasklet_init(&ccp->irq_tasklet, ccp_irq_bh,
 			     (unsigned long)ccp);
 
+	ccp->backlog_wq = alloc_workqueue("%s-backlog", WQ_MEM_RECLAIM, 0,
+					  ccp->name);
+	if (!ccp->backlog_wq) {
+		dev_err(dev, "unable to allocate backlog workqueue\n");
+		ret = -ENOMEM;
+		goto e_irq;
+	}
+
 	dev_dbg(dev, "Starting threads...\n");
 	/* Create a kthread for each queue */
 	for (i = 0; i < ccp->cmd_q_count; i++) {

@@ -501,10 +509,15 @@ 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);
 
+	destroy_workqueue(ccp->backlog_wq);
+
+e_irq:
 	sp_free_ccp_irq(ccp->sp, ccp);
 
 e_pool:

@@ -520,6 +533,9 @@ static void ccp_destroy(struct ccp_device *ccp)
 	struct ccp_cmd *cmd;
 	unsigned int i;
 
+	/* Complete pending backlog cmds instead of executing them */
+	ccp_halt_cmds(ccp);
+
 	/* Unregister the DMA engine */
 	ccp_dmaengine_unregister(ccp);

@@ -546,6 +562,9 @@ static void ccp_destroy(struct ccp_device *ccp)
 
 	sp_free_ccp_irq(ccp->sp, ccp);
 
+	/* Drain promoted backlog commands before flushing the command lists. */
+	destroy_workqueue(ccp->backlog_wq);
+
 	for (i = 0; i < ccp->cmd_q_count; i++)
 		dma_pool_destroy(ccp->cmd_q[i].dma_pool);

drivers/crypto/ccp/ccp-dev-v5.c +18 -0

index 7b73332d6aa1..307a408eff89 100644
@@ -943,6 +943,14 @@ static int ccp5_init(struct ccp_device *ccp)
 		ccp->cmd_q[i].sb_ctx = ccp_lsb_alloc(&ccp->cmd_q[i], 2);
 	}
 
+	ccp->backlog_wq = alloc_workqueue("%s-backlog", WQ_MEM_RECLAIM, 0,
+					  ccp->name);
+	if (!ccp->backlog_wq) {
+		dev_err(dev, "unable to allocate backlog workqueue\n");
+		ret = -ENOMEM;
+		goto e_irq;
+	}
+
 	dev_dbg(dev, "Starting threads...\n");
 	/* Create a kthread for each queue */
 	for (i = 0; i < ccp->cmd_q_count; i++) {

@@ -989,10 +997,14 @@ 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);
 
+	destroy_workqueue(ccp->backlog_wq);
+
 e_irq:
 	sp_free_ccp_irq(ccp->sp, ccp);

@@ -1009,6 +1021,9 @@ static void ccp5_destroy(struct ccp_device *ccp)
 	struct ccp_cmd *cmd;
 	unsigned int i;
 
+	/* Complete pending backlog cmds instead of executing them */
+	ccp_halt_cmds(ccp);
+
 	/* Unregister the DMA engine */
 	ccp_dmaengine_unregister(ccp);

@@ -1047,6 +1062,9 @@ static void ccp5_destroy(struct ccp_device *ccp)
 
 	sp_free_ccp_irq(ccp->sp, ccp);
 
+	/* Drain promoted backlog commands before flushing the command lists. */
+	destroy_workqueue(ccp->backlog_wq);
+
 	/* Flush the cmd and backlog queue */
 	while (!list_empty(&ccp->cmd)) {
 		/* Invoke the callback directly with an error code */

drivers/crypto/ccp/ccp-dev.c +28 -5

index 246801912e1a..b8eeaf0c3c95 100644
@@ -177,6 +177,15 @@ void ccp_del_device(struct ccp_device *ccp)
 	write_unlock_irqrestore(&ccp_unit_lock, flags);
 }
 
+/* Mark the device halting so draining backlog works complete with -ENODEV. */
+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)

@@ -349,6 +358,20 @@ static void ccp_do_cmd_backlog(struct work_struct *work)
 	unsigned long flags;
 	unsigned int i;
 
+	spin_lock_irqsave(&ccp->cmd_lock, flags);
+	if (ccp->halting) {
+		spin_unlock_irqrestore(&ccp->cmd_lock, flags);
+
+		/* The device is being removed; the cmd can no longer be
+		 * executed, so complete it with an error like the cmds
+		 * still queued on the cmd and backlog lists
+		 */
+		cmd->callback(cmd->data, -ENODEV);
+
+		return;
+	}
+	spin_unlock_irqrestore(&ccp->cmd_lock, flags);
+
 	cmd->callback(cmd->data, -EINPROGRESS);
 
 	spin_lock_irqsave(&ccp->cmd_lock, flags);

@@ -364,11 +387,11 @@ static void ccp_do_cmd_backlog(struct work_struct *work)
 		break;
 	}
 
-	spin_unlock_irqrestore(&ccp->cmd_lock, flags);
-
-	/* If we found an idle queue, wake it up */
-	if (i < ccp->cmd_q_count)
+	/* Keep this under cmd_lock so teardown cannot stop this kthread first. */
+	if (!ccp->halting && i < ccp->cmd_q_count)
 		wake_up_process(ccp->cmd_q[i].kthread);
+
+	spin_unlock_irqrestore(&ccp->cmd_lock, flags);
 }
 
 static struct ccp_cmd *ccp_dequeue_cmd(struct ccp_cmd_queue *cmd_q)

@@ -410,7 +433,7 @@ static struct ccp_cmd *ccp_dequeue_cmd(struct ccp_cmd_queue *cmd_q)
 
 	if (backlog) {
 		INIT_WORK(&backlog->work, ccp_do_cmd_backlog);
-		schedule_work(&backlog->work);
+		queue_work(ccp->backlog_wq, &backlog->work);
 	}
 
 	return cmd;

drivers/crypto/ccp/ccp-dev.h +4 -0

index 83350e2d9821..91b303ec46a9 100644
@@ -374,6 +374,9 @@ struct ccp_device {
 	struct list_head cmd;
 	struct list_head backlog;
 
+	struct workqueue_struct *backlog_wq;
+	bool halting;
+
 	/* The command queues. These represent the queues available on the
 	 * CCP that are available for processing cmds
 	 */

@@ -630,6 +633,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);

Herbert Xu

maintainer

2026-09-18 05:10 UTC #1 lore raw

Quoted 30 lines · Fan Wu

CCP_CMD_MAY_BACKLOG commands are removed from ccp->backlog by a queue kthread and advanced asynchronously by ccp_do_cmd_backlog(). The promoted command is no longer on either command list.
During device removal, ccp*_destroy() stops the queue kthreads and flushes the lists, but does not wait for a promoted work on system_wq. That work can subsequently access the devm-allocated ccp_device after it has been released, and wake a queue kthread whose task_struct kthread_stop() has already released.
Use a per-device workqueue for backlog advancement and destroy it after stopping all queue kthreads, so every promoted work has completed before the command lists are flushed. The queue kthreads are the only source of backlog works, so no new work can be queued once they are stopped. Individual cancellation is not possible because a promoted command is no longer reachable from either list.
Mark the device as halting before teardown. A draining backlog work then completes its command with -ENODEV instead of re-queuing it. Keep the halting check and the kthread wake under cmd_lock so teardown cannot stop the selected kthread between them.
This issue was found by an in-house static analysis tool.
Fixes 63b945091a07 ("crypto: ccp - CCP device driver and interface support")
Cc [email protected]
Assisted-by Codex:gpt-5.6
Co-developed-by Song Li <[email protected]>
Signed-off-by Song Li <[email protected]>
Signed-off-by Fan Wu <[email protected]>

Quoted 6 lines

---

drivers/crypto/ccp/ccp-dev-v3.c | 19 +++++++++++++++++++
drivers/crypto/ccp/ccp-dev-v5.c | 18 ++++++++++++++++++
drivers/crypto/ccp/ccp-dev.c    | 33 ++++++++++++++++++++++++++++-----
drivers/crypto/ccp/ccp-dev.h    |  4 ++++
4 files changed, 69 insertions(+), 5 deletions(-)

Thanks for catching this!

The backlog handling still looks a bit wrong though. When you detach a backlogged request after processing a normal request, there is a gap before the backlogged request is added to the normal queue.

In that gap new requests could have come in and taken up the space meant for the backlogged request.

I think this should be done atomically so that there is no gap between the two operations, to ensure that the space is actually reserved for the backlogged entry.

Once this is done, there should be no need to have a per-backlog work struct, which means that you don't need to add a separate work queue. In fact, there may not be a need for a work struct at all since this is all running out of process context anyway.

On a higher level, this driver should probably switch over to using crypto_engine rather than doing backlog handling manually.

Cheers,

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