netdev - Re: [PATCH net] net: remove MTU limits on a few ether

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:   Fri, 21 Oct 2016 10:44:41 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Jarod Wilson <jarod@...hat.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org,
        Asbjoern Sloth Toennesen <asbjorn@...jorn.st>,
        R Parameswaran <parameswaran.r7@...il.com>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Subject: Re: [PATCH net] net: remove MTU limits on a few ether_setup callers

On Thu, Oct 20, 2016 at 08:42:46PM -0700, Florian Fainelli wrote:
> Le 20/10/2016 à 20:25, Jarod Wilson a écrit :
> > These few drivers call ether_setup(), but have no ndo_change_mtu, and thus
> > were overlooked for changes to MTU range checking behavior. They
> > previously had no range checks, so for feature-parity, set their min_mtu
> > to 0 and max_mtu to ETH_MAX_MTU (65535), instead of the 68 and 1500
> > inherited from the ether_setup() changes. Fine-tuning can come after we get
> > back to full feature-parity here.
> > 
> > CC: netdev@...r.kernel.org
> > Reported-by: Asbjoern Sloth Toennesen <asbjorn@...jorn.st>
> > CC: Asbjoern Sloth Toennesen <asbjorn@...jorn.st>
> > CC: R Parameswaran <parameswaran.r7@...il.com>
> > Signed-off-by: Jarod Wilson <jarod@...hat.com>
> > ---
> 
> > diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> > index 68714a5..d0c7bce 100644
> > --- a/net/dsa/slave.c
> > +++ b/net/dsa/slave.c
> > @@ -1247,6 +1247,8 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
> >  	slave_dev->priv_flags |= IFF_NO_QUEUE;
> >  	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
> >  	slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
> > +	slave_dev->min_mtu = 0;
> > +	slave_dev->max_mtu = ETH_MAX_MTU;
> >  	SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
> 
> Actually for DSA, a reasonable minimum is probably 68 for the following
> reasons: ETH_ZLEN of 60 bytes + FCS (4 bytes) + 2/4/8 bytes of
> Ethernet switch tag (which is dependent on the tagging protocol
> used).

Humm, isn't the switch tag added by the layer below this? So this
should be - 2/4/8 bytes, so that the assembled frame that actually
hits the conduit interface has an MTU of 64.

> Such an Ethernet interface would submit packets through the conduit
> interface which is connected to an Ethernet switches, and those
> typically discard packets smaller than 64 bytes +/- their custom tag
> length.

I have a purely theoretical observation, i.e. i have not checked the
datasheets. You can also control some of the Marvell switches by
sending it ethernet frames containing commands. Most commands are 4
bytes long. So an Ethernet header + 4 bytes is < 64. I expect the
switch will accept command frames which are padded to stop them being
runts. Also, such frames will be submitted to the conduit interface,
not the slave interface.

    Andrew