Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Merge branch 'asix-rx-mem-handling'

Mark Craske says:

====================
Improve ASIX RX memory allocation error handling

The ASIX RX handler algorithm is weak on error handling.
There is a design flaw in the ASIX RX handler algorithm because the
implementation for handling RX Ethernet frames for the DUB-E100 C1 can
have Ethernet frames spanning multiple URBs. This means that payload data
from more than 1 URB is sometimes needed to fill the socket buffer with a
complete Ethernet frame. When the URB with the start of an Ethernet frame
is received then an attempt is made to allocate a socket buffer. If the
memory allocation fails then the algorithm sets the buffer pointer member
to NULL and the function exits (no crash yet). Subsequently, the RX hander
is called again to process the next URB which assumes there is a socket
buffer available and the kernel crashes when there is no buffer.

This patchset implements an improvement to the RX handling algorithm to
avoid a crash when no memory is available for the socket buffer.

The patchset will apply cleanly to the net-next master branch but the
created kernel has not been tested. The driver was tested on ARM kernels
v3.8 and v3.14 for a commercial product.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>

+75 -43
+1 -1
drivers/net/usb/asix.h
··· 168 168 struct asix_rx_fixup_info { 169 169 struct sk_buff *ax_skb; 170 170 u32 header; 171 - u16 size; 171 + u16 remaining; 172 172 bool split_head; 173 173 }; 174 174
+74 -42
drivers/net/usb/asix_common.c
··· 54 54 struct asix_rx_fixup_info *rx) 55 55 { 56 56 int offset = 0; 57 + u16 size; 58 + 59 + /* When an Ethernet frame spans multiple URB socket buffers, 60 + * do a sanity test for the Data header synchronisation. 61 + * Attempt to detect the situation of the previous socket buffer having 62 + * been truncated or a socket buffer was missing. These situations 63 + * cause a discontinuity in the data stream and therefore need to avoid 64 + * appending bad data to the end of the current netdev socket buffer. 65 + * Also avoid unnecessarily discarding a good current netdev socket 66 + * buffer. 67 + */ 68 + if (rx->remaining && (rx->remaining + sizeof(u32) <= skb->len)) { 69 + offset = ((rx->remaining + 1) & 0xfffe) + sizeof(u32); 70 + rx->header = get_unaligned_le32(skb->data + offset); 71 + offset = 0; 72 + 73 + size = (u16)(rx->header & 0x7ff); 74 + if (size != ((~rx->header >> 16) & 0x7ff)) { 75 + netdev_err(dev->net, "asix_rx_fixup() Data Header synchronisation was lost, remaining %d\n", 76 + rx->remaining); 77 + if (rx->ax_skb) { 78 + kfree_skb(rx->ax_skb); 79 + rx->ax_skb = NULL; 80 + /* Discard the incomplete netdev Ethernet frame 81 + * and assume the Data header is at the start of 82 + * the current URB socket buffer. 83 + */ 84 + } 85 + rx->remaining = 0; 86 + } 87 + } 57 88 58 89 while (offset + sizeof(u16) <= skb->len) { 59 - u16 remaining = 0; 90 + u16 copy_length; 60 91 unsigned char *data; 61 92 62 - if (!rx->size) { 63 - if ((skb->len - offset == sizeof(u16)) || 64 - rx->split_head) { 65 - if(!rx->split_head) { 66 - rx->header = get_unaligned_le16( 67 - skb->data + offset); 68 - rx->split_head = true; 69 - offset += sizeof(u16); 70 - break; 71 - } else { 72 - rx->header |= (get_unaligned_le16( 73 - skb->data + offset) 74 - << 16); 75 - rx->split_head = false; 76 - offset += sizeof(u16); 77 - } 93 + if (!rx->remaining) { 94 + if (skb->len - offset == sizeof(u16)) { 95 + rx->header = get_unaligned_le16( 96 + skb->data + offset); 97 + rx->split_head = true; 98 + offset += sizeof(u16); 99 + break; 100 + } 101 + 102 + if (rx->split_head == true) { 103 + rx->header |= (get_unaligned_le16( 104 + skb->data + offset) << 16); 105 + rx->split_head = false; 106 + offset += sizeof(u16); 78 107 } else { 79 108 rx->header = get_unaligned_le32(skb->data + 80 109 offset); 81 110 offset += sizeof(u32); 82 111 } 83 112 84 - /* get the packet length */ 85 - rx->size = (u16) (rx->header & 0x7ff); 86 - if (rx->size != ((~rx->header >> 16) & 0x7ff)) { 113 + /* take frame length from Data header 32-bit word */ 114 + size = (u16)(rx->header & 0x7ff); 115 + if (size != ((~rx->header >> 16) & 0x7ff)) { 87 116 netdev_err(dev->net, "asix_rx_fixup() Bad Header Length 0x%x, offset %d\n", 88 117 rx->header, offset); 89 - rx->size = 0; 90 118 return 0; 91 119 } 92 - rx->ax_skb = netdev_alloc_skb_ip_align(dev->net, 93 - rx->size); 94 - if (!rx->ax_skb) 120 + if (size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) { 121 + netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n", 122 + size); 95 123 return 0; 124 + } 125 + 126 + /* Sometimes may fail to get a netdev socket buffer but 127 + * continue to process the URB socket buffer so that 128 + * synchronisation of the Ethernet frame Data header 129 + * word is maintained. 130 + */ 131 + rx->ax_skb = netdev_alloc_skb_ip_align(dev->net, size); 132 + 133 + rx->remaining = size; 96 134 } 97 135 98 - if (rx->size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) { 99 - netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n", 100 - rx->size); 101 - kfree_skb(rx->ax_skb); 102 - rx->ax_skb = NULL; 103 - rx->size = 0U; 104 - 105 - return 0; 136 + if (rx->remaining > skb->len - offset) { 137 + copy_length = skb->len - offset; 138 + rx->remaining -= copy_length; 139 + } else { 140 + copy_length = rx->remaining; 141 + rx->remaining = 0; 106 142 } 107 143 108 - if (rx->size > skb->len - offset) { 109 - remaining = rx->size - (skb->len - offset); 110 - rx->size = skb->len - offset; 144 + if (rx->ax_skb) { 145 + data = skb_put(rx->ax_skb, copy_length); 146 + memcpy(data, skb->data + offset, copy_length); 147 + if (!rx->remaining) 148 + usbnet_skb_return(dev, rx->ax_skb); 111 149 } 112 150 113 - data = skb_put(rx->ax_skb, rx->size); 114 - memcpy(data, skb->data + offset, rx->size); 115 - if (!remaining) 116 - usbnet_skb_return(dev, rx->ax_skb); 117 - 118 - offset += (rx->size + 1) & 0xfffe; 119 - rx->size = remaining; 151 + offset += (copy_length + 1) & 0xfffe; 120 152 } 121 153 122 154 if (skb->len != offset) {