[PATCH] slimbus: qcom-ngd-ctrl: Release the TX descriptor on error paths

[PATCH] slimbus: qcom-ngd-ctrl: Release the TX descriptor on error paths

[Babanpreet Singh]

qcom_slim_ngd_xfer_msg() leaks the claimed TX descriptor on two error
paths; QCOM_SLIM_NGD_DESC_NUM such failures fill the ring for good and
every later transfer fails with -ENOMEM. Give the descriptor back.

Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
Fixes: 52490169cddf ("slimbus: ngd: add stream support")
Assisted-by: claude-opus-5
Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
---
 drivers/slimbus/qcom-ngd-ctrl.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 3071e46d03bea..697811b2c1e97 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -546,6 +546,25 @@ static u32 *qcom_slim_ngd_tx_msg_get(struct qcom_slim_ngd_ctrl *ctrl, int len,
 	return desc->base;
 }
 
+/*
+ * Give back a descriptor obtained from qcom_slim_ngd_tx_msg_get() that will
+ * not be submitted to the DMA engine.
+ *
+ * The ring is accounted by count rather than by index: tx_tail counts the
+ * descriptors handed out and tx_head the ones the controller is finished
+ * with, and qcom_slim_ngd_tx_msg_dma_cb() is the only other place that
+ * advances tx_head. A descriptor that is never posted therefore has to
+ * advance it here, or the ring loses that slot for good.
+ */
+static void qcom_slim_ngd_tx_msg_put(struct qcom_slim_ngd_ctrl *ctrl)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
+	ctrl->tx_head = (ctrl->tx_head + 1) % QCOM_SLIM_NGD_DESC_NUM;
+	spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
+}
+
 static void qcom_slim_ngd_tx_msg_dma_cb(void *args)
 {
 	struct qcom_slim_ngd_dma_desc *desc = args;
@@ -848,6 +867,7 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
 		ret = slim_alloc_txn_tid(sctrl, txn);
 		if (ret) {
 			dev_err(ctrl->dev, "Unable to allocate TID\n");
+			qcom_slim_ngd_tx_msg_put(ctrl);
 			return ret;
 		}
 
@@ -887,6 +907,7 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
 	mutex_lock(&ctrl->tx_lock);
 	ret = qcom_slim_ngd_tx_msg_post(ctrl, pbuf, txn->rl);
 	if (ret) {
+		qcom_slim_ngd_tx_msg_put(ctrl);
 		mutex_unlock(&ctrl->tx_lock);
 		return ret;
 	}
-- 
2.43.0

Re: [PATCH] slimbus: qcom-ngd-ctrl: Release the TX descriptor on error paths

[Babanpreet Singh]

Hi Srinivas,

Gentle ping on this one. It has been about three and a half weeks and I
could not find it in char-misc-next or your for-next, so I wanted to
check whether it fell through the cracks or whether you would like
something changed.

Thanks,
Babanpreet

Re: [PATCH] slimbus: qcom-ngd-ctrl: Release the TX descriptor on error paths

[Srinivas Kandagatla]

On 8/20/26 4:52 AM, Babanpreet Singh wrote:
> qcom_slim_ngd_xfer_msg() leaks the claimed TX descriptor on two error
> paths; QCOM_SLIM_NGD_DESC_NUM such failures fill the ring for good and
> every later transfer fails with -ENOMEM. Give the descriptor back.

>
Are you hitting this on a real target or is this something done from
static analysis?

How was this test verified?

> Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
> Fixes: 52490169cddf ("slimbus: ngd: add stream support")Missing Stable tag.
> Assisted-by: claude-opus-5
> Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
> ---
>  drivers/slimbus/qcom-ngd-ctrl.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
> index 3071e46d03bea..697811b2c1e97 100644
> --- a/drivers/slimbus/qcom-ngd-ctrl.c
> +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> @@ -546,6 +546,25 @@ static u32 *qcom_slim_ngd_tx_msg_get(struct qcom_slim_ngd_ctrl *ctrl, int len,
>  	return desc->base;
>  }
>  
> +/*
> + * Give back a descriptor obtained from qcom_slim_ngd_tx_msg_get() that will
> + * not be submitted to the DMA engine.
> + *
> + * The ring is accounted by count rather than by index: tx_tail counts the
> + * descriptors handed out and tx_head the ones the controller is finished
> + * with, and qcom_slim_ngd_tx_msg_dma_cb() is the only other place that
> + * advances tx_head. A descriptor that is never posted therefore has to
> + * advance it here, or the ring loses that slot for good.
> + */
> +static void qcom_slim_ngd_tx_msg_put(struct qcom_slim_ngd_ctrl *ctrl)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ctrl->tx_buf_lock, flags);
> +	ctrl->tx_head = (ctrl->tx_head + 1) % QCOM_SLIM_NGD_DESC_NUM;
> +	spin_unlock_irqrestore(&ctrl->tx_buf_lock, flags);
> +}

> +
>  static void qcom_slim_ngd_tx_msg_dma_cb(void *args)
>  {
>  	struct qcom_slim_ngd_dma_desc *desc = args;
> @@ -848,6 +867,7 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
>  		ret = slim_alloc_txn_tid(sctrl, txn);
>  		if (ret) {
>  			dev_err(ctrl->dev, "Unable to allocate TID\n");
> +			qcom_slim_ngd_tx_msg_put(ctrl);
>  			return ret;
>  		}
>  
> @@ -887,6 +907,7 @@ static int qcom_slim_ngd_xfer_msg(struct slim_controller *sctrl,
>  	mutex_lock(&ctrl->tx_lock);
>  	ret = qcom_slim_ngd_tx_msg_post(ctrl, pbuf, txn->rl);
>  	if (ret) {
> +		qcom_slim_ngd_tx_msg_put(ctrl);
>  		mutex_unlock(&ctrl->tx_lock);
>  		return ret;
>  	}

Re: [PATCH] slimbus: qcom-ngd-ctrl: Release the TX descriptor on error paths

[Babanpreet Singh]

Hi Srinivas,

Static analysis, then reading. smatch flagged the txn->msg check in
qcom_slim_ngd_xfer_msg() and while I was looking at that function I
noticed the two returns between tx_msg_get() and tx_msg_post(). I do not
have a SLIMbus target. The -ENOMEM consequence in the commit message is
derived from the code rather than observed: tx_head only moves in
qcom_slim_ngd_tx_msg_dma_cb() and in the re-init path, so a descriptor
that is claimed and never posted has no way back into the ring.

Verification was build only.

On the stable tag: I can send a v2 with Cc: stable and the "found by
static analysis, compile tested only" wording.

Thanks,
Babanpreet

Re: [PATCH] slimbus: qcom-ngd-ctrl: Release the TX descriptor on error paths

[Greg Kroah-Hartman]

On Thu, Oct 01, 2026 at 11:25:20PM +0000, Babanpreet Singh wrote:
> Hi Srinivas,
> 
> Static analysis, then reading. smatch flagged the txn->msg check in
> qcom_slim_ngd_xfer_msg() and while I was looking at that function I
> noticed the two returns between tx_msg_get() and tx_msg_post(). I do not
> have a SLIMbus target. The -ENOMEM consequence in the commit message is
> derived from the code rather than observed: tx_head only moves in
> qcom_slim_ngd_tx_msg_dma_cb() and in the re-init path, so a descriptor
> that is claimed and never posted has no way back into the ring.
> 
> Verification was build only.
> 
> On the stable tag: I can send a v2 with Cc: stable and the "found by
> static analysis, compile tested only" wording.

We have no context here at all, please never top-post, as my bot says:


A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top