Re: [PATCH] ihex: Fix 16 bit truncation in ihex_binrec_size()

[Vasileios Almpanis <vasilisalmpanis@gmail.com>]

On 9/2/26 5:31 PM, David Laight wrote:
> On Wed, 02 Sep 2026 12:21:19 +0200
> Vasileios Almpanis <vasilisalmpanis@gmail.com> wrote:
>
>> ihex_binrec_size() returns uint16_t while computing be16_to_cpu(p->len) +
>> sizeof(struct ihex_binrec), so record lengths of 65530 and above wrap.
>> __ihex_next_binrec() uses the result as the offset to the next record. A
>> length of 65530 gives an advance of zero, so ihex_validate_fw() spins on
>> the same record forever. Lengths of 65531 to 65535 advance by 4 or 8
>> instead of 65544, so a 14 byte image passes validation while its first
>> record claims 65535 bytes of payload. emi26_load_firmware() passes it to
>> emi26_writememory(), which kmemdup()s 65535 bytes out of a 14 byte buffer
>> producing the following splat:
>>
>>    BUG: KASAN: vmalloc-out-of-bounds in kmemdup_noprof+0x3b/0x50
>>    Read of size 65535 at addr ffffc90000075006 by task kworker/11:1/174
>>    Workqueue: usb_hub_wq hub_event
>>    Call Trace:
>>     <TASK>
>>     kasan_check_range+0x10f/0x1e0
>>     __asan_memcpy+0x23/0x60
>>     kmemdup_noprof+0x3b/0x50
>>     emi26_writememory+0x29/0xd0
>>     emi26_probe+0x2d1/0xb64
>>
>> Return size_t so neither the addition nor the following ALIGN() can wrap.
> The ALIGN() can't wrap, the u16 value is promoted to 'int' before anything
> is done with it.
> What it does save is the pointless '&= 0xffff' after the add.
>
> Since the result is added to a pointer it will need promoting to 'long'.
> But the compiler can assume that adding sizeof(*p) will zero the high
> bits and nothing extra is generated.
> (Not that this is a super-hot path...)
>
>> Fixes: 9fb4ab4d3dd6 ("ihex: Simplify next record offset calculation")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Vasileios Almpanis <vasilisalmpanis@gmail.com>
>> ---
>>   include/linux/ihex.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/ihex.h b/include/linux/ihex.h
>> index b824877e6d1b..0da1c4e3e693 100644
>> --- a/include/linux/ihex.h
>> +++ b/include/linux/ihex.h
>> @@ -21,7 +21,7 @@ struct ihex_binrec {
>>   	uint8_t data[];
>>   } __attribute__((packed));
>>   
>> -static inline uint16_t ihex_binrec_size(const struct ihex_binrec *p)
>> +static inline size_t ihex_binrec_size(const struct ihex_binrec *p)
>>   {
>>   	return be16_to_cpu(p->len) + sizeof(*p);
> I'd always put those in the other order - matching the memory contents.
> (But changing it would be churn.)
>
> David
Hi David,

Thanks for taking the time to review my patch.
I had already posted a v2 before your mail arrived.
https://lore.kernel.org/all/20260902-ihex-v2-1-30bb117cfc77@gmail.com/T/#u
So the problem in this case is the fact that I misplace where the wrap 
happens.
Do you think it makes sense to send a v3 to fix the commit message?

Kind regards,

Vasileios Almpanis <vasilisalmpanis@gmail.com>