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: <CAFr9PXmEMFC6aVFre6smw0BWF_3XTfC4OG-Gc9zSCo_cobkqjg@mail.gmail.com>
Date: Mon, 12 Jan 2026 12:10:27 +0900
From: Daniel Palmer <daniel@...ngy.jp>
To: Saravana Kannan <saravanak@...nel.org>
Cc: Rob Herring <robh@...nel.org>, linusw@...nel.org, brgl@...nel.org, 
	linux-gpio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/2] of: Add a variant of of_device_is_compatible()
 that can be build time culled

Hi Saravana,

On Mon, 12 Jan 2026 at 11:32, Saravana Kannan <saravanak@...nel.org> wrote:

> For the 10 or so instances in the core, I'm not sure the macro is even
> worth it. It's just hiding the IS_ENABLED() and obscuring the intent
> for not much of a reduction in code size. Not going to Nack it if Rob
> agrees, but I don't see the point of the macro. I see the point behind
> the idea though.

Understood. This is very niche. In my case by doing stuff like this I
could make the kernel small enough for my userland to boot again.
I have no mmu, 8MB of RAM, CPU frequency in the tens of MHz. Not very common.

But I think it'd also be nice if all of the tons of arm64 machines out
there don't check for hacks for stuff that is only in ppc and
therefore can never happen. :)
While looking through the users of of_device_is_compatible() I also
found some, I think, bad patterns that are probably worse than the
unneeded code/data.

For example, in drivers/usb/host/uhci-platform.c we have:

static int uhci_hcd_platform_probe(struct platform_device *pdev)
{
...

if (of_device_is_compatible(np, "aspeed,ast2400-uhci") ||
   of_device_is_compatible(np, "aspeed,ast2500-uhci") ||
   of_device_is_compatible(np, "aspeed,ast2600-uhci")) {
uhci->is_aspeed = 1;
dev_info(&pdev->dev,
"Enabled Aspeed implementation workarounds\n");
}
...

The compiler will always include those calls if this driver is built.
But the is_aspeed flag they are setting is accessed via a macro that
always returns false if the aspeed platform that needs workarounds
isn't enabled to remove the unneeded code.
I don't think this is a bug that can cause problems but this means if
you use a DT with those strings and the kernel doesn't have that
platform enabled (and it somehow boots far enough etc etc) you'd see
this message saying the workarounds are enabled but actually the
compiler removed all of that code and the message is a lie.

I think Rob wrote basically the same thing but it seems like
of_device_is_compatible() in probe functions is just a bad idea and
workaround flags etc should be in the match data not decided at
runtime with of_device_is_compatible().

Cheers,

Daniel

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ