[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()

[sashiko-bot]

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

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

--- Patch [1]: [PATCH] ACPI: PCI: Clear driver_data on all error paths in acpi_pci_root_add() ---
commit 9ff3c7d0724a7259ad865b11b68825024947383a
Author: Chen Pei <cp0613@linux.alibaba.com>

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

This commit addresses a dangling pointer issue in acpi_pci_root_add() by
ensuring device->driver_data is set to NULL on all error paths before
freeing the root object. This prevents use-after-free conditions during a
failed hot-add operation.

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

> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 4c06c3ffd0cbb..f6cddd11075cb 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c

[ ... ]

> @@ -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);

[Severity: High]
This isn't a bug introduced by this patch, but does this lockless cleanup
introduce a use-after-free race with concurrent readers?

If acpi_pci_root_add() publishes the root pointer early:

        device->driver_data = root;

and a concurrent thread (e.g., AER or CXL probe) calls
acpi_pci_find_root() and locklessly reads this pointer:

drivers/acpi/pci_root.c:acpi_pci_find_root() {
        ...
        root = acpi_driver_data(device);
        ...
}

Could that concurrent reader dereference a freed pointer if
acpi_pci_root_add() subsequently encounters an error, clears
device->driver_data, and executes kfree(root) here without an RCU
grace period or locking to synchronize the read and the free?

[Severity: High]
This is also a pre-existing issue, but while we are fixing the error paths
here, does acpi_pci_root_remove() have the same dangling pointer
vulnerability that this patch fixes for the add path?

Looking at acpi_pci_root_remove(), it appears to free the acpi_pci_root
object but fails to clear device->driver_data:

drivers/acpi/pci_root.c:acpi_pci_root_remove() {
        ...
        kfree(root);
        ...
}

If a PCI root bridge is hot-removed and another subsystem later calls
acpi_pci_find_root(), could it locklessly read the dangling pointer
and dereference it?

>  	return result;
>  }

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