[PATCH net v3] net: usb: asix: reject a truncated Data header in rx_fixup

[PATCH net v3] net: usb: asix: reject a truncated Data header in rx_fixup

[Aamir Ahmed]

asix_rx_fixup_internal() runs its parsing loop while two bytes remain,
but the branch that starts a new frame reads a four-byte Data header.
Only a two-byte tail is special-cased, via split_head, so a three-byte
tail reaches that read and leaves offset at skb->len + 1. The clamp
below it then takes the unsigned difference skb->len - offset, which
wraps, so copy_length becomes the full length the device asked for:
skb_put_data() copies from one byte past the received data and
usbnet_skb_return() passes the frame to the stack, before the trailing
skb->len != offset check can report it.

Reject a Data header that does not fit and reset the parser state, as
the other malformed-header paths do.

Only a device emitting an odd skb->len can get there, since offset
always advances by an even number of bytes.

Fixes: 8b5b6f5413e9 ("net: asix: handle packets crossing URB boundaries")
Assisted-by: LLM
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---

Notes:
    v3:
      - target net rather than net-next (Andy Shevchenko)
      - trim the last paragraph to the offset argument
      No code change from v2.
    v2: https://lore.kernel.org/netdev/AS8P251MB000193C798F98A11C73C3E9DC8BB2@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM/
    v1: https://lore.kernel.org/netdev/AS8P251MB00014BB4591CE6011152DB4FC8B22@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM/
    
    Built with W=1 (asix_common.o) on x86_64; no warnings. I have no asix
    hardware, so this is not runtime-tested.

 drivers/net/usb/asix_common.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 4f03f4e..8c5f863 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -179,6 +179,13 @@ int asix_rx_fixup_internal(struct usbnet *dev, struct sk_buff *skb,
 				rx->split_head = false;
 				offset += sizeof(u16);
 			} else {
+				if (offset + sizeof(u32) > skb->len) {
+					netdev_err(dev->net, "asix_rx_fixup() Short Data header, offset %d, len %d\n",
+						   offset, skb->len);
+					reset_asix_rx_fixup_info(rx);
+					return 0;
+				}
+
 				rx->header = get_unaligned_le32(skb->data +
 								offset);
 				offset += sizeof(u32);
-- 
2.53.0.windows.1

Re: [PATCH net v3] net: usb: asix: reject a truncated Data header in rx_fixup

[Andy Shevchenko]

On Tue, Sep 15, 2026 at 11:57:24PM +0100, Aamir Ahmed wrote:
> asix_rx_fixup_internal() runs its parsing loop while two bytes remain,
> but the branch that starts a new frame reads a four-byte Data header.
> Only a two-byte tail is special-cased, via split_head, so a three-byte
> tail reaches that read and leaves offset at skb->len + 1. The clamp
> below it then takes the unsigned difference skb->len - offset, which
> wraps, so copy_length becomes the full length the device asked for:
> skb_put_data() copies from one byte past the received data and
> usbnet_skb_return() passes the frame to the stack, before the trailing
> skb->len != offset check can report it.
> 
> Reject a Data header that does not fit and reset the parser state, as
> the other malformed-header paths do.
> 
> Only a device emitting an odd skb->len can get there, since offset
> always advances by an even number of bytes.

...

> int asix_rx_fixup_internal(struct usbnet *dev, struct sk_buff *skb,

> +				if (offset + sizeof(u32) > skb->len) {
> +					netdev_err(dev->net, "asix_rx_fixup() Short Data header, offset %d, len %d\n",
> +						   offset, skb->len);
> +					reset_asix_rx_fixup_info(rx);
> +					return 0;
> +				}

I don't know the rules about __func__ in the error messages in net, but above
may be simplified as

					netdev_err(dev->net, "%s(): Short Data header, offset %d, len %d\n",
						   __func__, offset, skb->len);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net v3] net: usb: asix: reject a truncated Data header in rx_fixup

[Aamir Ahmed]

On Wed, Sep 16, 2026 at 09:50:21AM +0300, Andy Shevchenko wrote:
> I don't know the rules about __func__ in the error messages in net, but above
> may be simplified as
>
>     netdev_err(dev->net, "%s(): Short Data header, offset %d, len %d\n",
>                __func__, offset, skb->len);

I've based the error messages on others I found in this function, here
are some examples:

  netdev_err(dev->net, "asix_rx_fixup() Data Header synchronisation was lost, remaining %d\n", ...);
  netdev_err(dev->net, "asix_rx_fixup() Bad Header Length 0x%x, offset %d\n", ...);
  netdev_dbg(dev->net, "asix_rx_fixup() Bad RX Length %d\n", ...);
  netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d, %d\n", ...);

__func__ here expands to asix_rx_fixup_internal, the internal helper
rather than the asix_rx_fixup() the others name.

Kind Regards
Aamir A.

Re: [PATCH net v3] net: usb: asix: reject a truncated Data header in rx_fixup

[Andy Shevchenko]

On Wed, Sep 16, 2026 at 09:19:13AM +0100, Aamir Ahmed wrote:
> On Wed, Sep 16, 2026 at 09:50:21AM +0300, Andy Shevchenko wrote:

...

> > I don't know the rules about __func__ in the error messages in net, but above
> > may be simplified as
> >
> >     netdev_err(dev->net, "%s(): Short Data header, offset %d, len %d\n",
> >                __func__, offset, skb->len);
> 
> I've based the error messages on others I found in this function, here
> are some examples:
> 
>   netdev_err(dev->net, "asix_rx_fixup() Data Header synchronisation was lost, remaining %d\n", ...);
>   netdev_err(dev->net, "asix_rx_fixup() Bad Header Length 0x%x, offset %d\n", ...);
>   netdev_dbg(dev->net, "asix_rx_fixup() Bad RX Length %d\n", ...);
>   netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d, %d\n", ...);
> 
> __func__ here expands to asix_rx_fixup_internal, the internal helper
> rather than the asix_rx_fixup() the others name.

Yes, and current state of affairs is confusing. If this is _internal() helper,
it should either take a name as a parameter, or use __func__, or ideally drop
it altogether and make error messages to be unique enough that one may easily
guess where the problem came from.

But okay, since it's a common style this discussion and possible amendment is
for another patch.

-- 
With Best Regards,
Andy Shevchenko