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 Signed-off-by: Song Li Signed-off-by: Fan Wu --- 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 -#include #include #include #include @@ -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;