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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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?

And root triggers this, right?


> 
> Return size_t so neither the addition nor the following ALIGN() can wrap.
> 
> 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)

but size_t changes depending on the platform, right?  Why not make it
u32 instead?

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?

thanks,

greg k-h