Re: [PATCH] usbnet.c, buf.overrun crash-bugfix, Kernel 2.6.12-rc1

[David Brownell]

On Thursday 24 March 2005 8:05 am, Jakemuksen spammiosote wrote:
> Atleast versions 2.6.5 - 2.6.12-rc1 crash if an USB device using usbnet 
> sends oversized packet. Such packets occur most likely with broken
> device. 

Care to mention what device(s) you saw this with?   And what HCD?


> -       skb_put (skb, urb->actual_length);
> -       entry->state = rx_done;
> -       entry->urb = NULL;
> +       if (unlikely((skb->tail + urb->actual_length) > skb->end)) {

This logic looks wrong.  If that ever happens, surely the problem is
that the rx_submit() code submitted an urb with transfer_size that
mismatched the SKB.  The host controller isn't allowed to overrun the
end of the buffer it's passed.  And if it's tempted to do so, it's
supposed to fill up to the end (skb->end in this case...) and then
report urb->status of -EOVERFLOW.

If you insist on changing this bit of logic, then the best way to
ignore the packet is just to force urb->status to -EOVERFLOW


> +               entry->state = rx_cleanup;
> +               dev->stats.rx_errors++;
> +               dev->stats.rx_length_errors++;
> +               entry->urb = NULL;
> +               printk(KERN_ERR
> +                      "USB RX packet too long, discarded. "
> +                      "Your slave device most likely is broken\n");
> +               /* lets hope upper level protocols will recover */
> +       } else {
> +               skb_put(skb, urb->actual_length);
> +               entry->state = rx_done;
> +               entry->urb = NULL;
> +       }
> 
>          switch (urb_status) {
>              // success
> 
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/