[PATCH] slimbus: messaging: hold lock until transaction is completed

[PATCH] slimbus: messaging: hold lock until transaction is completed

[Changyul Lee]

slim_msg_response() drops lock after idr_find() and dereferences txn.
If the requester times out in the meantime, it frees the TID and
returns, so the handler writes the reply into msg->rbuf and calls
complete() on txn->comp, both of which have already benn freed.

Keep lock held until complete(txn->comp) finishes, so a requester
in slim_free_txn_tid() and cannot return while txn is still in use.
Call idr_remove() because slim_free_txn_tid() takes the same lock.

Fixes: afbdcc7c384b ("slimbus: Add messaging APIs to slimbus framework")

Signed-off-by: Changyul Lee <lcy8047@gmail.com>
---
 drivers/slimbus/messaging.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/slimbus/messaging.c b/drivers/slimbus/messaging.c
index e2dbe4a66b70..aa98f5480ba4 100644
--- a/drivers/slimbus/messaging.c
+++ b/drivers/slimbus/messaging.c
@@ -29,22 +29,24 @@ void slim_msg_response(struct slim_controller *ctrl, u8 *reply, u8 tid, u8 len)
 
 	spin_lock_irqsave(&ctrl->txn_lock, flags);
 	txn = idr_find(&ctrl->tid_idr, tid);
-	spin_unlock_irqrestore(&ctrl->txn_lock, flags);
-
-	if (txn == NULL)
+	if (txn == NULL) {
+		spin_unlock_irqrestore(&ctrl->txn_lock, flags);
 		return;
+	}
 
 	msg = txn->msg;
 	if (msg == NULL || msg->rbuf == NULL) {
+		spin_unlock_irqrestore(&ctrl->txn_lock, flags);
 		dev_err(ctrl->dev, "Got response to invalid TID:%d, len:%d\n",
 				tid, len);
 		return;
 	}
 
-	slim_free_txn_tid(ctrl, txn);
+	idr_remove(&ctrl->tid_idr, txn->tid);
 	memcpy(msg->rbuf, reply, len);
 	if (txn->comp)
 		complete(txn->comp);
+	spin_unlock_irqrestore(&ctrl->txn_lock, flags);
 
 	/* Remove runtime-pm vote now that response was received for TID txn */
 	pm_runtime_mark_last_busy(ctrl->dev);
-- 
2.43.0

Re: [PATCH] slimbus: messaging: hold lock until transaction is completed

[Greg Kroah-Hartman]

On Fri, Sep 11, 2026 at 12:59:11AM -0400, Changyul Lee wrote:
> slim_msg_response() drops lock after idr_find() and dereferences txn.
> If the requester times out in the meantime, it frees the TID and
> returns, so the handler writes the reply into msg->rbuf and calls
> complete() on txn->comp, both of which have already benn freed.
> 
> Keep lock held until complete(txn->comp) finishes, so a requester
> in slim_free_txn_tid() and cannot return while txn is still in use.
> Call idr_remove() because slim_free_txn_tid() takes the same lock.
> 
> Fixes: afbdcc7c384b ("slimbus: Add messaging APIs to slimbus framework")
> 
> Signed-off-by: Changyul Lee <lcy8047@gmail.com>

No blank line between these please.

And how was this found and tested?

thanks,

greg k-h