Re: [PATCH 3/3] rpmsg: glink: smem: Use modulo for FIFO tail wrap-around in rx_advance

[Chunkai Deng <chunkai.deng@oss.qualcomm.com>]

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 <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 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
>>