netdev - Re: [PATCH v2] ravb: implement MTU change while device is up

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] [day] [month] [year] [list]

Date:   Fri, 14 Jun 2019 17:28:09 +0200 (CEST)
From:   Ulrich Hecht <uli@...nd.eu>
To:     David Miller <davem@...emloft.net>, uli+renesas@...nd.eu
Cc:     linux-renesas-soc@...r.kernel.org, netdev@...r.kernel.org,
        sergei.shtylyov@...entembedded.com, niklas.soderlund@...natech.se,
        wsa@...-dreams.de, horms@...ge.net.au, magnus.damm@...il.com
Subject: Re: [PATCH v2] ravb: implement MTU change while device is up


> On June 6, 2019 at 4:08 AM David Miller <davem@...emloft.net> wrote:
> 
> 
> From: Ulrich Hecht <uli+renesas@...nd.eu>
> Date: Wed,  5 Jun 2019 17:14:20 +0200
> 
> > @@ -1811,11 +1811,14 @@ static int ravb_do_ioctl(struct net_device *ndev, struct ifreq *req, int cmd)
> >  static int ravb_change_mtu(struct net_device *ndev, int new_mtu)
> >  {
> >  	if (netif_running(ndev))
> > -		return -EBUSY;
> > +		ravb_close(ndev);
> >  
> >  	ndev->mtu = new_mtu;
> >  	netdev_update_features(ndev);
> >  
> > +	if (netif_running(ndev))
> > +		return ravb_open(ndev);
> > +
> 
> And if ravb_open() fails?  The user sees a failure, but to the user the failure
> means the MTU change can't be done, yet the device has the new MTU set still.
> 
> This really is terrible behavior.
> 
> If you must do a prepare/commit kind of sequence for this to work properly if
> you are going to go down the road of taking the device down to change the MTU
> when the device is UP.

So would rolling back the MTU change in case of a re-open failure be acceptable?

If so, is there additional action that needs to be taken if a device unexpectedly goes down as a side-effect of an MTU change?

CU
Uli