[PATCH 0/2] mailbox: ti-msgmgr: Fixes for polled rx and trailing bytes

[PATCH 0/2] mailbox: ti-msgmgr: Fixes for polled rx and trailing bytes

[Beleswar Padhi]

This series fixes two issues in ti_msgmgr_send_data(), found while
refactoring the ti-msgmgr mbox client (TI-SCI).

Patch 1 fixes a duplicate message transmission in polled rx mode.
Patch 2 stops reading past the end of the message buffer when the
message length is not a multiple of 4. This is a preparatory patch for
when the mbox client can send data which is not a multiple of 4.

This series is independent and can be applied directly.

Testing Done:
 - Boot tested on TI K3 J784S4 EVM.
 - Tested that these patches do not introduce any new warning or error.

Thanks,
Beleswar

Beleswar Padhi (2):
  mailbox: ti-msgmgr: Do not report rx poll timeout as a send failure
  mailbox: ti-msgmgr: Read exact number of trailing message bytes

 drivers/mailbox/ti-msgmgr.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 1/2] mailbox: ti-msgmgr: Do not report rx poll timeout as a send failure

[Beleswar Padhi]

In polled rx mode, used from system suspend until resume,
ti_msgmgr_send_data() writes the message to the tx queue and then
waits for the response. If no response arrives in time it returns
the poll error.

The mailbox core treats a ->send_data() error as a send failure.
So, it leaves the message queued without making it the active request,
and mbox_send_message() still returns success. The message is then
submitted again on the next tx_tick() or mbox_send_message(), after the
client has probably timed out and moved on. So, the firmware receives
the same message twice, and later requests are sent late and can have
responses matched to the wrong request.

The message has already been transmitted at this point, so return 0
and only log the missing response. The client detects this case with
its own response timeout anyways.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
 drivers/mailbox/ti-msgmgr.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/mailbox/ti-msgmgr.c b/drivers/mailbox/ti-msgmgr.c
index 8eb8df8d95a4c..425d5f9d9d0e3 100644
--- a/drivers/mailbox/ti-msgmgr.c
+++ b/drivers/mailbox/ti-msgmgr.c
@@ -445,12 +445,24 @@ static int ti_msgmgr_send_data(struct mbox_chan *chan, void *data)
 		data_reg += sizeof(u32);
 	}
 
-	/* If we are in polled mode, wait for a response before proceeding */
-	if (ti_msgmgr_chan_has_polled_queue_rx(message->chan_rx))
+	/*
+	 * If we are in polled mode, wait for a response before proceeding.
+	 *
+	 * The message has already been transmitted at this point, so do not
+	 * report a missing response as a send failure. Doing so would make
+	 * the mailbox core keep the message queued and submit it again later,
+	 * after the client has possibly given up on it. The client detects the
+	 * missing response by itself timing out.
+	 */
+	if (ti_msgmgr_chan_has_polled_queue_rx(message->chan_rx)) {
 		ret = ti_msgmgr_queue_rx_poll_timeout(message->chan_rx,
 						      message->timeout_rx_ms * 1000);
+		if (ret)
+			dev_err(dev, "Queue %s timed out waiting for response: %d\n",
+				qinst->name, ret);
+	}
 
-	return ret;
+	return 0;
 }
 
 /**
-- 
2.34.1

[PATCH 2/2] mailbox: ti-msgmgr: Read exact number of trailing message bytes

[Beleswar Padhi]

ti_msgmgr_send_data() copies the message to the data registers in u32
units. This has been harmless so far because the clients (TI-SCI)
always passed its preallocated message buffer, which is sized to the
maximum message size (60 or 64 bytes, a multiple of 4), so the extra
bytes read were still within the buffer. But, with a later TI-SCI
update, the message size can be varying now and not guaranteed to be a
multiple of 4, which would trigger KASAN out-of-bounds reports.

Therefore, Build the trailing word by reading one byte at a time, only
up to the message length. The value written to the register is
unchanged.

Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
 drivers/mailbox/ti-msgmgr.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mailbox/ti-msgmgr.c b/drivers/mailbox/ti-msgmgr.c
index 425d5f9d9d0e3..44457a896510f 100644
--- a/drivers/mailbox/ti-msgmgr.c
+++ b/drivers/mailbox/ti-msgmgr.c
@@ -425,10 +425,18 @@ static int ti_msgmgr_send_data(struct mbox_chan *chan, void *data)
 
 	trail_bytes = message->len % sizeof(u32);
 	if (trail_bytes) {
-		u32 data_trail = *word_data;
-
-		/* Ensure all unused data is 0 */
-		data_trail &= 0xFFFFFFFF >> (8 * (sizeof(u32) - trail_bytes));
+		/*
+		 * Read the trailing bytes one at a time instead of as a full
+		 * u32, as the message buffer may end right after them and a
+		 * u32 read would go past the end of it. This also leaves all
+		 * unused data as 0.
+		 */
+		u8 *byte_data = (u8 *)word_data;
+		u32 data_trail = 0;
+		int i;
+
+		for (i = 0; i < trail_bytes; i++)
+			data_trail |= byte_data[i] << (8 * i);
 		writel(data_trail, data_reg);
 		data_reg += sizeof(u32);
 	}
-- 
2.34.1

Re: [PATCH 2/2] mailbox: ti-msgmgr: Read exact number of trailing message bytes

[Vignesh Raghavendra]

On 30/09/26 01:31, Beleswar Padhi wrote:
> ti_msgmgr_send_data() copies the message to the data registers in u32
> units. This has been harmless so far because the clients (TI-SCI)
> always passed its preallocated message buffer, which is sized to the
> maximum message size (60 or 64 bytes, a multiple of 4), so the extra
> bytes read were still within the buffer. But, with a later TI-SCI
> update, the message size can be varying now and not guaranteed to be a
> multiple of 4, which would trigger KASAN out-of-bounds reports.
> 
> Therefore, Build the trailing word by reading one byte at a time, only
> up to the message length. The value written to the register is
> unchanged.
> 
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>

This looks like a bug fixes and needs a Fixes: tag?

> ---
>  drivers/mailbox/ti-msgmgr.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mailbox/ti-msgmgr.c b/drivers/mailbox/ti-msgmgr.c
> index 425d5f9d9d0e3..44457a896510f 100644
> --- a/drivers/mailbox/ti-msgmgr.c
> +++ b/drivers/mailbox/ti-msgmgr.c
> @@ -425,10 +425,18 @@ static int ti_msgmgr_send_data(struct mbox_chan *chan, void *data)
>  
>  	trail_bytes = message->len % sizeof(u32);
>  	if (trail_bytes) {
> -		u32 data_trail = *word_data;
> -
> -		/* Ensure all unused data is 0 */
> -		data_trail &= 0xFFFFFFFF >> (8 * (sizeof(u32) - trail_bytes));
> +		/*
> +		 * Read the trailing bytes one at a time instead of as a full
> +		 * u32, as the message buffer may end right after them and a
> +		 * u32 read would go past the end of it. This also leaves all
> +		 * unused data as 0.
> +		 */
> +		u8 *byte_data = (u8 *)word_data;
> +		u32 data_trail = 0;
> +		int i;
> +
> +		for (i = 0; i < trail_bytes; i++)
> +			data_trail |= byte_data[i] << (8 * i);
>  		writel(data_trail, data_reg);
>  		data_reg += sizeof(u32);
>  	}

-- 
Regards
Vignesh
https://ti.com/opensource

Re: [PATCH 2/2] mailbox: ti-msgmgr: Read exact number of trailing message bytes

[Padhi, Beleswar]

On 9/30/2026 9:43 AM, Vignesh Raghavendra wrote:
>
> On 30/09/26 01:31, Beleswar Padhi wrote:
>> ti_msgmgr_send_data() copies the message to the data registers in u32
>> units. This has been harmless so far because the clients (TI-SCI)
>> always passed its preallocated message buffer, which is sized to the
>> maximum message size (60 or 64 bytes, a multiple of 4), so the extra
>> bytes read were still within the buffer. But, with a later TI-SCI
>> update, the message size can be varying now and not guaranteed to be a
>> multiple of 4, which would trigger KASAN out-of-bounds reports.
>>
>> Therefore, Build the trailing word by reading one byte at a time, only
>> up to the message length. The value written to the register is
>> unchanged.
>>
>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> This looks like a bug fixes and needs a Fixes: tag?


All mbox clients of ti-msgmgr currently send 60/64 byte sized messages. 
So this
is not a bug today. It is only a preparatory patch for future mbox 
clients which
can send message sizes like 19 bytes etc.

Thanks,
Beleswar

>
>> ---
>>   drivers/mailbox/ti-msgmgr.c | 16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mailbox/ti-msgmgr.c b/drivers/mailbox/ti-msgmgr.c
>> index 425d5f9d9d0e3..44457a896510f 100644
>> --- a/drivers/mailbox/ti-msgmgr.c
>> +++ b/drivers/mailbox/ti-msgmgr.c
>> @@ -425,10 +425,18 @@ static int ti_msgmgr_send_data(struct mbox_chan *chan, void *data)
>>   
>>   	trail_bytes = message->len % sizeof(u32);
>>   	if (trail_bytes) {
>> -		u32 data_trail = *word_data;
>> -
>> -		/* Ensure all unused data is 0 */
>> -		data_trail &= 0xFFFFFFFF >> (8 * (sizeof(u32) - trail_bytes));
>> +		/*
>> +		 * Read the trailing bytes one at a time instead of as a full
>> +		 * u32, as the message buffer may end right after them and a
>> +		 * u32 read would go past the end of it. This also leaves all
>> +		 * unused data as 0.
>> +		 */
>> +		u8 *byte_data = (u8 *)word_data;
>> +		u32 data_trail = 0;
>> +		int i;
>> +
>> +		for (i = 0; i < trail_bytes; i++)
>> +			data_trail |= byte_data[i] << (8 * i);
>>   		writel(data_trail, data_reg);
>>   		data_reg += sizeof(u32);
>>   	}

Re: [PATCH 2/2] mailbox: ti-msgmgr: Read exact number of trailing message bytes

[Andrew Davis]

On 9/30/26 1:37 AM, Padhi, Beleswar wrote:
> 
> On 9/30/2026 9:43 AM, Vignesh Raghavendra wrote:
>>
>> On 30/09/26 01:31, Beleswar Padhi wrote:
>>> ti_msgmgr_send_data() copies the message to the data registers in u32
>>> units. This has been harmless so far because the clients (TI-SCI)
>>> always passed its preallocated message buffer, which is sized to the
>>> maximum message size (60 or 64 bytes, a multiple of 4), so the extra
>>> bytes read were still within the buffer. But, with a later TI-SCI
>>> update, the message size can be varying now and not guaranteed to be a
>>> multiple of 4, which would trigger KASAN out-of-bounds reports.
>>>
>>> Therefore, Build the trailing word by reading one byte at a time, only
>>> up to the message length. The value written to the register is
>>> unchanged.
>>>
>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>> This looks like a bug fixes and needs a Fixes: tag?
> 
> 
> All mbox clients of ti-msgmgr currently send 60/64 byte sized messages. So this
> is not a bug today. It is only a preparatory patch for future mbox clients which
> can send message sizes like 19 bytes etc.
> 

It's still a bug, even if no one happened to run into it yet.
Probably good to add the Fixes tag anyway, just in case something
else every gets backported also that needs the non-4-byte aligned
reads.

Andrew

> Thanks,
> Beleswar
> 
>>
>>> ---
>>>   drivers/mailbox/ti-msgmgr.c | 16 ++++++++++++----
>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mailbox/ti-msgmgr.c b/drivers/mailbox/ti-msgmgr.c
>>> index 425d5f9d9d0e3..44457a896510f 100644
>>> --- a/drivers/mailbox/ti-msgmgr.c
>>> +++ b/drivers/mailbox/ti-msgmgr.c
>>> @@ -425,10 +425,18 @@ static int ti_msgmgr_send_data(struct mbox_chan *chan, void *data)
>>>       trail_bytes = message->len % sizeof(u32);
>>>       if (trail_bytes) {
>>> -        u32 data_trail = *word_data;
>>> -
>>> -        /* Ensure all unused data is 0 */
>>> -        data_trail &= 0xFFFFFFFF >> (8 * (sizeof(u32) - trail_bytes));
>>> +        /*
>>> +         * Read the trailing bytes one at a time instead of as a full
>>> +         * u32, as the message buffer may end right after them and a
>>> +         * u32 read would go past the end of it. This also leaves all
>>> +         * unused data as 0.
>>> +         */
>>> +        u8 *byte_data = (u8 *)word_data;
>>> +        u32 data_trail = 0;
>>> +        int i;
>>> +
>>> +        for (i = 0; i < trail_bytes; i++)
>>> +            data_trail |= byte_data[i] << (8 * i);
>>>           writel(data_trail, data_reg);
>>>           data_reg += sizeof(u32);
>>>       }

Re: [PATCH 2/2] mailbox: ti-msgmgr: Read exact number of trailing message bytes

[Padhi, Beleswar]

On 9/30/2026 8:34 PM, Andrew Davis wrote:
> On 9/30/26 1:37 AM, Padhi, Beleswar wrote:
>>
>> On 9/30/2026 9:43 AM, Vignesh Raghavendra wrote:
>>>
>>> On 30/09/26 01:31, Beleswar Padhi wrote:
>>>> ti_msgmgr_send_data() copies the message to the data registers in u32
>>>> units. This has been harmless so far because the clients (TI-SCI)
>>>> always passed its preallocated message buffer, which is sized to the
>>>> maximum message size (60 or 64 bytes, a multiple of 4), so the extra
>>>> bytes read were still within the buffer. But, with a later TI-SCI
>>>> update, the message size can be varying now and not guaranteed to be a
>>>> multiple of 4, which would trigger KASAN out-of-bounds reports.
>>>>
>>>> Therefore, Build the trailing word by reading one byte at a time, only
>>>> up to the message length. The value written to the register is
>>>> unchanged.
>>>>
>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>> This looks like a bug fixes and needs a Fixes: tag?
>>
>>
>> All mbox clients of ti-msgmgr currently send 60/64 byte sized 
>> messages. So this
>> is not a bug today. It is only a preparatory patch for future mbox 
>> clients which
>> can send message sizes like 19 bytes etc.
>>
>
> It's still a bug, even if no one happened to run into it yet.


So should every pointer be checked against NULL before dereferencing
in all drivers by that logic then?

> Probably good to add the Fixes tag anyway, just in case something
> else every gets backported also that needs the non-4-byte aligned
> reads.


Fair, makes sense. Will add Fixes tag in v3.

Thanks,
Beleswar

>
> Andrew
>
>> Thanks,
>> Beleswar
>>
>>>
>>>> ---
>>>>   drivers/mailbox/ti-msgmgr.c | 16 ++++++++++++----
>>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mailbox/ti-msgmgr.c b/drivers/mailbox/ti-msgmgr.c
>>>> index 425d5f9d9d0e3..44457a896510f 100644
>>>> --- a/drivers/mailbox/ti-msgmgr.c
>>>> +++ b/drivers/mailbox/ti-msgmgr.c
>>>> @@ -425,10 +425,18 @@ static int ti_msgmgr_send_data(struct 
>>>> mbox_chan *chan, void *data)
>>>>       trail_bytes = message->len % sizeof(u32);
>>>>       if (trail_bytes) {
>>>> -        u32 data_trail = *word_data;
>>>> -
>>>> -        /* Ensure all unused data is 0 */
>>>> -        data_trail &= 0xFFFFFFFF >> (8 * (sizeof(u32) - 
>>>> trail_bytes));
>>>> +        /*
>>>> +         * Read the trailing bytes one at a time instead of as a full
>>>> +         * u32, as the message buffer may end right after them and a
>>>> +         * u32 read would go past the end of it. This also leaves all
>>>> +         * unused data as 0.
>>>> +         */
>>>> +        u8 *byte_data = (u8 *)word_data;
>>>> +        u32 data_trail = 0;
>>>> +        int i;
>>>> +
>>>> +        for (i = 0; i < trail_bytes; i++)
>>>> +            data_trail |= byte_data[i] << (8 * i);
>>>>           writel(data_trail, data_reg);
>>>>           data_reg += sizeof(u32);
>>>>       }
>

Re: [PATCH 2/2] mailbox: ti-msgmgr: Read exact number of trailing message bytes

[Andrew Davis]

On 9/30/26 10:11 AM, Padhi, Beleswar wrote:
> 
> On 9/30/2026 8:34 PM, Andrew Davis wrote:
>> On 9/30/26 1:37 AM, Padhi, Beleswar wrote:
>>>
>>> On 9/30/2026 9:43 AM, Vignesh Raghavendra wrote:
>>>>
>>>> On 30/09/26 01:31, Beleswar Padhi wrote:
>>>>> ti_msgmgr_send_data() copies the message to the data registers in u32
>>>>> units. This has been harmless so far because the clients (TI-SCI)
>>>>> always passed its preallocated message buffer, which is sized to the
>>>>> maximum message size (60 or 64 bytes, a multiple of 4), so the extra
>>>>> bytes read were still within the buffer. But, with a later TI-SCI
>>>>> update, the message size can be varying now and not guaranteed to be a
>>>>> multiple of 4, which would trigger KASAN out-of-bounds reports.
>>>>>
>>>>> Therefore, Build the trailing word by reading one byte at a time, only
>>>>> up to the message length. The value written to the register is
>>>>> unchanged.
>>>>>
>>>>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>>>> This looks like a bug fixes and needs a Fixes: tag?
>>>
>>>
>>> All mbox clients of ti-msgmgr currently send 60/64 byte sized messages. So this
>>> is not a bug today. It is only a preparatory patch for future mbox clients which
>>> can send message sizes like 19 bytes etc.
>>>
>>
>> It's still a bug, even if no one happened to run into it yet.
> 
> 
> So should every pointer be checked against NULL before dereferencing
> in all drivers by that logic then?
> 

I get what you are saying, but in many cases if passing NULL is
allowed or not is an implied contract between caller and callee.
In this case the caller explicitly states the message length to
read, and the function doesn't respect that value. Which is a more
clear case of broken contract, and therefor a bug IMHO.

Andrew

>> Probably good to add the Fixes tag anyway, just in case something
>> else every gets backported also that needs the non-4-byte aligned
>> reads.
> 
> 
> Fair, makes sense. Will add Fixes tag in v3.
> 
> Thanks,
> Beleswar
> 
>>
>> Andrew
>>
>>> Thanks,
>>> Beleswar
>>>
>>>>
>>>>> ---
>>>>>   drivers/mailbox/ti-msgmgr.c | 16 ++++++++++++----
>>>>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mailbox/ti-msgmgr.c b/drivers/mailbox/ti-msgmgr.c
>>>>> index 425d5f9d9d0e3..44457a896510f 100644
>>>>> --- a/drivers/mailbox/ti-msgmgr.c
>>>>> +++ b/drivers/mailbox/ti-msgmgr.c
>>>>> @@ -425,10 +425,18 @@ static int ti_msgmgr_send_data(struct mbox_chan *chan, void *data)
>>>>>       trail_bytes = message->len % sizeof(u32);
>>>>>       if (trail_bytes) {
>>>>> -        u32 data_trail = *word_data;
>>>>> -
>>>>> -        /* Ensure all unused data is 0 */
>>>>> -        data_trail &= 0xFFFFFFFF >> (8 * (sizeof(u32) - trail_bytes));
>>>>> +        /*
>>>>> +         * Read the trailing bytes one at a time instead of as a full
>>>>> +         * u32, as the message buffer may end right after them and a
>>>>> +         * u32 read would go past the end of it. This also leaves all
>>>>> +         * unused data as 0.
>>>>> +         */
>>>>> +        u8 *byte_data = (u8 *)word_data;
>>>>> +        u32 data_trail = 0;
>>>>> +        int i;
>>>>> +
>>>>> +        for (i = 0; i < trail_bytes; i++)
>>>>> +            data_trail |= byte_data[i] << (8 * i);
>>>>>           writel(data_trail, data_reg);
>>>>>           data_reg += sizeof(u32);
>>>>>       }
>>