Skip to content

Commit 037cf58

Browse files
committed
Merge: CVE-2024-46750: PCI: Add missing bridge lock to pci_bus_lock()
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/5214 JIRA: https://issues.redhat.com/browse/RHEL-59331 CVE: CVE-2024-46750 ``` PCI: Add missing bridge lock to pci_bus_lock() One of the true positives that the cfg_access_lock lockdep effort identified is this sequence: WARNING: CPU: 14 PID: 1 at drivers/pci/pci.c:4886 pci_bridge_secondary_bus_reset+0x5d/0x70 RIP: 0010:pci_bridge_secondary_bus_reset+0x5d/0x70 Call Trace: <TASK> ? __warn+0x8c/0x190 ? pci_bridge_secondary_bus_reset+0x5d/0x70 ? report_bug+0x1f8/0x200 ? handle_bug+0x3c/0x70 ? exc_invalid_op+0x18/0x70 ? asm_exc_invalid_op+0x1a/0x20 ? pci_bridge_secondary_bus_reset+0x5d/0x70 pci_reset_bus+0x1d8/0x270 vmd_probe+0x778/0xa10 pci_device_probe+0x95/0x120 Where pci_reset_bus() users are triggering unlocked secondary bus resets. Ironically pci_bus_reset(), several calls down from pci_reset_bus(), uses pci_bus_lock() before issuing the reset which locks everything *but* the bridge itself. For the same motivation as adding: bridge = pci_upstream_bridge(dev); if (bridge) pci_dev_lock(bridge); to pci_reset_function() for the "bus" and "cxl_bus" reset cases, add pci_dev_lock() for @bus->self to pci_bus_lock(). Link: https://lore.kernel.org/r/171711747501.1628941.15217746952476635316.stgit@dwillia2-xfh.jf.intel.com Reported-by: Imre Deak <imre.deak@intel.com> Closes: http://lore.kernel.org/r/6657833b3b5ae_14984b29437@dwillia2-xfh.jf.intel.com.notmuch Signed-off-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Keith Busch <kbusch@kernel.org> [bhelgaas: squash in recursive locking deadlock fix from Keith Busch: https://lore.kernel.org/r/20240711193650.701834-1-kbusch@meta.com] Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Tested-by: Hans de Goede <hdegoede@redhat.com> Tested-by: Kalle Valo <kvalo@kernel.org> Reviewed-by: Dave Jiang <dave.jiang@intel.com> (cherry picked from commit a4e7728) ``` Signed-off-by: CKI Backport Bot <cki-ci-bot+cki-gitlab-backport-bot@redhat.com> --- <small>Created 2024-09-18 10:00 UTC by backporter - [KWF FAQ](https://red.ht/kernel_workflow_doc) - [Slack #team-kernel-workflow](https://redhat-internal.slack.com/archives/C04LRUPMJQ5) - [Source](https://gitlab.com/cki-project/kernel-workflow/-/blob/main/webhook/utils/backporter.py) - [Documentation](https://gitlab.com/cki-project/kernel-workflow/-/blob/main/docs/README.backporter.md) - [Report an issue](https://gitlab.com/cki-project/kernel-workflow/-/issues/new?issue%5Btitle%5D=backporter%20webhook%20issue)</small> Approved-by: Myron Stowe <mstowe@redhat.com> Approved-by: John W. Linville <linville@redhat.com> Approved-by: CKI KWF Bot <cki-ci-bot+kwf-gitlab-com@redhat.com> Merged-by: Rado Vrbovsky <rvrbovsk@redhat.com>
2 parents b9d7a86 + 699ed49 commit 037cf58

File tree

1 file changed

+21
-14
lines changed

1 file changed

+21
-14
lines changed

drivers/pci/pci.c

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5360,10 +5360,12 @@ static void pci_bus_lock(struct pci_bus *bus)
53605360
{
53615361
struct pci_dev *dev;
53625362

5363+
pci_dev_lock(bus->self);
53635364
list_for_each_entry(dev, &bus->devices, bus_list) {
5364-
pci_dev_lock(dev);
53655365
if (dev->subordinate)
53665366
pci_bus_lock(dev->subordinate);
5367+
else
5368+
pci_dev_lock(dev);
53675369
}
53685370
}
53695371

@@ -5375,33 +5377,37 @@ static void pci_bus_unlock(struct pci_bus *bus)
53755377
list_for_each_entry(dev, &bus->devices, bus_list) {
53765378
if (dev->subordinate)
53775379
pci_bus_unlock(dev->subordinate);
5378-
pci_dev_unlock(dev);
5380+
else
5381+
pci_dev_unlock(dev);
53795382
}
5383+
pci_dev_unlock(bus->self);
53805384
}
53815385

53825386
/* Return 1 on successful lock, 0 on contention */
53835387
static int pci_bus_trylock(struct pci_bus *bus)
53845388
{
53855389
struct pci_dev *dev;
53865390

5391+
if (!pci_dev_trylock(bus->self))
5392+
return 0;
5393+
53875394
list_for_each_entry(dev, &bus->devices, bus_list) {
5388-
if (!pci_dev_trylock(dev))
5389-
goto unlock;
53905395
if (dev->subordinate) {
5391-
if (!pci_bus_trylock(dev->subordinate)) {
5392-
pci_dev_unlock(dev);
5396+
if (!pci_bus_trylock(dev->subordinate))
53935397
goto unlock;
5394-
}
5395-
}
5398+
} else if (!pci_dev_trylock(dev))
5399+
goto unlock;
53965400
}
53975401
return 1;
53985402

53995403
unlock:
54005404
list_for_each_entry_continue_reverse(dev, &bus->devices, bus_list) {
54015405
if (dev->subordinate)
54025406
pci_bus_unlock(dev->subordinate);
5403-
pci_dev_unlock(dev);
5407+
else
5408+
pci_dev_unlock(dev);
54045409
}
5410+
pci_dev_unlock(bus->self);
54055411
return 0;
54065412
}
54075413

@@ -5433,9 +5439,10 @@ static void pci_slot_lock(struct pci_slot *slot)
54335439
list_for_each_entry(dev, &slot->bus->devices, bus_list) {
54345440
if (!dev->slot || dev->slot != slot)
54355441
continue;
5436-
pci_dev_lock(dev);
54375442
if (dev->subordinate)
54385443
pci_bus_lock(dev->subordinate);
5444+
else
5445+
pci_dev_lock(dev);
54395446
}
54405447
}
54415448

@@ -5461,14 +5468,13 @@ static int pci_slot_trylock(struct pci_slot *slot)
54615468
list_for_each_entry(dev, &slot->bus->devices, bus_list) {
54625469
if (!dev->slot || dev->slot != slot)
54635470
continue;
5464-
if (!pci_dev_trylock(dev))
5465-
goto unlock;
54665471
if (dev->subordinate) {
54675472
if (!pci_bus_trylock(dev->subordinate)) {
54685473
pci_dev_unlock(dev);
54695474
goto unlock;
54705475
}
5471-
}
5476+
} else if (!pci_dev_trylock(dev))
5477+
goto unlock;
54725478
}
54735479
return 1;
54745480

@@ -5479,7 +5485,8 @@ static int pci_slot_trylock(struct pci_slot *slot)
54795485
continue;
54805486
if (dev->subordinate)
54815487
pci_bus_unlock(dev->subordinate);
5482-
pci_dev_unlock(dev);
5488+
else
5489+
pci_dev_unlock(dev);
54835490
}
54845491
return 0;
54855492
}

0 commit comments

Comments
 (0)