netdev - Re: [PATCH v2 net] net: ftmac100: do not reject packets bigger than 1514

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: <Y0gcblXFum4GsSve@lunn.ch>
Date:   Thu, 13 Oct 2022 16:10:54 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Sergei Antonov <saproj@...il.com>
Cc:     David Laight <David.Laight@...lab.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>
Subject: Re: [PATCH v2 net] net: ftmac100: do not reject packets bigger than
 1514

> > Without an 802.1Q tag (probably a VLAN tag?) the max frame has
> > 1514 data bytes (inc mac addresses, but excl crc).
> > Unless you are using VLANs that should be the frame limit.
> > The IP+TCP is limited to the 1500 byte payload.
> 
> Exactly! Incoming packets first go through a switch chip (Marvell
> 88E6060), so packets should get tagged.
> 
> > So if the sender is generating longer packets it is buggy!
> 
> Looking into it.
> 
> On the sender computer:
> sudo ifconfig eno1 mtu 1500 up
> ssh receiver_computer
> 
> On the receiver computer:
> in ftmac100_rx_packet_error() I call
> ftmac100_rxdes_frame_length(rxdes) and it returns 1518. I suppose, it
> is 1500 + 18(ethernet overhead) + 4(switch tag) - 4(crc).
> 
> Would you like me to dump the entire packet and verify?

You did not mention DSA before. That is an important fact.

What should happen is that the DSA framework should take the DSA frame
header into account. It should be calling into the MAC driver and
asking it to change its MTU to allow for the additional 4 bytes of the
switch tag.

But there is some history here. For a long time, it was just assumed
the MAC driver would accept any length of packet, i.e. the MRU,
Maximum Receive Unit, was big enough for DSA to just work. A Marvell
switch is normally combined with a Marvell MAC, and this was
true. This worked for a long time, until it did not work for some
combination of switch and MAC. It then became necessary to change the
MTU on the master interface, so it would actually receive the bigger
frames. But we had the backwards compatibility problem, that some MAC
drivers which work with DSA because there MRU is sufficient, don't
support changing the MTU. So we could not make it fatal if the change
of the MTU failed.

Does this driver support the MTU change op? If not, you should try
implementing it. If the hardware cannot actually receive longer
packets, you need to take the opposite approach. You need to make the
MTU on the slave interfaces smaller, so that packets from the switch
to the master interface are small enough to be correctly received when
including the switch header.

	  Andrew