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: <20221013144853.k6jetben5vidyqkj@skbuf>
Date:   Thu, 13 Oct 2022 17:48:53 +0300
From:   Vladimir Oltean <olteanv@...il.com>
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

On Thu, Oct 13, 2022 at 01:29:00PM +0300, Sergei Antonov wrote:
> On Thu, 13 Oct 2022 at 00:41, David Laight <David.Laight@...lab.com> wrote:
> >
> > From: Sergei Antonov
> > > Sent: 12 October 2022 17:43
> > >
> > > On Wed, 12 Oct 2022 at 19:13, David Laight <David.Laight@...lab.com> wrote:
> > > >
> > > > From: Sergei Antonov
> > > > > Sent: 12 October 2022 16:38
> > > > >
> > > > > Despite the datasheet [1] saying the controller should allow incoming
> > > > > packets of length >=1518, it only allows packets of length <=1514.
> > > >
> > > > Shouldn't that be <=1518 and <1518 ??
> > >
> > > Oh, thanks for noticing. But still it should be slightly different:
> > > <= 1518 and <=1514
> > > Here is my test results of different packet sizes:
> > > packets of 1518 / 1517 / 1516 / 1515 bytes did not come to the driver
> > > (before my patch)
> > > packets of 1514 and less bytes did come
> >
> > I had to double check the frames sizes, not written an ethernet driver
> > for nearly 30 years! There is a nice description that is 90% accurate
> > at https://en.wikipedia.org/wiki/Ethernet_frame
> >
> > 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.

Well, this is the first time you mention of any switch DSA tag.

To my knowledge, what Linux understands by MTU is the maximum size of
the payload (SDU) accepted by the 802.3 MAC or 802.1Q layer. So a MTU of
1500 should allow a frame size, with Ethernet header and FCS, of 1518
octets, or optionally 1522 octets in case it is also VLAN-tagged.

A DSA header, or trailer, is part of the L2 payload, so DSA automatically
tries to increase the MTU on the DSA master to make sure that MTU-sized
frames going through the switch interface don't exceed the MTU of the
master when an extra tag gets inserted.

The only bug in the ftmac100 driver is that it reports netdev->max_mtu
of MAX_PKT_SIZE (1518) instead of 1500. It does *not* support this MTU,
it only supports 1500.

Had it properly reported 1500 as max MTU, you'd have seen that DSA tries
to call dev_set_mtu() on the ftmac100, to account for its tag length.
The driver does not implement ndo_change_mtu, only blindly says that
everything up to 1518 L2 payload length is accepted, and DSA thinks
everything is ok (1500 + 4 = 1504, still less than 1518).