netdev - Re: [PATCH net] net: dsa: link interfaces with the DSA master to get rid of lockdep warnings

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: <20200713174227.p6owrtgyccxfbuj5@skbuf>
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