> -----Original Message-----
> From: Eric Dumazet [mailto:eric.duma...@gmail.com]
> Sent: 2017年9月21日 23:02
> To: Zhang Shengju <zhangshen...@cmss.chinamobile.com>
> Cc: da...@davemloft.net; will...@google.com;
> step...@networkplumber.org; netdev@vger.kernel.org
> Subject: Re: [net-next 1/2] dummy: add device MTU validation check
>
> On Thu, 2017-09-21 at 21:32 +0800, Zhang Shengju wrote:
> > Currently, any mtu value can be assigned when adding a new dummy device:
> > [~]# ip link add name dummy1 mtu 100000 type dummy [~]# ip link show
> > dummy1
> > 15: dummy1: <BROADCAST,NOARP> mtu 100000 qdisc noop state DOWN
> mode DEFAULT group default qlen 1000
> > link/ether 0a:61:6b:16:14:ce brd ff:ff:ff:ff:ff:ff
> >
> > This patch adds device MTU validation check.
>
> What is wrong with big MTU on dummy ?
>
> If this is a generic rule, this check should belong in core network stack.
>
dummy_setup() function setup mtu range: [0, ETH_MAX_MTU].
This will be checked at dev_set_mtu() function in core network stack.
So if you add a new dummy device without specify mtu value, you can't set a
value out of range [0, ETH_MAX_MTU] afterward.
BUT you can set any mtu when adding new device. This cause an inconsistence.
> >
> > Signed-off-by: Zhang Shengju <zhangshen...@cmss.chinamobile.com>
> > ---
> > drivers/net/dummy.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c index
> > e31ab3b..0276b2b 100644
> > --- a/drivers/net/dummy.c
> > +++ b/drivers/net/dummy.c
> > @@ -365,6 +365,14 @@ static int dummy_validate(struct nlattr *tb[], struct
> nlattr *data[],
> > if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
> > return -EADDRNOTAVAIL;
> > }
> > +
> > + if (tb[IFLA_MTU]) {
> > + u32 mtu = nla_get_u32(tb[IFLA_MTU]);
>
> You do not verify/validate nla_len(tb[IFLA_MTU]).
>
> Do not ever trust user space.
MTU attribute is just u32, do you think it's necessary to check the length?
Actually I don't see any place to check the length of mtu attribute in network
stack code.
>
> > +
> > + if (mtu > ETH_MAX_MTU)
> > + return -EINVAL;
> > + }
> > +
> > return 0;
> > }
> >
>
