From: Brian Norris <briannorris@chromium.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rob Herring (Arm)" <robh@kernel.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] driver core: Don't match device with NULL of_node/fwnode
Date: Mon, 9 Dec 2024 15:47:46 -0800 [thread overview]
Message-ID: <Z1eBotg2DiaXLWqn@google.com> (raw)
In-Reply-To: <2024120450-jogging-duty-fad4@gregkh>
Hi Greg,
On Wed, Dec 04, 2024 at 10:37:06AM +0100, Greg Kroah-Hartman wrote:
> On Tue, Dec 03, 2024 at 06:02:59PM -0600, Rob Herring (Arm) wrote:
> > It recently came up that of_find_device_by_node() will match a device
> > with a NULL of_node pointer. This is not desired behavior. The returned
> > struct device is also not deterministic.
>
> It's not deterministic because a NULL pointer will cause that to happen,
> or for some other reason?
It'll pick the first platform device with no of_node. That likely yields
something very wrong, but doesn't produce a visible problem until a
caller does something with the result. Commit 5c8418cf4025
("PCI/pwrctrl: Unregister platform device only if one actually exists")
has plenty of explanation of what really goes wrong.
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> > ---
> > It would be a bit more efficient to check this up front before we
> > iterate thru devices, but there's a number of users of these functions
> > and this isn't really a hot path.
>
> Yeah, this should be fine. Does this fix a problem now and we need it
> merged for 6.13-final and backported, or can it just wait for 6.14-rc1?
It's a preventive measure to help head off future confusing bugs. It
doesn't need expedited merging or backporting.
FWIW, last week, I also cooked this change locally (+ the ACPI change;
and a kunit test for added fun), before I noticed Rob submitted this
one. If you'd rather, I can submit my patch series. Or I can submit my
patch series on top of this. Whichever you'd prefer.
Brian
next prev parent reply other threads:[~2024-12-09 23:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-04 0:02 [PATCH] driver core: Don't match device with NULL of_node/fwnode Rob Herring (Arm)
2024-12-04 9:37 ` Greg Kroah-Hartman
2024-12-09 23:47 ` Brian Norris [this message]
2024-12-10 12:33 ` Rob Herring
2024-12-10 19:17 ` Brian Norris
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z1eBotg2DiaXLWqn@google.com \
--to=briannorris@chromium.org \
--cc=bhelgaas@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.