[PATCH v2] net: usb: ax88179_178a: ax88179_rx_fixup corrections

[Jose Alonso <joalonsof@gmail.com>]

This patch corrects the receiving of packets in ax88179_rx_fixup.

corrections:
- the size check of the bounds of the metadata array.
- the handling of the metadata array.
   The current code is allways exiting with return 0
   while trying to access pkt_hdr out of metadata array and
   generating RX Errors.
- avoid changing the skb->data content (reverse bytes) in case
   of big-endian. le32_to_cpus(pkt_hdr) 

Tested with: 0b95:1790 ASIX Electronics Corp. AX88179 Gigabit Ethernet

Signed-off-by: Jose Alonso <joalonsof@gmail.com>

---

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 4704ed6f00ef..d2f868dd3c10 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1445,18 +1445,18 @@ static void ax88179_unbind(struct usbnet *dev, struct usb_interface *intf)
 }
 
 static void
-ax88179_rx_checksum(struct sk_buff *skb, u32 *pkt_hdr)
+ax88179_rx_checksum(struct sk_buff *skb, u32 pkt_hdr_val)
 {
 	skb->ip_summed = CHECKSUM_NONE;
 
 	/* checksum error bit is set */
-	if ((*pkt_hdr & AX_RXHDR_L3CSUM_ERR) ||
-	    (*pkt_hdr & AX_RXHDR_L4CSUM_ERR))
+	if ((pkt_hdr_val & AX_RXHDR_L3CSUM_ERR) ||
+	    (pkt_hdr_val & AX_RXHDR_L4CSUM_ERR))
 		return;
 
 	/* It must be a TCP or UDP packet with a valid checksum */
-	if (((*pkt_hdr & AX_RXHDR_L4_TYPE_MASK) == AX_RXHDR_L4_TYPE_TCP) ||
-	    ((*pkt_hdr & AX_RXHDR_L4_TYPE_MASK) == AX_RXHDR_L4_TYPE_UDP))
+	if (((pkt_hdr_val & AX_RXHDR_L4_TYPE_MASK) == AX_RXHDR_L4_TYPE_TCP) ||
+	    ((pkt_hdr_val & AX_RXHDR_L4_TYPE_MASK) == AX_RXHDR_L4_TYPE_UDP))
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 }
 
@@ -1467,11 +1467,47 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	u32 rx_hdr;
 	u16 hdr_off;
 	u32 *pkt_hdr;
+	u32 pkt_hdr_val;
 
 	/* At the end of the SKB, there's a header telling us how many packets
 	 * are bundled into this buffer and where we can find an array of
 	 * per-packet metadata (which contains elements encoded into u16).
 	 */
+
+	/* SKB contents for current firmware:
+	 *   <packet 1> <padding>
+	 *   ...
+	 *   <packet N> <padding>
+	 *   <per-packet metadata entry 1> <dummy header>
+	 *   ...
+	 *   <per-packet metadata entry N> <dummy header>
+	 *   <padding2> <rx_hdr>
+	 *
+	 * where:
+	 *   <packet N> contains pkt_len bytes:
+	 *		2 bytes of IP alignment pseudo header
+	 *		packet received
+	 *   <per-packet metadata entry N> contains 4 bytes:
+	 *		pkt_len and fields AX_RXHDR_*
+	 *   <padding>	0-7 bytes to terminate at
+	 *		8 bytes boundary (64-bit).
+	 *   <padding2> 4 bytes
+	 *   <dummy-header> contains 4 bytes:
+	 *		pkt_len=0 & AX_RXHDR_DROP_ERR
+	 *   <rx-hdr>	contains 4 bytes:
+	 *		pkt_cnt and hdr_off (offset of 
+	 *		  <per-packet metadata entry 1>)
+	 *
+	 * pkt_cnt is number of entrys in the per-packet metadata.
+	 * In current firmware there is 2 entrys per packet.
+	 * The first points to the packet and the
+	 *  second is a dummy header.
+	 * This was done probably to align fields in 64-bit and
+	 *  maintain compatibility with old firmware.
+	 * This code assumes that <dummy header> and <padding2> are
+	 *  optional.
+	 */
+
 	if (skb->len < 4)
 		return 0;
 	skb_trim(skb, skb->len - 4);
@@ -1485,51 +1521,63 @@ static int ax88179_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	/* Make sure that the bounds of the metadata array are inside the SKB
 	 * (and in front of the counter at the end).
 	 */
-	if (pkt_cnt * 2 + hdr_off > skb->len)
+	if (pkt_cnt * 4 + hdr_off > skb->len)
 		return 0;
 	pkt_hdr = (u32 *)(skb->data + hdr_off);
 
 	/* Packets must not overlap the metadata array */
 	skb_trim(skb, hdr_off);
 
-	for (; ; pkt_cnt--, pkt_hdr++) {
+	for (; pkt_cnt > 0; pkt_cnt--, pkt_hdr++) {
 		u16 pkt_len;
+		u16 pkt_len_buf;
 
-		le32_to_cpus(pkt_hdr);
-		pkt_len = (*pkt_hdr >> 16) & 0x1fff;
+		pkt_hdr_val = get_unaligned_le32(pkt_hdr);
+		pkt_len = (pkt_hdr_val >> 16) & 0x1fff;
+		pkt_len_buf = (pkt_len + 7) & 0xfff8;
 
-		if (pkt_len > skb->len)
+		/* Skip dummy header used for alignment
+		 */
+		if (pkt_len == 0)
+			continue;
+
+		if (pkt_len_buf > skb->len)
 			return 0;
 
 		/* Check CRC or runt packet */
-		if (((*pkt_hdr & (AX_RXHDR_CRC_ERR | AX_RXHDR_DROP_ERR)) == 0) &&
-		    pkt_len >= 2 + ETH_HLEN) {
-			bool last = (pkt_cnt == 0);
-
-			if (last) {
-				ax_skb = skb;
-			} else {
-				ax_skb = skb_clone(skb, GFP_ATOMIC);
-				if (!ax_skb)
-					return 0;
-			}
-			ax_skb->len = pkt_len;
-			/* Skip IP alignment pseudo header */
-			skb_pull(ax_skb, 2);
-			skb_set_tail_pointer(ax_skb, ax_skb->len);
-			ax_skb->truesize = pkt_len + sizeof(struct sk_buff);
-			ax88179_rx_checksum(ax_skb, pkt_hdr);
+		if ((pkt_hdr_val & (AX_RXHDR_CRC_ERR | AX_RXHDR_DROP_ERR)) ||
+		    pkt_len < 2 + ETH_HLEN) {
+			dev->net->stats.rx_errors++;
+			skb_pull(skb, pkt_len_buf);
+			continue;
+		}
 
-			if (last)
-				return 1;
+		/* last packet */
+		if (pkt_len_buf == skb->len) {
+			skb_trim(skb, pkt_len);
 
-			usbnet_skb_return(dev, ax_skb);
+			/* Skip IP alignment pseudo header */
+			skb_pull(skb, 2);
+
+			ax88179_rx_checksum(skb, pkt_hdr_val);
+			return 1;
 		}
 
-		/* Trim this packet away from the SKB */
-		if (!skb_pull(skb, (pkt_len + 7) & 0xFFF8))
+		ax_skb = skb_clone(skb, GFP_ATOMIC);
+		if (!ax_skb)
 			return 0;
+		skb_trim(ax_skb, pkt_len);
+
+		/* Skip IP alignment pseudo header */
+		skb_pull(ax_skb, 2);
+
+		ax88179_rx_checksum(ax_skb, pkt_hdr_val);
+		usbnet_skb_return(dev, ax_skb);
+
+		skb_pull(skb, pkt_len_buf);
 	}
+
+	return 0;
 }
 
 static struct sk_buff *

--