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

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

[Chen Pei]

acpi_pci_root_add() assigns the freshly allocated root to
device->driver_data before dmar_device_add() and pci_acpi_scan_root().
Both failure paths reach the end: label where root is kfree()'d, but
only the pci_acpi_scan_root() path clears driver_data first.

When dmar_device_add() fails during a hot-add, root is freed while
device->driver_data still points at it.  The ACPI core does not clear
driver_data on attach failure, so a later acpi_pci_find_root() call
may dereference this dangling pointer.

Move the NULL assignment to the shared end: label so every error path
clears driver_data before freeing root.

Fixes: db89b4f0dbab ("ACPI: catch calls of acpi_driver_data on pointer of wrong type")
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260526025118.38935-1-cp0613@linux.alibaba.com
Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
 drivers/acpi/pci_root.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 4c06c3ffd0cb..f6cddd11075c 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -725,7 +725,6 @@ static int acpi_pci_root_add(struct acpi_device *device,
 		dev_err(&device->dev,
 			"Bus %04x:%02x not present in PCI namespace\n",
 			root->segment, (unsigned int)root->secondary.start);
-		device->driver_data = NULL;
 		result = -ENODEV;
 		goto remove_dmar;
 	}
@@ -765,6 +764,7 @@ static int acpi_pci_root_add(struct acpi_device *device,
 	if (hotadd)
 		dmar_device_remove(handle);
 end:
+	device->driver_data = NULL;
 	kfree(root);
 	return result;
 }
-- 
2.50.1

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