netdev - [PATCH] net: usb: pegasus: Request Ethernet FCS from hardware

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-next>] [day] [month] [year] [list]

Message-Id: <20211226132502.7056-1-ott@mirix.org>
Date:   Sun, 26 Dec 2021 14:25:02 +0100
From:   Matthias-Christian Ott <ott@...ix.org>
To:     Petko Manolov <petkan@...leusys.com>
Cc:     linux-usb@...r.kernel.org, netdev@...r.kernel.org,
        Matthias-Christian Ott <ott@...ix.org>
Subject: [PATCH] net: usb: pegasus: Request Ethernet FCS from hardware

Commit 1a8deec09d12 ("pegasus: fixes reported packet length") tried to
configure the hardware to not include the FCS/CRC of Ethernet frames.
Unfortunately, this does not work with the D-Link DSB-650TX (USB IDs
2001:4002 and 2001:400b): the transferred "packets" (in the terminology
of the hardware) still contain 4 additional octets. For IP packets in
Ethernet this is not a problem as IP packets contain their own lengths
fields but other protocols also see Ethernet frames that include the FCS
in the payload which might be a problem for some protocols.

I was not able to open the D-Link DSB-650TX as the case is a very tight
press fit and opening it would likely destroy it. However, according to
the source code the earlier revision of the D-Link DSB-650TX (USB ID
2001:4002) is a Pegasus (possibly AN986) and not Pegasus II (AN8511)
device. I also tried it with the later revision of the D-Link DSB-650TX
(USB ID 2001:400b) which is a Pegasus II device according to the source
code and had the same results. Therefore, I'm not sure whether the RXCS
(rx_crc_sent) field of the EC0 (Ethernet control_0) register has any
effect or in which revision of the hardware it is usable and has an
effect. As a result, it seems best to me to revert commit
1a8deec09d12 ("pegasus: fixes reported packet length") and to set the
RXCS (rx_crc_sent) field of the EC0 (Ethernet control_0) register so
that the FCS/CRC is always included.

Fixes: 1a8deec09d12 ("pegasus: fixes reported packet length")
Signed-off-by: Matthias-Christian Ott <ott@...ix.org>
---
 drivers/net/usb/pegasus.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index c4cd40b090fd..140d11ae6688 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -422,7 +422,13 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
 	ret = read_mii_word(pegasus, pegasus->phy, MII_LPA, &linkpart);
 	if (ret < 0)
 		goto fail;
-	data[0] = 0xc8; /* TX & RX enable, append status, no CRC */
+	/* At least two hardware revisions of the D-Link DSB-650TX (USB IDs
+	 * 2001:4002 and 2001:400b) include the Ethernet FCS in the packets,
+	 * even if RXCS is set to 0 in the EC0 register and the hardware is
+	 * instructed to not include the Ethernet FCS in the packet.Therefore,
+	 * it seems best to set RXCS to 1 and later ignore the Ethernet FCS.
+	 */
+	data[0] = 0xc9; /* TX & RX enable, append status, CRC */
 	data[1] = 0;
 	if (linkpart & (ADVERTISE_100FULL | ADVERTISE_10FULL))
 		data[1] |= 0x20;	/* set full duplex */
@@ -513,6 +519,13 @@ static void read_bulk_callback(struct urb *urb)
 		pkt_len = buf[count - 3] << 8;
 		pkt_len += buf[count - 4];
 		pkt_len &= 0xfff;
+		/* The FCS at the end of the packet is ignored. So subtract
+		 * its length to ignore it.
+		 */
+		pkt_len -= ETH_FCS_LEN;
+		/* Subtract the length of the received status at the end of the
+		 * packet as it is not part of the Ethernet frame.
+		 */
 		pkt_len -= 4;
 	}

-- 
2.30.2