linux-kernel - Re: [PATCH v1 2/2] gpiolib: acpi: Make sure we fill struct acpi_gpio

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: <4793eafb-bd9c-4857-a020-944fdb97fb9e@ijzerbout.nl>
Date: Fri, 11 Apr 2025 20:22:41 +0200
From: Kees Bakker <kees@...erbout.nl>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 linux-gpio@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-kernel@...r.kernel.org, Mika Westerberg <westeri@...nel.org>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH v1 2/2] gpiolib: acpi: Make sure we fill struct
 acpi_gpio_info

Op 10-04-2025 om 20:41 schreef Andy Shevchenko:
> Wed, Apr 09, 2025 at 08:16:59PM +0200, Kees Bakker kirjoitti:
>> Op 09-04-2025 om 15:27 schreef Andy Shevchenko:
> ...
>
>> Can you check and confirm that at least info.gpioint is filled in (or
>> initialized)?
> Yes, I can confirm this. And that's how I have tested it, on Intel
> Edison/Arduino the first GPIO expander (PCAL9555, serviced by
> drivers/gpio/gpio-pca953x.c) is able to deliver an interrupt to the SoC.
>
> Before this series that doesn't show up, now it works as expected.
>
>> The callers of `__acpi_find_gpio` pass in an uninitialized `struct
>> acpi_gpio_info`
> True.
>
>> and after the call they read `info.gpioint`.
> ...when GPIO descriptor is valid.
>
> ...
>
> Yes, I agree that NULLifying info maybe good to have, but I don't see currently
> if we have bugs with it. Can you be more specific in case you have observed
> anything wrong?
Thanks for confirming.
I have nothing specific, just manually reviewing the (new) patch.
So, I guess this code in `acpi_populate_gpio_lookup` takes care of it.
         lookup->desc = desc;
         info->pin_config = agpio->pin_config;
         info->debounce = agpio->debounce_timeout;
         info->gpioint = gpioint;
         info->wake_capable = acpi_gpio_irq_is_wake(&info->adev->dev, 
agpio);
That would sound plausible.
-- 
Kees