netdev - Re: [RFT] sky2: fix status length check on older chips

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120427135514.48b03ce8@nehalam.linuxnetplumber.net>
Date:	Fri, 27 Apr 2012 13:55:14 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	Stephen Hemminger <shemminger@...tta.com>
Cc:	Niccolò Belli <darkbasic@...uxsystems.it>,
	netdev@...r.kernel.org
Subject: Re: [RFT] sky2: fix status length check on older chips

Mirko and I were working on a related issue, here is his fix.
I proposed a similar alternative, but he was unavailable to test it,
and therefore was still pending.  Does this fix your problem?

Resend with missing bits.

The cleaner alternative is to not use the boolean flag, but instead
put tag on skb directly (eliminating rx_tag and rx_vlan from
data structure).
=================

From: From: Mirko Lindner <mlindner@...vell.com>

Bug: The VLAN bit of the MAC RX Status Word is unreliable in several older
supported chips. Sometimes the VLAN bit is not set for valid VLAN packets
and also sometimes the VLAN bit is set for non-VLAN packets that came after
a VLAN packet. This results in a receive length error when VLAN hardware
tagging is enabled.

Fix: The driver uses only VLAN information included in VLAN status list elements,
that signals that the VLAN tag field is valid. It must ignore the VLAN bit in the
MAC RX Status Word. An additional variable set when evaluating the VLAN opcodes
is used to indicate that the received packet is a VLAN packet and a packet length
correction (subtraction of VLAN header length) must be done.

Signed-off-by: Mirko Lindner <mlindner@...vell.com>
Signed-off-by: Stephen Hemminger <shemminger@...tta.com>

---
--- a/drivers/net/ethernet/marvell/sky2.c	2012-04-18 23:44:40.637276647 -0700
+++ b/drivers/net/ethernet/marvell/sky2.c	2012-04-27 13:52:06.154756866 -0700
@@ -2580,7 +2580,7 @@ static struct sk_buff *sky2_receive(stru
 	struct sk_buff *skb = NULL;
 	u16 count = (status & GMR_FS_LEN) >> 16;

-	if (status & GMR_FS_VLAN)
+	if (sky2->rx_vlan)
 		count -= VLAN_HLEN;	/* Account for vlan tag */

 	netif_printk(sky2, rx_status, KERN_DEBUG, dev,
@@ -2646,11 +2646,13 @@ static inline void sky2_tx_done(struct n
 	}
 }

-static inline void sky2_skb_rx(const struct sky2_port *sky2,
-			       u32 status, struct sk_buff *skb)
+static inline void sky2_skb_rx(struct sky2_port *sky2,
+				struct sk_buff *skb)
 {
-	if (status & GMR_FS_VLAN)
+	if (sky2->rx_vlan) {
 		__vlan_hwaccel_put_tag(skb, be16_to_cpu(sky2->rx_tag));
+		sky2->rx_vlan = 0;
+	}

 	if (skb->ip_summed == CHECKSUM_NONE)
 		netif_receive_skb(skb);
@@ -2772,10 +2774,12 @@ static int sky2_status_intr(struct sky2_
 			break;

 		case OP_RXVLAN:
+			sky2->rx_vlan = 1;
 			sky2->rx_tag = length;
 			break;

 		case OP_RXCHKSVLAN:
+			sky2->rx_vlan = 1;
 			sky2->rx_tag = length;
 			/* fall through */
 		case OP_RXCHKS:
--- a/drivers/net/ethernet/marvell/sky2.h	2012-02-13 09:23:53.642447236 -0800
+++ b/drivers/net/ethernet/marvell/sky2.h	2012-04-27 13:36:07.487834432 -0700
@@ -2242,6 +2242,7 @@ struct sky2_port {
 	u16		     rx_data_size;
 	u16		     rx_nfrags;
 	u16		     rx_tag;
+	u8		     rx_vlan;

 	struct {
 		unsigned long last;
--
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