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

["Padhi, Beleswar" <b-padhi@ti.com>]

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);
>>>>       }
>