On 6/15/2026 6:54 AM, Bjorn Andersson wrote: > On Wed, Jun 03, 2026 at 06:14:30PM +0800, Chunkai Deng wrote: >> glink_smem_rx_advance() wraps the tail index with a single subtraction, >> which only corrects for one full wrap. The advance count is derived from >> remote-supplied packet fields (up to sizeof(glink_msg) + 0xffff bytes); >> if such a count reaches or exceeds pipe->native.length, the tail remains >> outside [0, length) after the subtraction and the next FIFO access uses >> an out-of-bounds offset. >> >> Use modulo so the tail is always normalised back into [0, length), >> keeping it consistent with the index bounds enforced by the WARN_ON_ONCE >> checks added to the FIFO helpers. >> >> Signed-off-by: Chunkai Deng <[email protected]> >> --- >> 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 42ad315d7910..4f143921b719 100644 >> --- a/drivers/rpmsg/qcom_glink_smem.c >> +++ b/drivers/rpmsg/qcom_glink_smem.c >> @@ -129,7 +129,7 @@ static void glink_smem_rx_advance(struct qcom_glink_pipe >> *np, >> >> tail += count; >> if (tail >= pipe->native.length) >> - tail -= pipe->native.length; >> + tail %= pipe->native.length; > It seems unlikely that the "tail" will point to the start of a valid > header after this. How can we make sure this is more robust? > > Regards, > Bjorn
Agreed -- modulo only normalises the offset arithmetically; the protocol state is already desynchronised at that point and the tail won't land on a valid header boundary. The patch hides the symptom (OOB access) without addressing the underlying issue (a bogus remote-supplied size). I'll drop this patch in v2 and keep just the WARN_ON_ONCE patch as a defensive backstop. If a proper hardening of the receive path is wanted (e.g. validating chunk_size against rx_pipe->length on header peek, mirroring the existing "tlen >= tx_pipe->length" check on the TX side), I'll send it as a separate patch so it can be discussed on its own. Thanks, Chunkai >> >> *pipe->tail = cpu_to_le32(tail); >> } >> >> -- >> 2.34.1 >>
