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

On Sat, 22 Jan 2022 02:53:42 +0100
Andrew Lunn <andrew@...n.ch> wrote:

> > > 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.

Thank you Andrew! I used this second version of your patch
against 5.16.1 and it seems to work:

    $ sudo ping -f 172.16.0.1

    613 root 20 0 0 0 0 S 11.0 0.0 0:07.46 napi/eth0-385
    614 root 20 0 0 0 0 R  5.3 0.0 0:03.96 napi/eth0-386

Posting used diff as is just in case:

Tested-by: Sergei Trofimovich <slyich@...il.com>

--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -2706,6 +2706,15 @@ static int atl1c_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_alloc_etherdev;
 	}

+	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;
+
 	err = atl1c_init_netdev(netdev, pdev);
 	if (err) {
 		dev_err(&pdev->dev, "init netdevice failed\n");

-- 

  Sergei