Re: [PATCH] ACPI: PCI: Clear driver_data on all error paths in acpi_pci_root_add()

[Chen Pei]

Thanks for the review.

> Pre-existing issues:
> - [High] The acpi_pci_root_remove() function frees the acpi_pci_root
>   object but fails to clear device->driver_data, leaving a dangling
>   pointer.

Agreed. acpi_pci_root_remove() calls kfree(root) without resetting
device->driver_data, which leaves exactly the same dangling pointer
this patch removes from the add() error paths. I'll fix it in v2 by
also clearing device->driver_data in acpi_pci_root_remove(), so the
add and remove paths leave driver_data consistent.

> - [High] Lockless access to `device->driver_data` causes a TOCTOU
>   Use-After-Free race during hot-add failure and hot-remove.

I don't think this is reachable, so I'll leave the locking model
unchanged:

- driver_data is published early (device->driver_data = root), but the
  PCI hierarchy below this root bridge is not created until
  pci_acpi_scan_root() and pci_bus_add_devices(), both of which run
  only on the success path, after the dmar_device_add() error goto.
  During the error window there are no child PCI devices yet, so an AER
  or CXL consumer under this bridge has no way to obtain the handle and
  call acpi_pci_find_root() against a half-initialized root.

- The early publish of driver_data is pre-existing; this patch does not
  change it, it only makes the error-path cleanup consistent.

- ACPI .add and .remove are serialized by acpi_scan_lock, so add cannot
  race remove.

If there is a concrete caller that can reach acpi_pci_find_root() for
this handle during the add-failure window, I'm happy to revisit. Adding
RCU or locking to the find_root() fast path otherwise seems unwarranted
for this fix.

I'll send a v2 with the acpi_pci_root_remove() fix.

Thanks,
Pei