linux-kernel - Re: [PATCH] PCI/pwrctl: Do not assume device node presence

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL_JsqL7w4BBOqb=NaOh7Xewe5QXrSF+7SYoFtay0O5hw1QnTw@mail.gmail.com>
Date: Tue, 3 Dec 2024 17:49:24 -0600
From: Rob Herring <robh@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Chen-Yu Tsai <wenst@...omium.org>, Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Klara Modin <klarasmodin@...il.com>, 
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, 
	Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, stable+noautosel@...nel.org, 
	Saravana Kannan <saravanak@...gle.com>, devicetree@...r.kernel.org
Subject: Re: [PATCH] PCI/pwrctl: Do not assume device node presence

On Thu, Nov 21, 2024 at 10:46 AM Bjorn Helgaas <helgaas@...nel.org> wrote:
>
> [+cc OF folks]
>
> On Thu, Nov 21, 2024 at 05:40:19PM +0800, Chen-Yu Tsai wrote:
> > A PCI device normally does not have a device node, since the bus is
> > fully enumerable. Assuming that a device node is presence is likely
> > bad.
>
> > The newly added pwrctl code assumes such and crashes with a NULL
> > pointer dereference.
>
> > Besides that, of_find_device_by_node(NULL)
> > is likely going to return some random device.
>
> I thought this sounded implausible, but after looking at the code, I
> think you're right, because bus_find_device() will use
> device_match_of_node(), which decides the device matches if
> "dev->of_node == np" (where "np" is NULL in this case).
>
> I'm sure many devices will have "dev->of_node == NULL", so it does
> seem like of_find_device_by_node(NULL) will return the first one it
> finds.
>
> This seems ... pretty janky and unexpected to me, but it's been this
> way for years, so maybe it's safe?  Cc'ing the OF folks just in case.

This is a surprise to me, too. I think ACPI matching is broken in this
way too. I'm sending out a fix.

Rob