On Mon, 23 Sep 2024, Wassenberg, Dennis wrote: > Hi together, > > we did some further analysis on this: > > Because we are working on kernel 6.8.12, I will use some logs from this kernel version, just for demonstration. The > initial report was based on 6.11. > > After we tried a KASAN build (dmesg-ramoops-kasan) it looks like it is exactly the same pciehp flow which leads to the > UAF. > Both going through pciehp_ist -> pciehp_disable_slot -> pciehp_unconfigure_device -> pci_remove_bus_device -> ... > This means there are two consecutive interrupts, running on CPU 12 and both will execute the same flow. > At the latest the pci_lock_rescan_remove should be taken in pciehp_unconfigure_device to prevent accessing the pci/bus > structures in parallel. > > I had a look if there are shared data structures accessed in this code path: > For me the access to "*parent = ctrl->pcie->port->subordinate;" looks fishy in pciehp_unconfigure_device. The parent ptr > will be obtained before getting the lock (pci_lock_rescan_remove). Now, if there are two concurrent/consecutive flows > come into this function, both will get the pointer to the parent bridge/subordinate. One thread will enter the lock and > the other one is waiting until the lock is gone. The thread which enters the lock at first will completely remove the > bridge and the subordinate: pciehp_unconfigure_device -> pci_stop_and_remove_bus_device -> pci_remove_bus_device -> > pci_destroy_dev: This will destroy the pci_dev and the subordinate is a part the this structure as well. Now everything > is gone below this pci_bus (childs included). In pci_remove_bus_device there is a loop which iterates over all child > devices and call pci_remove_bus_device again. This means even the child bridges of the current bridge will be deleted. > In the end: everything is gone below the bridge which is regarded here at first. Doesn't that end up removing portdrv/hotplug too so pciehp_remove() does release ctrl? I'm not sure if ctrl can be safely accessed even if the lock is taken first? -- i. > After this the thread leaves the lock with pci_unlock_rescan_remove in pciehp_unconfigure_device. Now the second > thread/ISR will enter the lock. If the second thread belongs to a child bridge of the bridge which was already removed, > it will run into an UAF. This is because the parent bridge destroys all child bridges as well, but the second thread > gets the subordinate pointer before accessing the lock. This means it could be possible hat the second thread uses the > already destroyed subordinate pointer which makes the subordinate invalid. Accessing the pci_bus structure via this > subordinate will definitely run into an UAF. > > In addition we looked closer at pci_stop_and_remove_bus_device_locked() and noticed that while pci_stop_bus_device() is > also walking the ->devices list in reverse order, pci_remove_bus_device() isn't. Maybe it should, to ensure a consistent > order of destruction? > > We addressed both with the following patch: v2-0001-PCI-pcihp-fix-subordinate-access-in-pciehp_unconf.patch > > Whats your thoughts about this? > > > After applying this patch on top of 6.8.12 the initial UAF is gone (the one shown in dmesg-ramoops-kasan), but a > different UAF comes up (dmesg-ramoops-kasan-v2). This new one is more similar to the one which I reported initially on > Kernel 6.11. I think even though the UAF in dmesg-ramoops-kasan is not easy to reproduce on vanilla 6.11, because an > other one will happen, it is a valid fix which should be applied anyway because the code in 6.11 and 6.8.18 doesn't > differ in this area. > I attached a KASAN log as well where both patches are integrated: (v2-0001-PCI-pcihp-fix-subordinate-access-in- > pciehp_unconf.patch + PCI: Don't access freed bus in pci_slot_release() from Ilpo (dmesg-ramoops-kasan-v2+patch_ilpo). > > > In addition I am currently trying to reproduce this on vanilla 6.11 with activated KASAN but I was not lucky enough to > catch this until yet (without KASAN it is easy to reproduce for me). > > Thank you & best regards, > Dennis > > > > On Thu, 2024-09-19 at 10:06 +0200, Dennis Wassenberg wrote: > > Hi together, > > > > we are facing into issues which seems to be PCI related and asking for your estimations. > > > > Background: > > We want to boot up an Intel MeteorLake based system (e.g. Lenovo ThinkPad X13 Gen5) with the Lenovo Thunderbolt 4 > > universal dock attached during boot. On some devices it is nearly 100% reproducible that the boot will fail. Other > > systems will never show this issue (e.g. older devices based on RaptorLake or AlderLake platform). > > > > We did some debugging on this and came to the conclusion that there is a use-after-free in pci_slot_release. > > The Thunderbolt 4 Dock will expose a PCI hierarchy at first and shortly after that, due to the device is inaccessible, > > it will release the additional buses/ports. This seems to end up in a race where pci_slot_release accesses &slot->bus > > which as already freed: > > > > 0000:00 [root bus] > > -> 0000:00:07.0 [bridge to 20-49] > > -> 0000:20:00.0 [bridge to 21-49] > > -> 0000:21:00.0 [bridge to 22] > > 0000:21:01.0 [bridge to 23-2e] > > 0000:21:02.0 [bridge to 2f-3a] > > 0000:21:03.0 [bridge to 3b-48] > > 0000:21:04.0 [bridge to 49] > > 0000:00:07.2 [bridge to 50-79] > > > > > > We are currently running on kernel 6.8.12. Because this kernel is out of support I tried it on 6.11. This kernel shows > > exactly the same issue. I attached two log files: > > dmesg-ramoops-0: Based on kernel 6.11 with added kernel command line option "slab_debug" in order to force a kernel > > Oops > > while accessing freed memory. > > dmesg-ramoops-0-pci_dbg: This it like dmesg-ramoops-0 with additional kernel command line option '"dyndbg=file > > drivers/pci/* +p" ignore_loglevel' in order to give you more insight whats happening on the pci bus. > > > > I would appreciate any kind of help on this. > > > > Thank you & best regards, > > Dennis > > > > > >
