[PATCH v3 0/2] rpmsg: glink: smem: robustness and debuggability fixes

[PATCH v3 0/2] rpmsg: glink: smem: robustness and debuggability fixes

[Chunkai Deng]

This series makes two small improvements to the SMEM GLINK transport:

1. Validate the raw FIFO head/tail indices (and caller-supplied
   offset/count) against the FIFO length in the read/write helpers,
   so a broken index is rejected before it turns into an
   out-of-bounds access into the FIFO.

2. Pass dev_name(&smem->dev) as the IRQ name instead of the static
   string "glink-smem".  On platforms with multiple remoteprocs each
   SMEM GLINK instance registers its own IRQ, and the unique device
   name makes each entry in /proc/interrupts identifiable without
   adding a new struct field.

Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
---
Changes in v3:
- Rework the FIFO-index patch per review: validate the raw head/tail
  values (not a derived length or post-normalized index), use
  dev_warn_ratelimited() for remote-written indices and WARN_ON_ONCE()
  for local ones, and bounds-check offset/count and hlen/dlen.
  Retitled to "rpmsg: glink: smem: Validate FIFO indices before use".
- No changes to the IRQ-name patch; carrying Dmitry's Reviewed-by.
- Link to v2: https://patch.msgid.link/20260617-rpmsg-improvements-v2-0-477d4eb569dc@oss.qualcomm.com

Changes in v2:
- Drop the modulo patch ("rpmsg: glink: smem: Use modulo for FIFO
  tail wrap-around in rx_advance"); per review, it papers over an
  unreachable scenario without restoring a valid protocol state.
  Proper RX-side validation of chunk_size against rx_pipe->length
  will be sent as a separate patch.
- "rpmsg: glink: smem: Add WARN_ON_ONCE for FIFO index invariants":
  - Drop the trailing comma after "skipped" in the commit message
    body (per review).
  - Add Fixes: caf989c350e8 ("rpmsg: glink: Introduce glink smem
    based transport") and Cc: stable@vger.kernel.org (per review).
- Carry Dmitry's Reviewed-by on "rpmsg: glink: smem: Use device
  name as IRQ name" (no code changes since v1).
- Link to v1: https://patch.msgid.link/20260603-rpmsg-improvements-v1-0-dcfc22ed69f7@oss.qualcomm.com

---
Chunkai Deng (2):
      rpmsg: glink: smem: Use device name as IRQ name
      rpmsg: glink: smem: Validate FIFO indices before use

 drivers/rpmsg/qcom_glink_smem.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)
---
base-commit: f7af91adc230aa99e23330ecf85bc9badd9780ad
change-id: 20260601-rpmsg-improvements-643a6e133f66

Best regards,
--  
Chunkai Deng <chunkai.deng@oss.qualcomm.com>

[PATCH v3 1/2] rpmsg: glink: smem: Use device name as IRQ name

[Chunkai Deng]

A SoC typically has multiple remoteprocs (ADSP, MPSS, CDSP, etc.), each
registering its own SMEM GLINK instance. With the static name "glink-smem"
all instances appear identically in /proc/interrupts, making it impossible
to associate an IRQ with a specific remote processor.

Pass dev_name(&smem->dev) to devm_request_irq() instead. The device name
is already set to "<parent>:<edge-node>" which uniquely identifies each
instance without requiring an additional field in the driver struct.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
---
 drivers/rpmsg/qcom_glink_smem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
index 62adc4db2317..edab912557ac 100644
--- a/drivers/rpmsg/qcom_glink_smem.c
+++ b/drivers/rpmsg/qcom_glink_smem.c
@@ -307,7 +307,7 @@ struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
 	smem->irq = of_irq_get(smem->dev.of_node, 0);
 	ret = devm_request_irq(&smem->dev, smem->irq, qcom_glink_smem_intr,
 			       IRQF_NO_SUSPEND | IRQF_NO_AUTOEN,
-			       "glink-smem", smem);
+			       dev_name(&smem->dev), smem);
 	if (ret) {
 		dev_err(&smem->dev, "failed to request IRQ\n");
 		goto err_put_dev;

-- 
2.34.1

[PATCH v3 2/2] rpmsg: glink: smem: Validate FIFO indices before use

[Chunkai Deng]

The FIFO read/write helpers assume the head and tail indices stay within
[0, pipe->native.length) and use them directly as offsets into the
mapped FIFO region. If that invariant is ever broken, the subsequent
memcpy or memcpy_fromio would access memory outside the FIFO.

Validate the raw head and tail values (and, where applicable, the
caller-supplied offset/count) against pipe->native.length in the
avail/peek/write helpers. Checking the derived length or the
post-normalized index is not sufficient: two out-of-range indices can
still produce an in-range difference, and reducing a bad index modulo
the FIFO length hides the original invariant violation.

Fixes: caf989c350e8 ("rpmsg: glink: Introduce glink smem based transport")
Cc: stable@vger.kernel.org
Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
---
 drivers/rpmsg/qcom_glink_smem.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
index edab912557ac..df3081855b02 100644
--- a/drivers/rpmsg/qcom_glink_smem.c
+++ b/drivers/rpmsg/qcom_glink_smem.c
@@ -85,6 +85,22 @@ static size_t glink_smem_rx_avail(struct qcom_glink_pipe *np)
 	head = le32_to_cpu(*pipe->head);
 	tail = le32_to_cpu(*pipe->tail);
 
+	/*
+	 * head is written by the remote peer via SMEM; tail is written
+	 * locally. Check the raw values, not a derived length: two
+	 * out-of-range indices can still produce an in-range difference,
+	 * and reducing a bad index modulo the FIFO length would hide the
+	 * invariant violation.
+	 */
+	if (unlikely(head >= pipe->native.length)) {
+		dev_warn_ratelimited(&smem->dev,
+				     "rx head out of range: head=%u length=%zu\n",
+				     head, pipe->native.length);
+		return 0;
+	}
+	if (WARN_ON_ONCE(tail >= pipe->native.length))
+		return 0;
+
 	if (head < tail)
 		return pipe->native.length - tail + head;
 	else
@@ -99,6 +115,11 @@ static void glink_smem_rx_peek(struct qcom_glink_pipe *np,
 	u32 tail;
 
 	tail = le32_to_cpu(*pipe->tail);
+	if (WARN_ON_ONCE(tail >= pipe->native.length))
+		return;
+	if (WARN_ON_ONCE(offset + count > pipe->native.length))
+		return;
+
 	tail += offset;
 	if (tail >= pipe->native.length)
 		tail -= pipe->native.length;
@@ -129,6 +150,7 @@ static void glink_smem_rx_advance(struct qcom_glink_pipe *np,
 static size_t glink_smem_tx_avail(struct qcom_glink_pipe *np)
 {
 	struct glink_smem_pipe *pipe = to_smem_pipe(np);
+	struct qcom_glink_smem *smem = pipe->smem;
 	u32 head;
 	u32 tail;
 	u32 avail;
@@ -136,6 +158,19 @@ static size_t glink_smem_tx_avail(struct qcom_glink_pipe *np)
 	head = le32_to_cpu(*pipe->head);
 	tail = le32_to_cpu(*pipe->tail);
 
+	/*
+	 * head is written locally; tail is written by the remote peer via
+	 * SMEM. Check the raw values, not a derived length.
+	 */
+	if (WARN_ON_ONCE(head >= pipe->native.length))
+		return 0;
+	if (unlikely(tail >= pipe->native.length)) {
+		dev_warn_ratelimited(&smem->dev,
+				     "tx tail out of range: tail=%u length=%zu\n",
+				     tail, pipe->native.length);
+		return 0;
+	}
+
 	if (tail <= head)
 		avail = pipe->native.length - head + tail;
 	else
@@ -177,6 +212,10 @@ static void glink_smem_tx_write(struct qcom_glink_pipe *glink_pipe,
 	unsigned int head;
 
 	head = le32_to_cpu(*pipe->head);
+	if (WARN_ON_ONCE(head >= pipe->native.length))
+		return;
+	if (WARN_ON_ONCE(hlen + dlen > pipe->native.length))
+		return;
 
 	head = glink_smem_tx_write_one(pipe, head, hdr, hlen);
 	head = glink_smem_tx_write_one(pipe, head, data, dlen);

-- 
2.34.1

Re: [PATCH v3 1/2] rpmsg: glink: smem: Use device name as IRQ name

[Konrad Dybcio]

On 7/20/26 3:25 PM, Chunkai Deng wrote:
> A SoC typically has multiple remoteprocs (ADSP, MPSS, CDSP, etc.), each
> registering its own SMEM GLINK instance. With the static name "glink-smem"
> all instances appear identically in /proc/interrupts, making it impossible
> to associate an IRQ with a specific remote processor.
> 
> Pass dev_name(&smem->dev) to devm_request_irq() instead. The device name
> is already set to "<parent>:<edge-node>" which uniquely identifies each
> instance without requiring an additional field in the driver struct.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad