netdev - Re: atl1c drivers run 'napi/eth%d-385' named threads with unsubstituted %d

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: <YetjpvBgQFApTRu0@lunn.ch>
Date:   Sat, 22 Jan 2022 02:53:42 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     Sergei Trofimovich <slyich@...il.com>, netdev@...r.kernel.org
Subject: Re: atl1c drivers run 'napi/eth%d-385' named threads with
 unsubstituted %d

> > So please give this a try. I've not even compile tested it...
> > 
> > iff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > index da595242bc13..983a52f77bda 100644
> > --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > @@ -2706,6 +2706,10 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >                 goto err_alloc_etherdev;
> >         }
> >  
> > +       err = dev_alloc_name(netdev, netdev->name);
> > +       if (err < 0)
> > +               goto err_init_netdev;
> > +
> >         err = atl1c_init_netdev(netdev, pdev);
> >         if (err) {
> >                 dev_err(&pdev->dev, "init netdevice failed\n");
> > 
> > If this works, i can turn it into a real patch submission.
> > 
> >    Andrew
> 
> 
> This may not work right because probe is not called with RTNL.
> And the alloc_name is using RTNL to prevent two devices from
> getting the same name.

Oh, yes. I looked at some of the users. And some do take rtnl before
calling it. And some don't!

Looking at register_netdev(), it seems we need something like:

	if (rtnl_lock_killable()) {
	       err = -EINTR;
	       goto err_init_netdev;
	}
	err = dev_alloc_name(netdev, netdev->name);
	rtnl_unlock();
	if (err < 0)
		goto err_init_netdev;


It might also be a good idea to put a ASSERT_RTNL() in
__dev_alloc_name() to catch any driver doing this wrong.

	   Andrew