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: <BN3PR03MB22277BEB00D3BFE0C1C06F5ACEFE0@BN3PR03MB2227.namprd03.prod.outlook.com>
Date:   Tue, 13 Sep 2016 17:33:24 +0000
From:   Long Li <longli@...rosoft.com>
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" <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: 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.

> 
> Thanks,
> -- Dexuan