Re: UAF during boot on MTL based devices with attached dock

[Lukas Wunner <lukas@wunner.de>]

On Mon, Oct 07, 2024 at 04:49:19PM +0000, Wassenberg, Dennis wrote:
> > The unplug event happens at the top of the hierarchy (below the Root Port).
> > So pci_bus_add_devices() binds the Root Port, its driver starts stopping
> > and removing the hierarchy below, all the while pci_bus_add_devices()
> > continues binding drivers to the child devices.
> > 
> > Could you try this patch (in addition to the one below and to the one
> > I sent yesterday):
> > 
> > https://lore.kernel.org/all/20241003084342.27501-1-brgl@bgdev.pl/
> > 
> > It should prevent pci_bus_add_devices() from racing with pciehp stopping
> > and removing devices.
> 
> I checked the combination of all 3 patches as well. In the end it behaves
> the same like if I apply the first patch only (the one you sent the day
> before).

Thanks a lot for testing and the detailed feedback.

Would it be possible for you to try the above-linked patch alone
(on top of a recent stock kernel), i.e. without the refcounting
fix that you say was sufficient to avoid the UAF?

And I'd also appreciate if you could try the match_driver approach ...

https://lore.kernel.org/all/Zv-dIHDXNNYomG2Y@wunner.de/

... alone, i.e. without any other patches.

It's interesting that the refcounting fix was sufficient to avoid
the UAF but I can't get over the fact that the pcieport driver is
unbound from pci_remove_bus_device(), when it should no longer be
bound in the first place.  My impression is that teardown of the
hierarchy by pciehp races with driver binding after the initial
root bus scan, so we probably should try to avoid that.  I'd like
to confirm (or disprove) that hunch.

The refcounting fix could be applied as a safety net but normally
shouldn't be necessary if driver unbinding happens in pci_stop_dev()
and the device remains unbound afterwards.  The match_driver patch
should achieve that.  And the other patch by Bartosz (linked above)
should achieve the same by serializing driver binding after bus
enumeration with driver unbinding by pciehp.

Finally, I'd appreciate if you could send me dmesg output with the
refcounting fix applied.  As said before, the MTL Thunderbolt controller
claims that the link and slot presence bits are cleared, so it
de-enumerates everything attached via Thunderbolt.  I'm wondering
if it then re-enumerates the Thunderbolt-attached devices so they're
actually usable?

I'm hoping Mika can clarify with Intel Thunderbolt CoE whether this
is a hardware issue in MTL that can e.g. be fixed through a firmware
or BIOS update.

Thanks!

Lukas