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

[Vasileios Almpanis]

On 9/2/26 5:31 PM, David Laight wrote:
> On Wed, 02 Sep 2026 12:21:19 +0200
> Vasileios Almpanis <vasilisalmpanis@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Vasileios Almpanis <vasilisalmpanis@xxxxxxxxx>
> > ---
> >   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@xxxxxxxxx/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@xxxxxxxxx>