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  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:   Mon, 13 Jul 2020 20:42:27 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, f.fainelli@...il.com,
        vivien.didelot@...il.com, xiyou.wangcong@...il.com,
        ap420073@...il.com
Subject: Re: [PATCH net] net: dsa: link interfaces with the DSA master to get
 rid of lockdep warnings

On Mon, Jul 13, 2020 at 08:33:19PM +0300, Vladimir Oltean wrote:
> On Mon, Jul 13, 2020 at 08:30:49PM +0300, Vladimir Oltean wrote:
> > Hi Andrew,
> > 
> > On Mon, Jul 13, 2020 at 06:47:28PM +0200, Andrew Lunn wrote:
> > > > diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> > > > index 743caabeaaa6..a951b2a7d79a 100644
> > > > --- a/net/dsa/slave.c
> > > > +++ b/net/dsa/slave.c
> > > > @@ -1994,6 +1994,13 @@ int dsa_slave_create(struct dsa_port *port)
> > > >  			   ret, slave_dev->name);
> > > >  		goto out_phy;
> > > >  	}
> > > > +	rtnl_lock();
> > > > +	ret = netdev_upper_dev_link(master, slave_dev, NULL);
> > > > +	rtnl_unlock();
> > > > +	if (ret) {
> > > > +		unregister_netdevice(slave_dev);
> > > > +		goto out_phy;
> > > > +	}
> > > 
> > > Hi Vladimir
> > > 
> > > A common pattern we see in bugs is that the driver sets up something
> > > critical after calling register_netdev(), not realising that that call
> > > can go off and really start using the interface before it returns. So
> > > in general, i like to have register_netdev() last, nothing after it.
> > > 
> > > Please could you move this before register_netdev().
> > > 
> > > Thanks
> > > 	Andrew
> > 
> > It doesn't work after register_netdev(). The call to
> 
> I mean it doesn't work when netdev_upper_dev_link() is _before_
> register_netdev().
> 
> > netdev_upper_dev_link() fails and no network interface gets probed. VLAN
> > performs registration and linkage in the same order:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/net/8021q/vlan.c#n175
> > 
> > So I think this part is fine.
> > 
> > Thanks,
> > -Vladimir

One difference from VLAN is that in that case, the entire
register_vlan_device() function runs under RTNL.
When those bugs that you talk about are found, who starts using the
network interface too early? User space or someone else? Would RTNL be
enough to avoid that?

Thanks,
-Vladimir

Powered by blists - more mailing lists