linux-kernel - RE: [PATCH net] hv_netvsc: fix race of netvsc and VF register

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]

Message-ID: <BL1PR21MB3113F0E65A58AEB1E617C2C4CADDA@BL1PR21MB3113.namprd21.prod.outlook.com>
Date:   Thu, 26 Oct 2023 14:52:07 +0000
From:   Haiyang Zhang <haiyangz@...rosoft.com>
To:     Wojciech Drewek <wojciech.drewek@...el.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     KY Srinivasan <kys@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Dexuan Cui <decui@...rosoft.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH net] hv_netvsc: fix race of netvsc and VF
 register_netdevice



> -----Original Message-----
> From: Wojciech Drewek <wojciech.drewek@...el.com>
> Sent: Thursday, October 26, 2023 6:48 AM
> To: Haiyang Zhang <haiyangz@...rosoft.com>; linux-hyperv@...r.kernel.org;
> netdev@...r.kernel.org
> Cc: KY Srinivasan <kys@...rosoft.com>; wei.liu@...nel.org; Dexuan Cui
> <decui@...rosoft.com>; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; davem@...emloft.net; linux-kernel@...r.kernel.org;
> stable@...r.kernel.org
> Subject: Re: [PATCH net] hv_netvsc: fix race of netvsc and VF
> register_netdevice
> 
> [You don't often get email from wojciech.drewek@...el.com. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 25.10.2023 23:16, Haiyang Zhang wrote:
> > The rtnl lock also needs to be held before rndis_filter_device_add()
> > which advertises nvsp_2_vsc_capability / sriov bit, and triggers
> > VF NIC offering and registering. If VF NIC finished register_netdev()
> > earlier it may cause name based config failure.
> >
> > To fix this issue, move the call to rtnl_lock() before
> > rndis_filter_device_add(), so VF will be registered later than netvsc
> > / synthetic NIC, and gets a name numbered (ethX) after netvsc.
> >
> > And, move register_netdevice_notifier() earlier, so the call back
> > function is set before probing.
> >
> > Cc: stable@...r.kernel.org
> > Fixes: e04e7a7bbd4b ("hv_netvsc: Fix a deadlock by getting rtnl lock earlier
> in netvsc_probe()")
> > Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>
> >
> > ---
> >  drivers/net/hyperv/netvsc_drv.c | 30 +++++++++++++++++++-----------
> >  1 file changed, 19 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> > index 3ba3c8fb28a5..feca1391f756 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -2531,15 +2531,6 @@ static int netvsc_probe(struct hv_device *dev,
> >               goto devinfo_failed;
> >       }
> >
> > -     nvdev = rndis_filter_device_add(dev, device_info);
> > -     if (IS_ERR(nvdev)) {
> > -             ret = PTR_ERR(nvdev);
> > -             netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
> > -             goto rndis_failed;
> > -     }
> > -
> > -     eth_hw_addr_set(net, device_info->mac_adr);
> > -
> >       /* We must get rtnl lock before scheduling nvdev->subchan_work,
> >        * otherwise netvsc_subchan_work() can get rtnl lock first and wait
> >        * all subchannels to show up, but that may not happen because
> > @@ -2547,9 +2538,23 @@ static int netvsc_probe(struct hv_device *dev,
> >        * -> ... -> device_add() -> ... -> __device_attach() can't get
> >        * the device lock, so all the subchannels can't be processed --
> >        * finally netvsc_subchan_work() hangs forever.
> > +      *
> > +      * The rtnl lock also needs to be held before rndis_filter_device_add()
> > +      * which advertises nvsp_2_vsc_capability / sriov bit, and triggers
> > +      * VF NIC offering and registering. If VF NIC finished register_netdev()
> > +      * earlier it may cause name based config failure.
> >        */
> >       rtnl_lock();
> >
> > +     nvdev = rndis_filter_device_add(dev, device_info);
> > +     if (IS_ERR(nvdev)) {
> > +             ret = PTR_ERR(nvdev);
> > +             netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
> > +             goto rndis_failed;
> 
> In case of error rtnl won't be unlocked.

Good catch! Will correct this. 

Thanks,
- Haiyang