| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 19 Dec 2012 08:50:42 +0100
From: Christian Riesch <christian.riesch@...cron.at>
To: Lucas Stach <dev@...xeye.de>
CC: netdev@...r.kernel.org, Oliver Neukum <oneukum@...e.de>,
linux-usb@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [PATCH v2 2/2] net: asix: handle packets crossing URB boundaries
Hi Lucas,
On 2012-12-18 16:21, Lucas Stach wrote:
> ASIX AX88772B started to pack data even more tightly. Packets and the ASIX packet
> header may now cross URB boundaries. To handle this we have to introduce
> some state between individual calls to asix_rx_fixup().
cc'ed Eric Dumazet, he did some asix_rx_fixup fixes in spring.
>
> Signed-off-by: Lucas Stach <dev@...xeye.de>
> ---
> I've running this patch for some weeks already now and it gets rid of all
> the commonly seen rx failures with AX88772B.
>
> v2: don't forget to free driver_private
> ---
> drivers/net/usb/asix.h | 11 ++++++
> drivers/net/usb/asix_common.c | 81 +++++++++++++++++++++++++++++-------------
> drivers/net/usb/asix_devices.c | 17 +++++++++
> 3 Dateien geändert, 85 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)
Your patch breaks the driver of the AX88172A in
drivers/net/usb/ax88172a.c. It uses the asix_rx_fixup() function as
well, but you did not add the struct asix_rx_fixup_info to its
driver_priv struct.
Regards, Christian
>
> diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
> index 7afe8ac..4dfdbf6 100644
> --- a/drivers/net/usb/asix.h
> +++ b/drivers/net/usb/asix.h
> @@ -167,6 +167,17 @@ struct asix_data {
> u8 res;
> };
>
> +struct asix_rx_fixup_info {
> + struct sk_buff *ax_skb;
> + u32 header;
> + u16 size;
> + bool split_head;
> +};
> +
> +struct asix_private {
> + struct asix_rx_fixup_info rx_fixup_info;
> +};
> +
> /* ASIX specific flags */
> #define FLAG_EEPROM_MAC (1UL << 0) /* init device MAC from eeprom */
>
> diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
> index 50d1673..17f9801 100644
> --- a/drivers/net/usb/asix_common.c
> +++ b/drivers/net/usb/asix_common.c
> @@ -53,44 +53,77 @@ void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
>
> int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> {
> + struct asix_private *dp = dev->driver_priv;
> + struct asix_rx_fixup_info *rx = &dp->rx_fixup_info;
> int offset = 0;
>
> - while (offset + sizeof(u32) < skb->len) {
> - struct sk_buff *ax_skb;
> - u16 size;
> - u32 header = get_unaligned_le32(skb->data + offset);
> -
> - offset += sizeof(u32);
> -
> - /* get the packet length */
> - size = (u16) (header & 0x7ff);
> - if (size != ((~header >> 16) & 0x07ff)) {
> - netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
> - return 0;
> + while (offset + sizeof(u16) <= skb->len) {
> + u16 remaining = 0;
> + unsigned char *data;
> +
> + if (!rx->size) {
> + if ((skb->len - offset == sizeof(u16)) ||
> + rx->split_head) {
> + if(!rx->split_head) {
> + rx->header = get_unaligned_le16(
> + skb->data + offset);
> + rx->split_head = true;
> + offset += sizeof(u16);
> + break;
> + } else {
> + rx->header |= (get_unaligned_le16(
> + skb->data + offset)
> + << 16);
> + rx->split_head = false;
> + offset += sizeof(u16);
> + }
> + } else {
> + rx->header = get_unaligned_le32(skb->data +
> + offset);
> + offset += sizeof(u32);
> + }
> +
> + /* get the packet length */
> + rx->size = (u16) (rx->header & 0x7ff);
> + if (rx->size != ((~rx->header >> 16) & 0x7ff)) {
> + netdev_err(dev->net, "asix_rx_fixup() Bad Header Length 0x%x, offset %d\n",
> + rx->header, offset);
> + rx->size = 0;
> + return 0;
> + }
> + rx->ax_skb = netdev_alloc_skb_ip_align(dev->net,
> + rx->size);
> + if (!rx->ax_skb)
> + return 0;
> }
>
> - if ((size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) ||
> - (size + offset > skb->len)) {
> + if (rx->size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) {
> netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
> - size);
> + rx->size);
> + kfree_skb(rx->ax_skb);
> return 0;
> }
> - ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
> - if (!ax_skb)
> - return 0;
>
> - skb_put(ax_skb, size);
> - memcpy(ax_skb->data, skb->data + offset, size);
> - usbnet_skb_return(dev, ax_skb);
> + if (rx->size > skb->len - offset) {
> + remaining = rx->size - (skb->len - offset);
> + rx->size = skb->len - offset;
> + }
> +
> + data = skb_put(rx->ax_skb, rx->size);
> + memcpy(data, skb->data + offset, rx->size);
> + if (!remaining)
> + usbnet_skb_return(dev, rx->ax_skb);
>
> - offset += (size + 1) & 0xfffe;
> + offset += (rx->size + 1) & 0xfffe;
> + rx->size = remaining;
> }
>
> if (skb->len != offset) {
> - netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
> - skb->len);
> + netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d, %d\n",
> + skb->len, offset);
> return 0;
> }
> +
> return 1;
> }
>
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index 0ecc3bc..c651243 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -495,9 +495,19 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
> dev->rx_urb_size = 2048;
> }
>
> + dev->driver_priv = kzalloc(sizeof(struct asix_private), GFP_KERNEL);
> + if (!dev->driver_priv)
> + return -ENOMEM;
> +
> return 0;
> }
>
> +void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
> +{
> + if (dev->driver_priv)
> + kfree(dev->driver_priv);
> +}
> +
> static const struct ethtool_ops ax88178_ethtool_ops = {
> .get_drvinfo = asix_get_drvinfo,
> .get_link = asix_get_link,
> @@ -829,6 +839,10 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
> dev->rx_urb_size = 2048;
> }
>
> + dev->driver_priv = kzalloc(sizeof(struct asix_private), GFP_KERNEL);
> + if (!dev->driver_priv)
> + return -ENOMEM;
> +
> return 0;
> }
>
> @@ -875,6 +889,7 @@ static const struct driver_info hawking_uf200_info = {
> static const struct driver_info ax88772_info = {
> .description = "ASIX AX88772 USB 2.0 Ethernet",
> .bind = ax88772_bind,
> + .unbind = ax88772_unbind,
> .status = asix_status,
> .link_reset = ax88772_link_reset,
> .reset = ax88772_reset,
> @@ -886,6 +901,7 @@ static const struct driver_info ax88772_info = {
> static const struct driver_info ax88772b_info = {
> .description = "ASIX AX88772B USB 2.0 Ethernet",
> .bind = ax88772_bind,
> + .unbind = ax88772_unbind,
> .status = asix_status,
> .link_reset = ax88772_link_reset,
> .reset = ax88772_reset,
> @@ -899,6 +915,7 @@ static const struct driver_info ax88772b_info = {
> static const struct driver_info ax88178_info = {
> .description = "ASIX AX88178 USB 2.0 Ethernet",
> .bind = ax88178_bind,
> + .unbind = ax88772_unbind,
> .status = asix_status,
> .link_reset = ax88178_link_reset,
> .reset = ax88178_reset,
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists