[PATCH v2 0/4] ACPI: Add and use acpi_dev_get_pci_dev()

[PATCH v2 0/4] ACPI: Add and use acpi_dev_get_pci_dev()

[Rafael J. Wysocki]

Hi All,

This is an update of

https://lore.kernel.org/linux-acpi/5115219.31r3eYUQgx@rafael.j.wysocki/

that is sent to address review comments from Andy.

The original series description below still applies.

This series is based on the observation that the majority of acpi_get_pci_dev()
callers already know the ACPI device they want to look up the PCI device for
and so they don't need to look up the ACPI device.  Accordingly, a function for
PCI device lookup based on an ACPI device is introduced and used.

The first patch is a simplification utilizing a mutex guard.

The second patch introduces acpi_dev_get_pci_dev() and redefines
acpi_get_pci_dev() as a static inline helper.

The third patch moves a device_put() call in the ACPI video bus driver to
a place after the device in question has been used.

The last patch updates the ACPI video bus driver, which is the heaviest user
of acpi_get_pci_dev(), to use acpi_dev_get_pci_dev() instead of it.

Thanks!

[PATCH v2 1/4] ACPI: PCI: Use a mutex guard to simplify acpi_get_pci_dev()

[Rafael J. Wysocki]

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

Use a mutex guard in acpi_get_pci_dev() for the physical_node_lock
locking and drop local variable pci_dev that becomes redundant after
that change.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Add cleanup.h include to pci_root.c (Andy)

---
 drivers/acpi/pci_root.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 4c06c3ffd0cb..d0dafc38b0cf 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -8,6 +8,7 @@
 
 #define pr_fmt(fmt) "ACPI: " fmt
 
+#include <linux/cleanup.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -307,24 +308,20 @@ struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
 {
 	struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
 	struct acpi_device_physical_node *pn;
-	struct pci_dev *pci_dev = NULL;
 
 	if (!adev)
 		return NULL;
 
-	mutex_lock(&adev->physical_node_lock);
+	guard(mutex)(&adev->physical_node_lock);
 
 	list_for_each_entry(pn, &adev->physical_node_list, node) {
 		if (dev_is_pci(pn->dev)) {
 			get_device(pn->dev);
-			pci_dev = to_pci_dev(pn->dev);
-			break;
+			return to_pci_dev(pn->dev);
 		}
 	}
 
-	mutex_unlock(&adev->physical_node_lock);
-
-	return pci_dev;
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
 
-- 
2.51.0

[PATCH v2 2/4] ACPI: PCI: Introduce acpi_dev_get_pci_dev()

[Rafael J. Wysocki]

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

Some acpi_get_pci_dev() callers already have a struct ACPI device for
which they want to get the struct pci_dev pointer of the associated
PCI device, so they don't need to look for one.

For this reason, add acpi_dev_get_pci_dev() that will get a PCI device
for a given ACPI one (if possible) and turn acpi_get_pci_dev() into
a static inline helper passing the acpi_fetch_acpi_dev() return value
directly to acpi_dev_get_pci_dev().

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Rephrase a new comment in acpi_dev_get_pci_dev() (Andy)

---
 drivers/acpi/pci_root.c     | 22 +++++++++++-----------
 include/acpi/acpi_drivers.h |  9 +++++++--
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index d0dafc38b0cf..424f1b350983 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -293,20 +293,20 @@ struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
 EXPORT_SYMBOL_GPL(acpi_pci_find_root);
 
 /**
- * acpi_get_pci_dev - convert ACPI CA handle to struct pci_dev
- * @handle: the handle in question
+ * acpi_dev_get_pci_dev - Get a struct pci_dev for a given ACPI device
+ * @adev: Target ACPI device.
  *
- * Given an ACPI CA handle, the desired PCI device is located in the
- * list of PCI devices.
+ * Find the PCI device associated with @adev, if any, and bump up its reference
+ * counter.
  *
- * If the device is found, its reference count is increased and this
- * function returns a pointer to its data structure.  The caller must
- * decrement the reference count by calling pci_dev_put().
- * If no device is found, %NULL is returned.
+ * Callers are responsible for dropping the PCI device reference obtained by
+ * this function.
+ *
+ * Return: The struct pci_dev pointer of a reference-counted PCI device on
+ * success or NULL on failure.
  */
-struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
+struct pci_dev *acpi_dev_get_pci_dev(struct acpi_device *adev)
 {
-	struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
 	struct acpi_device_physical_node *pn;
 
 	if (!adev)
@@ -323,7 +323,7 @@ struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
 
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
+EXPORT_SYMBOL_GPL(acpi_dev_get_pci_dev);
 
 /**
  * acpi_pci_osc_control_set - Request control of PCI root _OSC features.
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 402b97d12138..bff2ca035fcf 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -59,14 +59,19 @@ int acpi_pci_link_free_irq(acpi_handle handle);
 struct pci_bus;
 
 #ifdef CONFIG_PCI
-struct pci_dev *acpi_get_pci_dev(acpi_handle);
+struct pci_dev *acpi_dev_get_pci_dev(struct acpi_device *adev);
 #else
-static inline struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
+static inline struct pci_dev *acpi_dev_get_pci_dev(struct acpi_device *adev)
 {
 	return NULL;
 }
 #endif
 
+static inline struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
+{
+	return acpi_dev_get_pci_dev(acpi_fetch_acpi_dev(handle));
+}
+
 /* Arch-defined function to add a bus to the system */
 
 struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root);
-- 
2.51.0

[PATCH v2 3/4] ACPI: video: Drop backlight parent device reference later

[Rafael J. Wysocki]

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

Update acpi_video_dev_register_backlight() to put the parent device after
registering the backlight class device under it instead of attempting to
register the backlight class device under a parent that (theoretically)
may be gone at that point.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

v1 -> v2:
   * Add R-by from Andy

---
 drivers/acpi/acpi_video.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index f93e877f87f6..a738265279cd 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1719,10 +1719,8 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
 
 	if (ACPI_SUCCESS(acpi_get_parent(device->dev->handle, &acpi_parent))) {
 		pdev = acpi_get_pci_dev(acpi_parent);
-		if (pdev) {
+		if (pdev)
 			parent = &pdev->dev;
-			pci_dev_put(pdev);
-		}
 	}
 
 	memset(&props, 0, sizeof(struct backlight_properties));
@@ -1734,6 +1732,7 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
 						      device,
 						      &acpi_backlight_ops,
 						      &props);
+	put_device(parent);
 	kfree(name);
 	if (IS_ERR(device->backlight)) {
 		device->backlight = NULL;
-- 
2.51.0

[PATCH v2 4/4] ACPI: video: Use acpi_dev_get_pci_dev() instead of acpi_get_pci_dev()

[Rafael J. Wysocki]

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

In acpi_video_bus_check() and find_video(), simply replace
acpi_get_pci_dev() with acpi_dev_get_pci_dev() that can be used
in both places because the ACPI device needed to do the lookup is
available.

In acpi_video_dev_register_backlight(), instead of doing a parent ACPI
handle lookup based on the handle of an ACPI device that is already
available, pass that ACPI device to acpi_dev_parent() which is much
more straightforward and pass the return value of the latter directly
to acpi_dev_get_pci_dev() to get the PCI device associated with it.
That allows local variable acpi_parent to be eliminated.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

v1 -> v2:
   * Add R-by from Andy

---
 drivers/acpi/acpi_video.c   | 11 ++++-------
 drivers/acpi/video_detect.c |  2 +-
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index a738265279cd..4d6fd9f6e9ad 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1048,7 +1048,7 @@ static int acpi_video_bus_check(struct acpi_video_bus *video)
 	if (!video)
 		return -EINVAL;
 
-	dev = acpi_get_pci_dev(video->device->handle);
+	dev = acpi_dev_get_pci_dev(video->device);
 	if (!dev)
 		return -ENODEV;
 	pci_dev_put(dev);
@@ -1702,7 +1702,6 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
 {
 	struct backlight_properties props;
 	struct pci_dev *pdev;
-	acpi_handle acpi_parent;
 	struct device *parent = NULL;
 	int result;
 	static int count;
@@ -1717,11 +1716,9 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
 		return;
 	count++;
 
-	if (ACPI_SUCCESS(acpi_get_parent(device->dev->handle, &acpi_parent))) {
-		pdev = acpi_get_pci_dev(acpi_parent);
-		if (pdev)
-			parent = &pdev->dev;
-	}
+	pdev = acpi_dev_get_pci_dev(acpi_dev_parent(device->dev));
+	if (pdev)
+		parent = &pdev->dev;
 
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_FIRMWARE;
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 458efa4fe9d4..c5b08c19e847 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -73,7 +73,7 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
 	};
 
 	if (acpi_dev && !acpi_match_device_ids(acpi_dev, video_ids)) {
-		dev = acpi_get_pci_dev(handle);
+		dev = acpi_dev_get_pci_dev(acpi_dev);
 		if (!dev)
 			return AE_OK;
 		pci_dev_put(dev);
-- 
2.51.0

Re: [PATCH v2 1/4] ACPI: PCI: Use a mutex guard to simplify acpi_get_pci_dev()

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Use a mutex guard in acpi_get_pci_dev() for the physical_node_lock
> locking and drop local variable pci_dev that becomes redundant after
> that change.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/12935400.O9o76ZdvQC@rafael.j.wysocki?part=1

Re: [PATCH v2 3/4] ACPI: video: Drop backlight parent device reference later

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Update acpi_video_dev_register_backlight() to put the parent device after
> registering the backlight class device under it instead of attempting to
> register the backlight class device under a parent that (theoretically)
> may be gone at that point.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/12935400.O9o76ZdvQC@rafael.j.wysocki?part=3

Re: [PATCH v2 2/4] ACPI: PCI: Introduce acpi_dev_get_pci_dev()

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-after-free in acpi_get_pci_dev() due to unpinned acpi_device pointer
--

--- Patch [2]: [PATCH v2 2/4] ACPI: PCI: Introduce acpi_dev_get_pci_dev() ---
commit 96691f509a95b92252e42efea5c07ebf2acebfd2
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

ACPI: PCI: Introduce acpi_dev_get_pci_dev()

Some acpi_get_pci_dev() callers already have a struct ACPI device for
which they want to get the struct pci_dev pointer of the associated
PCI device. Add acpi_dev_get_pci_dev() to handle this and turn
acpi_get_pci_dev() into a static inline helper.

> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index 402b97d121381..bff2ca035fcf3 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -59,14 +59,19 @@ int acpi_pci_link_free_irq(acpi_handle handle);
[ ... ]
> +static inline struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
> +{
> +	return acpi_dev_get_pci_dev(acpi_fetch_acpi_dev(handle));
> +}

[Severity: High]
This is a pre-existing issue, but does this inline function expose a
race condition with ACPI hot-remove?

acpi_fetch_acpi_dev() returns an unpinned acpi_device pointer without
incrementing its reference count. If a concurrent ACPI hot-remove event
occurs immediately after the lookup, the device object could be destroyed
and freed by the hotplug workqueue.

When the dangling pointer is passed into acpi_dev_get_pci_dev(), it
gets dereferenced here in drivers/acpi/pci_root.c:

acpi_dev_get_pci_dev() {
    ...
    guard(mutex)(&adev->physical_node_lock);

Could this be prevented by using acpi_get_acpi_dev() to acquire a reference
before calling acpi_dev_get_pci_dev(), and then releasing it with
acpi_dev_put()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/12935400.O9o76ZdvQC@rafael.j.wysocki?part=2

Re: [PATCH v2 4/4] ACPI: video: Use acpi_dev_get_pci_dev() instead of acpi_get_pci_dev()

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> In acpi_video_bus_check() and find_video(), simply replace
> acpi_get_pci_dev() with acpi_dev_get_pci_dev() that can be used
> in both places because the ACPI device needed to do the lookup is
> available.
> 
> In acpi_video_dev_register_backlight(), instead of doing a parent ACPI
> handle lookup based on the handle of an ACPI device that is already
> available, pass that ACPI device to acpi_dev_parent() which is much
> more straightforward and pass the return value of the latter directly
> to acpi_dev_get_pci_dev() to get the PCI device associated with it.
> That allows local variable acpi_parent to be eliminated.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/12935400.O9o76ZdvQC@rafael.j.wysocki?part=4

Re: [PATCH v2 2/4] ACPI: PCI: Introduce acpi_dev_get_pci_dev()

[Rafael J. Wysocki (Intel)]

On Thu, Jul 16, 2026 at 4:18 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Use-after-free in acpi_get_pci_dev() due to unpinned acpi_device pointer
> --
>
> --- Patch [2]: [PATCH v2 2/4] ACPI: PCI: Introduce acpi_dev_get_pci_dev() ---
> commit 96691f509a95b92252e42efea5c07ebf2acebfd2
> Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> ACPI: PCI: Introduce acpi_dev_get_pci_dev()
>
> Some acpi_get_pci_dev() callers already have a struct ACPI device for
> which they want to get the struct pci_dev pointer of the associated
> PCI device. Add acpi_dev_get_pci_dev() to handle this and turn
> acpi_get_pci_dev() into a static inline helper.
>
> > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> > index 402b97d121381..bff2ca035fcf3 100644
> > --- a/include/acpi/acpi_drivers.h
> > +++ b/include/acpi/acpi_drivers.h
> > @@ -59,14 +59,19 @@ int acpi_pci_link_free_irq(acpi_handle handle);
> [ ... ]
> > +static inline struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
> > +{
> > +     return acpi_dev_get_pci_dev(acpi_fetch_acpi_dev(handle));
> > +}
>
> [Severity: High]
> This is a pre-existing issue, but does this inline function expose a
> race condition with ACPI hot-remove?
>
> acpi_fetch_acpi_dev() returns an unpinned acpi_device pointer without
> incrementing its reference count. If a concurrent ACPI hot-remove event
> occurs immediately after the lookup, the device object could be destroyed
> and freed by the hotplug workqueue.
>
> When the dangling pointer is passed into acpi_dev_get_pci_dev(), it
> gets dereferenced here in drivers/acpi/pci_root.c:
>
> acpi_dev_get_pci_dev() {
>     ...
>     guard(mutex)(&adev->physical_node_lock);
>
> Could this be prevented by using acpi_get_acpi_dev() to acquire a reference
> before calling acpi_dev_get_pci_dev(), and then releasing it with
> acpi_dev_put()?

This just retains the previous behavior of the function in question
and after the 4th patch in the series, it is not a real issue for the
last 2 remaining callers.

Re: [PATCH v2 1/4] ACPI: PCI: Use a mutex guard to simplify acpi_get_pci_dev()

[Andy Shevchenko]

On Thu, Jul 16, 2026 at 04:04:43PM +0200, Rafael J. Wysocki wrote:

> Use a mutex guard in acpi_get_pci_dev() for the physical_node_lock
> locking and drop local variable pci_dev that becomes redundant after
> that change.
> 
> No intentional functional impact.

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


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/4] ACPI: PCI: Introduce acpi_dev_get_pci_dev()

[Andy Shevchenko]

On Thu, Jul 16, 2026 at 04:04:53PM +0200, Rafael J. Wysocki wrote:

> Some acpi_get_pci_dev() callers already have a struct ACPI device for
> which they want to get the struct pci_dev pointer of the associated
> PCI device, so they don't need to look for one.
> 
> For this reason, add acpi_dev_get_pci_dev() that will get a PCI device
> for a given ACPI one (if possible) and turn acpi_get_pci_dev() into
> a static inline helper passing the acpi_fetch_acpi_dev() return value
> directly to acpi_dev_get_pci_dev().
> 
> No intentional functional impact.

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

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/4] ACPI: PCI: Use a mutex guard to simplify acpi_get_pci_dev()

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

On Thu, 16 Jul 2026, Rafael J. Wysocki wrote:

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Use a mutex guard in acpi_get_pci_dev() for the physical_node_lock
> locking and drop local variable pci_dev that becomes redundant after
> that change.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> v1 -> v2:
>    * Add cleanup.h include to pci_root.c (Andy)
> 
> ---
>  drivers/acpi/pci_root.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 4c06c3ffd0cb..d0dafc38b0cf 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -8,6 +8,7 @@
>  
>  #define pr_fmt(fmt) "ACPI: " fmt
>  
> +#include <linux/cleanup.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
> @@ -307,24 +308,20 @@ struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
>  {
>  	struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
>  	struct acpi_device_physical_node *pn;
> -	struct pci_dev *pci_dev = NULL;
>  
>  	if (!adev)
>  		return NULL;
>  
> -	mutex_lock(&adev->physical_node_lock);
> +	guard(mutex)(&adev->physical_node_lock);
>  
>  	list_for_each_entry(pn, &adev->physical_node_list, node) {
>  		if (dev_is_pci(pn->dev)) {
>  			get_device(pn->dev);
> -			pci_dev = to_pci_dev(pn->dev);
> -			break;
> +			return to_pci_dev(pn->dev);
>  		}
>  	}
>  
> -	mutex_unlock(&adev->physical_node_lock);
> -
> -	return pci_dev;
> +	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(acpi_get_pci_dev);

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.