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

[Vasileios Almpanis]

On 2026-09-02 13:06:27+02:00, Greg Kroah-Hartman wrote:
> On Wed, Sep 02, 2026 at 12:21:19PM +0200, Vasileios Almpanis 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
> 
> Is this a real firmware image being sent?  Or a fake one?
> 

I used fake images. 14 bytes long for the out-of-bounds bug and 6 bytes
for the infinite loop. I passed them through the sysfs fallback loader.

Out-Of-Bounds bug image:
00 00 00 00 ff ff 00 00 00 00 00 00 00 00

Infinite-loop image:
00 00 00 00 ff fa

> And root triggers this, right?

Yes its root only. The attributes are accessible only through root
and it also needs raw-gadget.

> 
> > Return size_t so neither the addition nor the following ALIGN() can wrap.
> > 
> > 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)
> 
> but size_t changes depending on the platform, right?  Why not make it
> u32 instead?

It does, but the value tops out at 65535 + 6 = 65541, so 17 bits, and
size_t is at least 32 bits. I chose size_t because sizeof(*p) is already
size_t. If you don't like it I can send v2 with u32.

> 
> And if this hasn't shown a problem yet, in the 8 years since that commit
> happened, is this really a bug?
> 
> What changed to suddenly cause this to show up?
> 

Actually nothing changed, I was just looking into
https://syzkaller.appspot.com/bug?extid=baf3cbba7dd980984f0d and ended
up reading ihex_validate_fw. There I noticed the 16 bit return value.
Both cases are reproduceable (infinite worker loop and out-of-bounds read).

> thanks,
> 
> greg k-h

Kind regards,