[PATCH v2] ACPI: PCI: Clear driver_data on all paths that free the acpi_pci_root

[PATCH v2] ACPI: PCI: Clear driver_data on all paths that free the acpi_pci_root

[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.

acpi_pci_root_remove() has the same problem: it frees root without
clearing device->driver_data, leaving a dangling pointer behind after
the root bridge is removed.

Move the NULL assignment to the shared end: label so every error path in
acpi_pci_root_add() clears driver_data before freeing root, and clear it
in acpi_pci_root_remove() as well, so the object is never left reachable
through driver_data after being freed.

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
Link: https://sashiko.dev/#/patchset/20260707121258.11640-1-cp0613@linux.alibaba.com
Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
Changes in v2:
 - Also clear device->driver_data in acpi_pci_root_remove() before
   kfree(root), so the remove path no longer leaves a dangling pointer
   (Sashiko AI review).

 drivers/acpi/pci_root.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 4c06c3ffd0cb..408ba12362a7 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;
 }
@@ -788,6 +788,7 @@ static void acpi_pci_root_remove(struct acpi_device *device)
 
 	pci_unlock_rescan_remove();
 
+	device->driver_data = NULL;
 	kfree(root);
 }
 
-- 
2.50.1

Re: [PATCH v2] ACPI: PCI: Clear driver_data on all paths that free the acpi_pci_root

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Clearing `device->driver_data = NULL` followed immediately by `kfree(root)` leaves a Use-After-Free race window for concurrent lockless readers.
- [High] Publishing `root` to `device->driver_data` before full initialization and without memory barriers allows readers to see uninitialized fields.
--

--- Patch [1]: [PATCH v2] ACPI: PCI: Clear driver_data on all paths that free the acpi_pci_root ---
commit df0768c14918e7decb436f5ec80561fb6dbd2aed
Author: Chen Pei <cp0613@linux.alibaba.com>

ACPI: PCI: Clear driver_data on all paths that free the acpi_pci_root

This commit fixes dangling pointers in the ACPI PCI root code by ensuring
device->driver_data is cleared to NULL before the root bridge structure
is freed in both error and removal paths.

Link: https://sashiko.dev/#/patchset/20260526025118.38935-1-cp0613@linux.alibaba.com
Link: https://sashiko.dev/#/patchset/20260707121258.11640-1-cp0613@linux.alibaba.com

> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 4c06c3ffd0cbb..408ba12362a79 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;

[Severity: High]
This isn't a bug introduced by this patch, but does the earlier assignment
of device->driver_data = root allow readers to see uninitialized fields?

Earlier in acpi_pci_root_add(), the pointer is published to
device->driver_data before critical fields like root->bus are initialized
later in the function:

acpi_pci_root_add() {
    ...
    device->driver_data = root;
    ...
    root->bus = pci_acpi_scan_root(root);
    ...
}

If a concurrent caller (such as bus_for_each_dev iteration in
cxl_acpi_probe()) fetches the root object, could it observe a NULL root->bus
and dereference it before initialization is complete?

Furthermore, without memory barriers, could the CPU reorder the
initialization such that readers observe partially written state?

>  		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;
>  }

[Severity: High]
This is a pre-existing issue, but does clearing device->driver_data
immediately before kfree(root) leave a use-after-free window for concurrent
lockless readers?

If a lockless reader like hisi_pcie_handle_error() or add_host_bridge_uport()
calls acpi_pci_find_root(), it reads device->driver_data without taking a
lock:

acpi_pci_find_root() {
    ...
    root = acpi_driver_data(device);
    ...
}

If the reader fetches the pointer just before device->driver_data is set
to NULL, and then this code proceeds to kfree(root), wouldn't the concurrent
reader dereference freed memory?

> @@ -788,6 +788,7 @@ static void acpi_pci_root_remove(struct acpi_device *device)
>  
>  	pci_unlock_rescan_remove();
>  
> +	device->driver_data = NULL;
>  	kfree(root);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715135048.3278-1-cp0613@linux.alibaba.com?part=1