[PATCH v1 0/3] ACPI: bus: Rework acquiring the primary physical for a given ACPI one

[PATCH v1 0/3] ACPI: bus: Rework acquiring the primary physical for a given ACPI one

[Rafael J. Wysocki]

Hi All,

This is somewhat late, but since there will be follow-up changes tree-wide,
it would be good to get it into the mainline sooner than later.

It is based on the observation that the majority of
acpi_get_first_physical_node() callers want to hold a reference on the returned
device and the function is potentially racy because it doesn't get such a
reference itself.  There were also some comments from Sashiko pointing
out to this as a "preexisting issue".

The first patch adds a acpi_get_first_physical_node() replacement called
acpi_bus_get_primary_device() and returning a reference-counted device (the
former is retained for the time being, but made use the same code as the
latter).

The next two patches change the core ACPI code to use the new function
instead of the old one.

Later on, patches will be sent to switch all of the callers of
acpi_get_first_physical_node() over to using acpi_bus_get_primary_device()
and finally to drop the former.

Thanks!

[PATCH v1 1/3] ACPI: bus: Introduce acpi_bus_get_primary_device()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The function used for obtaining the first "physical" device for which
the given ACPI one is the ACPI companion, acpi_get_first_physical_node(),
may return a stale device pointer (mostly in theory) because
acpi_unbind_one() may run as a whole after dropping the ACPI device's
physical_node_lock in acpi_get_first_physical_node() and before it
returns.  The last reference to the "physical" device may be dropped
then before the pointer to it is returned to the caller.

If that happens and the acpi_get_first_physical_node() caller invokes
get_device() on the pointer obtained from it, which is done by the
majority of its callers, a use-after-free will occur.

To prepare for addressing this problem, introduce a new function for
getting the first "physical" device associated with the given ACPI one
(the "primary physical device") that will also reference count the
device in question before returning a pointer to it.

Make that new function and acpi_get_first_physical_node() share the
physical node list lookup code.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/bus.c      |   51 ++++++++++++++++++++++++++++++++----------------
 include/acpi/acpi_bus.h |    6 +++++
 2 files changed, 41 insertions(+), 16 deletions(-)

--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -774,30 +774,49 @@ static int __init acpi_setup_sb_notify_h
                              Device Matching
    -------------------------------------------------------------------------- */
 
+
+static struct device *primary_physical_device(struct acpi_device *adev)
+{
+	if (list_empty(&adev->physical_node_list))
+		return NULL;
+
+	return list_first_entry(&adev->physical_node_list,
+				struct acpi_device_physical_node, node)->dev;
+}
+
+/**
+ * acpi_bus_get_primary_device - Get first physical device for a given ACPI one
+ * @adev: ACPI device to get the first physical device for.
+ *
+ * Find the first physical device for which @adev is the ACPI companion and
+ * reference count it if present.
+ *
+ * Return: Pointer to the first physical counterpart of @adev or NULL if there
+ * are none.  Callers are responsible for invoking put_device() on the returned
+ * device.
+ */
+struct device *acpi_bus_get_primary_device(struct acpi_device *adev)
+{
+	if (!adev)
+		return NULL;
+
+	guard(mutex)(&adev->physical_node_lock);
+
+	return get_device(primary_physical_device(adev));
+}
+EXPORT_SYMBOL_GPL(acpi_bus_get_primary_device);
+
 /**
- * acpi_get_first_physical_node - Get first physical node of an ACPI device
+ * acpi_get_first_physical_node - Find first physical node of an ACPI device
  * @adev:	ACPI device in question
  *
  * Return: First physical node of ACPI device @adev
  */
 struct device *acpi_get_first_physical_node(struct acpi_device *adev)
 {
-	struct mutex *physical_node_lock = &adev->physical_node_lock;
-	struct device *phys_dev;
+	guard(mutex)(&adev->physical_node_lock);
 
-	mutex_lock(physical_node_lock);
-	if (list_empty(&adev->physical_node_list)) {
-		phys_dev = NULL;
-	} else {
-		const struct acpi_device_physical_node *node;
-
-		node = list_first_entry(&adev->physical_node_list,
-					struct acpi_device_physical_node, node);
-
-		phys_dev = node->dev;
-	}
-	mutex_unlock(physical_node_lock);
-	return phys_dev;
+	return primary_physical_device(adev);
 }
 EXPORT_SYMBOL_GPL(acpi_get_first_physical_node);
 
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -645,6 +645,7 @@ int acpi_scan_add_handler(struct acpi_sc
 int acpi_bus_scan(acpi_handle handle);
 void acpi_bus_trim(struct acpi_device *start);
 acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
+struct device *acpi_bus_get_primary_device(struct acpi_device *adev);
 int acpi_match_device_ids(struct acpi_device *device,
 			  const struct acpi_device_id *ids);
 void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
@@ -947,6 +948,11 @@ int acpi_scan_add_dep(acpi_handle handle
 u32 arch_acpi_add_auto_dep(acpi_handle handle);
 #else	/* CONFIG_ACPI */
 
+static inline struct device *acpi_bus_get_primary_device(struct acpi_device *adev)
+{
+	return NULL;
+}
+
 static inline bool acpi_of_match_device(const struct acpi_device *adev,
 					const struct of_device_id *of_match_table,
 					const struct of_device_id **of_id)

[PATCH v1 2/3] ACPI: platform: Use acpi_bus_get_primary_device()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The acpi_get_first_physical_node() usage in acpi_platform_fill_resource()
and acpi_create_platform_device() is generally unsafe because in theory
the device returned by it may be freed at any time [1].

It is also inefficient because acpi_get_first_physical_node() is called
multiple times for the same argument which can be avoided.

Address these issues by using acpi_bus_get_primary_device() instead of
acpi_get_first_physical_node() and adjusting the code to call it just
once at the beginning of and acpi_create_platform_device() and drop
the device reference acquired by it upon the return from that function.

Fixes: 3b95bd160547 ("ACPI: introduce a function to find the first physical device")
Fixes: a252d881c558 ("ACPI / platform: Pay attention to parent device's resources")
Link: https://sashiko.dev/#/patchset/12955541.O9o76ZdvQC%40rafael.j.wysocki [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_platform.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -102,18 +102,15 @@ static unsigned int acpi_platform_adjust
 	return count;
 }
 
-static void acpi_platform_fill_resource(struct acpi_device *adev,
-	const struct resource *src, struct resource *dest)
+static void acpi_platform_fill_resource(struct device *parent,
+					const struct resource *src,
+					struct resource *dest)
 {
-	struct device *parent;
-
 	*dest = *src;
-
 	/*
 	 * If the device has parent we need to take its resources into
 	 * account as well because this device might consume part of those.
 	 */
-	parent = acpi_get_first_physical_node(acpi_dev_parent(adev));
 	if (parent && dev_is_pci(parent))
 		dest->parent = pci_find_resource(to_pci_dev(parent), dest);
 }
@@ -141,7 +138,8 @@ static unsigned int acpi_platform_resour
 struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
 						    const struct property_entry *properties)
 {
-	struct acpi_device *parent = acpi_dev_parent(adev);
+	struct acpi_device *p = acpi_dev_parent(adev);
+	struct device *parent __free(put_device) = acpi_bus_get_primary_device(p);
 	struct platform_device *pdev = NULL;
 	struct platform_device_info pdevinfo;
 	const struct acpi_device_id *match;
@@ -187,7 +185,7 @@ struct platform_device *acpi_create_plat
 								       rentry->res,
 								       resources,
 								       count);
-				acpi_platform_fill_resource(adev, rentry->res,
+				acpi_platform_fill_resource(parent, rentry->res,
 							    &resources[count++]);
 			}
 			acpi_dev_free_resource_list(&resource_list);
@@ -200,7 +198,7 @@ struct platform_device *acpi_create_plat
 	 * attached to it, that physical device should be the parent of the
 	 * platform device we are about to create.
 	 */
-	pdevinfo.parent = parent ? acpi_get_first_physical_node(parent) : NULL;
+	pdevinfo.parent = parent;
 	pdevinfo.name = dev_name(&adev->dev);
 	pdevinfo.id = PLATFORM_DEVID_NONE;
 	pdevinfo.res = resources;

[PATCH v1 3/3] ACPI: scan: Use acpi_bus_get_primary_device()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The acpi_get_first_physical_node() usage in acpi_create_video_bus_device()
is generally unsafe because in theory the device returned by it may be
freed at any time.

Address this issues by using acpi_bus_get_primary_device() instead of
acpi_get_first_physical_node() and dropping the device reference
acquired by it after registering the child.

Fixes: 6ab3532b4c98 ("ACPI: video: Switch over to auxiliary bus type")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |   20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2203,29 +2203,27 @@ static void acpi_create_video_bus_device
 	struct auxiliary_device *aux_dev;
 	static unsigned int aux_dev_id;
 
+	struct device *phys_parent __free(put_device) = acpi_bus_get_primary_device(parent);
+	if (!phys_parent)
+		return;
+
 	aux_dev = kzalloc_obj(*aux_dev);
 	if (!aux_dev)
 		return;
 
 	aux_dev->id = aux_dev_id++;
 	aux_dev->name = "video_bus";
-	aux_dev->dev.parent = acpi_get_first_physical_node(parent);
-	if (!aux_dev->dev.parent)
-		goto err;
-
+	aux_dev->dev.parent = phys_parent;
 	aux_dev->dev.release = acpi_video_bus_device_release;
 
-	if (auxiliary_device_init(aux_dev))
-		goto err;
+	if (auxiliary_device_init(aux_dev)) {
+		kfree(aux_dev);
+		return;
+	}
 
 	ACPI_COMPANION_SET(&aux_dev->dev, adev);
 	if (__auxiliary_device_add(aux_dev, "acpi"))
 		auxiliary_device_uninit(aux_dev);
-
-	return;
-
-err:
-	kfree(aux_dev);
 }
 
 struct acpi_scan_system_dev {

Re: [PATCH v1 0/3] ACPI: bus: Rework acquiring the primary physical for a given ACPI one

[Rafael J. Wysocki (Intel)]

On Mon, Aug 10, 2026 at 1:40 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> Hi All,
>
> This is somewhat late, but since there will be follow-up changes tree-wide,
> it would be good to get it into the mainline sooner than later.
>
> It is based on the observation that the majority of
> acpi_get_first_physical_node() callers want to hold a reference on the returned
> device and the function is potentially racy because it doesn't get such a
> reference itself.  There were also some comments from Sashiko pointing
> out to this as a "preexisting issue".
>
> The first patch adds a acpi_get_first_physical_node() replacement called
> acpi_bus_get_primary_device() and returning a reference-counted device (the
> former is retained for the time being, but made use the same code as the
> latter).
>
> The next two patches change the core ACPI code to use the new function
> instead of the old one.
>
> Later on, patches will be sent to switch all of the callers of
> acpi_get_first_physical_node() over to using acpi_bus_get_primary_device()
> and finally to drop the former.
>
> Thanks!

I should have mentioned that this series applies to the linux-next
branch of the linux-pm.git tree (and will apply to linux-next when
that branch is included into it).

Re: [PATCH v1 1/3] ACPI: bus: Introduce acpi_bus_get_primary_device()

[Andy Shevchenko]

On Mon, Aug 10, 2026 at 01:33:41PM +0200, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> The function used for obtaining the first "physical" device for which
> the given ACPI one is the ACPI companion, acpi_get_first_physical_node(),
> may return a stale device pointer (mostly in theory) because
> acpi_unbind_one() may run as a whole after dropping the ACPI device's
> physical_node_lock in acpi_get_first_physical_node() and before it
> returns.  The last reference to the "physical" device may be dropped
> then before the pointer to it is returned to the caller.
> 
> If that happens and the acpi_get_first_physical_node() caller invokes
> get_device() on the pointer obtained from it, which is done by the
> majority of its callers, a use-after-free will occur.
> 
> To prepare for addressing this problem, introduce a new function for
> getting the first "physical" device associated with the given ACPI one
> (the "primary physical device") that will also reference count the
> device in question before returning a pointer to it.
> 
> Make that new function and acpi_get_first_physical_node() share the
> physical node list lookup code.
> 
> No intentional functional impact.

...

> +static struct device *primary_physical_device(struct acpi_device *adev)
> +{
> +	if (list_empty(&adev->physical_node_list))
> +		return NULL;
> +
> +	return list_first_entry(&adev->physical_node_list,
> +				struct acpi_device_physical_node, node)->dev;

This is open-coded list_first_entry_or_null().

I see the ->dev, so having temporary variable will suit this

	struct ... *...;

	... = list_first_entry_or_null(...);
	if (...)
		return ...->dev;

	return NULL;

> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/3] ACPI: bus: Rework acquiring the primary physical for a given ACPI one

[Andy Shevchenko]

On Mon, Aug 10, 2026 at 01:28:44PM +0200, Rafael J. Wysocki wrote:
> Hi All,
> 
> This is somewhat late, but since there will be follow-up changes tree-wide,
> it would be good to get it into the mainline sooner than later.
> 
> It is based on the observation that the majority of
> acpi_get_first_physical_node() callers want to hold a reference on the returned
> device and the function is potentially racy because it doesn't get such a
> reference itself.  There were also some comments from Sashiko pointing
> out to this as a "preexisting issue".
> 
> The first patch adds a acpi_get_first_physical_node() replacement called
> acpi_bus_get_primary_device() and returning a reference-counted device (the
> former is retained for the time being, but made use the same code as the
> latter).
> 
> The next two patches change the core ACPI code to use the new function
> instead of the old one.
> 
> Later on, patches will be sent to switch all of the callers of
> acpi_get_first_physical_node() over to using acpi_bus_get_primary_device()
> and finally to drop the former.

I have one comment in patch 1 (see separate reply) but overall LGTM and I like
the end result.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko