linux-kernel - Re: [PATCH v1 1/4] ACPI: PNP: Drop PNP0C01 and PNP0C02 from acpi_pnp_device

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aVKI9lVAepGw4lVg@smile.fi.intel.com>
Date: Mon, 29 Dec 2025 15:58:14 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Linux ACPI <linux-acpi@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Bjorn Helgaas <helgaas@...nel.org>, Zhang Rui <rui.zhang@...el.com>,
	Chen Yu <yu.c.chen@...el.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Linux PCI <linux-pci@...r.kernel.org>,
	Alex Hung <alexhung@...il.com>, Hans de Goede <hansg@...nel.org>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	platform-driver-x86@...r.kernel.org,
	AceLan Kao <acelan.kao@...onical.com>
Subject: Re: [PATCH v1 1/4] ACPI: PNP: Drop PNP0C01 and PNP0C02 from
 acpi_pnp_device_ids[]

On Mon, Dec 29, 2025 at 02:12:10PM +0100, Rafael J. Wysocki wrote:
> On Sun, Dec 28, 2025 at 8:11 PM Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> wrote:
> > On Mon, Dec 15, 2025 at 02:34:06PM +0100, Rafael J. Wysocki wrote:

...

> > > +     if (match_string(acpi_system_dev_ids, -1, acpi_device_hid(device)) >= 0) {
> >
> > Using -1 makes sense when we have no direct visibility of the mentioned array.
> > Here we have it visible and statically defined, hence the ARRAY_SIZE() is more
> > appropriate.
> 
> But as it stands it is more consistent with what happens elsewhere in this file.

It's not a big deal, especially if there is some consistency involved.

...

> > > +static void acpi_scan_claim_resources(struct acpi_device *adev)
> > > +{
> > > +     struct list_head resource_list = LIST_HEAD_INIT(resource_list);
> > > +     struct resource_entry *rentry;
> > > +     unsigned int count = 0;
> > > +     const char *regionid;
> >
> > > +     if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0)
> > > +             return;
> >
> > Strictly speaking the acpi_dev_free_resource_list() still needs to be called
> > on 0 return as it's called only for the error cases.
> 
> The return value of 0 means that the list is empty, so there's nothing to free.
> 
> > I think this is the first and the only time I see a combined comparison <= 0
> > for the acpi_dev_get_resources().
> 
> I don't see a problem here though.

There is no problem, the only minor thing is that the pattern I see seems only
here.

> > > +     regionid = kstrdup(dev_name(&adev->dev), GFP_KERNEL);
> > > +     if (!regionid)
> > > +             goto exit;
> > > +
> > > +     list_for_each_entry(rentry, &resource_list, node) {
> > > +             struct resource *res = rentry->res;
> > > +             struct resource *r;
> > > +
> > > +             /* Skip disabled and invalid resources. */
> > > +             if ((res->flags & IORESOURCE_DISABLED) || res->end < res->start)
> > > +                     continue;
> >
> > > +             if (res->flags & IORESOURCE_IO) {
> >
> > We have resource_type() helper.
> 
> I had considered using it, but then I couldn't convince myself that it
> would matter, so I went for simpler code.
> 
> > And I believe the direct comparison in this case is better.
> 
> You seem to be arguing that it matters after all, so why does it matter?

In the light of the resent findings of the resource_size() use on zeroed data
it also might be that in some cases the opposite happens, i.e. the resource had
initially a more bits of type assigned while checked for the certain one(s).
In such case the resource_type() == IORESOURCE_foo will fail on those.

I assume, nevertheless, that the code in question is quite unlikely exposed to
such mistakes.

-- 
With Best Regards,
Andy Shevchenko