linux-kernel - RE: [PATCH 2/2] pci-hyperv: properly handle device eject

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: <BN3PR03MB222768E7E1D6DDD80560DFEACEFE0@BN3PR03MB2227.namprd03.prod.outlook.com>
Date:   Tue, 13 Sep 2016 17:41:00 +0000
From:   Long Li <longli@...rosoft.com>
To:     Long Li <longli@...rosoft.com>, Dexuan Cui <decui@...rosoft.com>,
        "KY Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>
CC:     "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
Subject: RE: [PATCH 2/2] pci-hyperv: properly handle device eject



> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@...uxdriverproject.org] On
> Behalf Of Long Li
> Sent: Tuesday, September 13, 2016 10:33 AM
> To: Dexuan Cui <decui@...rosoft.com>; KY Srinivasan
> <kys@...rosoft.com>; Haiyang Zhang <haiyangz@...rosoft.com>; Bjorn
> Helgaas <bhelgaas@...gle.com>
> Cc: devel@...uxdriverproject.org; linux-kernel@...r.kernel.org; linux-
> pci@...r.kernel.org
> Subject: RE: [PATCH 2/2] pci-hyperv: properly handle device eject
> 
> This sender failed our fraud detection checks and may not be who they
> appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing
> 
> > -----Original Message-----
> > From: Dexuan Cui
> > Sent: Tuesday, September 13, 2016 2:51 AM
> > To: Long Li <longli@...rosoft.com>; KY Srinivasan <kys@...rosoft.com>;
> > Haiyang Zhang <haiyangz@...rosoft.com>; Bjorn Helgaas
> > <bhelgaas@...gle.com>
> > Cc: devel@...uxdriverproject.org; linux-kernel@...r.kernel.org; linux-
> > pci@...r.kernel.org
> > Subject: RE: [PATCH 2/2] pci-hyperv: properly handle device eject
> >
> > > From: devel [mailto:driverdev-devel-bounces@...uxdriverproject.org]
> > > On Behalf Of Long Li
> > > Sent: Tuesday, September 13, 2016 7:54 ...
> > > A PCI_EJECT message can arrive at the same time we are calling
> > > pci_scan_child_bus in the workqueue for the previous
> > PCI_BUS_RELATIONS
> > > message, in this case we could potentailly modify the bus from two
> places.
> > > Properly lock the bus access.
> > >
> > > --- a/drivers/pci/host/pci-hyperv.c
> > > +++ b/drivers/pci/host/pci-hyperv.c
> > > @@ -1587,7 +1587,7 @@ static void hv_eject_device_work(struct
> > > work_struct
> > > *work)
> > >         pdev =
> > > pci_get_domain_bus_and_slot(hpdev->hbus->sysdata.domain,
> > 0,
> > >                                            wslot);
> > >         if (pdev) {
> > > -               pci_stop_and_remove_bus_device(pdev);
> > > +               pci_stop_and_remove_bus_device_locked(pdev);
> > >                 pci_dev_put(pdev);
> > >         }
> >
> > The _locked version tries to get the mutex pci_rescan_remove_lock.
> >
> > But it looks pci_scan_child_bus() doesn't try to get the mutex(?), so
> > how can this patch make sure the 2 code paths are not running
> simultaneously?
> 
> Thanks for the review.
> 
> The lock is to protect the following call to pci_scan_child_bus() in
> pci_devices_present_work():
> 
>                 /*
>                  * Tell the core to rescan bus
>                  * because there may have been changes.
>                  */
>                 pci_lock_rescan_remove();
>                 pci_scan_child_bus(hbus->pci_bus);
>                 pci_unlock_rescan_remove();
> 
> This race condition has shown up in the tests.
> 
> You raised a valid concern in create_root_hv_pci_bus(). There might be
> another race condition there. I'll look into this.

I think this code is safe here. If we reach the code pci_stop_and_remove_bus_device_locked, create_root_hv_pci_bus() is already called.

> 
> >
> > Thanks,
> > -- Dexuan
> _______________________________________________
> devel mailing list
> devel@...uxdriverproject.org
> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fdriverde
> v.linuxdriverproject.org%2fmailman%2flistinfo%2fdriverdev-
> devel&data=02%7c01%7clongli%40microsoft.com%7c3d12ee6d87c140eb5114
> 08d3dbfc1713%7c72f988bf86f141af91ab2d7cd011db47%7c1%7c0%7c6360938
> 48185348266&sdata=a2GYqIBsQAFxszkKg3fl1nqqPgvZHh%2bAY2255RgrvUU
> %3d