netdev - Re: [PATCH net v2] net: usb: pegasus: Do not drop long Ethernet frames

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]

Date:   Mon, 27 Dec 2021 13:11:41 +0200
From:   Petko Manolov <petkan@...leusys.com>
To:     Matthias-Christian Ott <ott@...ix.org>
Cc:     linux-usb@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net v2] net: usb: pegasus: Do not drop long Ethernet
 frames

On 21-12-26 23:12:08, Matthias-Christian Ott wrote:
> The D-Link DSB-650TX (2001:4002) is unable to receive Ethernet frames that are
> longer than 1518 octets, for example, Ethernet frames that contain 802.1Q VLAN
> tags.
> 
> The frames are sent to the pegasus driver via USB but the driver discards them
> because they have the Long_pkt field set to 1 in the received status report.
> The function read_bulk_callback of the pegasus driver treats such received
> "packets" (in the terminology of the hardware) as errors but the field simply
> does just indicate that the Ethernet frame (MAC destination to FCS) is longer
> than 1518 octets.
> 
> It seems that in the 1990s there was a distinction between "giant" (> 1518)
> and "runt" (< 64) frames and the hardware includes flags to indicate this
> distinction. It seems that the purpose of the distinction "giant" frames was
> to not allow infinitely long frames due to transmission errors and to allow
> hardware to have an upper limit of the frame size. However, the hardware
> already has such limit with its 2048 octet receive buffer and, therefore,
> Long_pkt is merely a convention and should not be treated as a receive error.
> 
> Actually, the hardware is even able to receive Ethernet frames with 2048
> octets which exceeds the claimed limit frame size limit of the driver of 1536
> octets (PEGASUS_MTU).

2048 is not mentioned anywhere in both, adm8511 and adm8515 documents.  In the
latter I found 1638 as max packet length, but that's not the default.  The
default is 1528 and i don't feel like changing it without further investigation.

Thus, i assume it is safe to change PEGASUS_MTU to 1528 for the moment.  VLAN
frames have 4 additional bytes so we aren't breaking neither pegasus I nor
pegasus II devices.

If you're going to make ver 3 of this change, you might as well modify
PEGASUS_MTU in pegasus.h as a separate patch within the same series.


		Petko


> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Matthias-Christian Ott <ott@...ix.org>
> ---
> V1 -> V2: Included "Fixes:" tag
> 
>  drivers/net/usb/pegasus.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index 140d11ae6688..2582daf23015 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -499,11 +499,11 @@ static void read_bulk_callback(struct urb *urb)
>  		goto goon;
>  
>  	rx_status = buf[count - 2];
> -	if (rx_status & 0x1e) {
> +	if (rx_status & 0x1c) {
>  		netif_dbg(pegasus, rx_err, net,
>  			  "RX packet error %x\n", rx_status);
>  		net->stats.rx_errors++;
> -		if (rx_status & 0x06)	/* long or runt	*/
> +		if (rx_status & 0x04)	/* runt	*/
>  			net->stats.rx_length_errors++;
>  		if (rx_status & 0x08)
>  			net->stats.rx_crc_errors++;
> -- 
> 2.30.2
> 
>