Re: [PATCH v1 1/3] ACPI: bus: Introduce acpi_bus_get_primary_device()
"Rafael J. Wysocki (Intel)" <[email protected]> Wed, 12 Aug 2026 14:02:31 +0200
| Newsgroups | gmane.linux.kernel,gmane.linux.acpi.devel |
|---|---|
| Message-ID | <CAJZ5v0gjJ7N-gRXPrk2=K+4-y3Oim0TE7xbj9hnZc4VbJDuU1A@mail.gmail.com> |
On Mon, Aug 10, 2026 at 5:43 PM Andy Shevchenko <[email protected]> wrote: > > On Mon, Aug 10, 2026 at 01:33:41PM +0200, Rafael J. Wysocki wrote: > > From: "Rafael J. Wysocki" <[email protected]> > > > > 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(). You're right, I forgot about this one. I'll send an update of this particular patch shortly. > I see the ->dev, so having temporary variable will suit this > > struct ... *...; > > ... = list_first_entry_or_null(...); > if (...) > return ...->dev; > > return NULL; > > > +} > > --