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