diff mbox series

[6.2,regression,fix] gpiolib: Fix using uninitialized lookup-flags on ACPI platforms

Message ID 20221229164501.76044-1-hdegoede@redhat.com
State Accepted
Commit ba2dc1cb5491712a6946d0595cf11ba463f50e64
Headers show
Series [6.2,regression,fix] gpiolib: Fix using uninitialized lookup-flags on ACPI platforms | expand

Commit Message

Hans de Goede Dec. 29, 2022, 4:45 p.m. UTC
Commit 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups") refactors
fwnode_get_named_gpiod() and gpiod_get_index() into a unified
gpiod_find_and_request() helper.

The old functions both initialized their local lookupflags variable to
GPIO_LOOKUP_FLAGS_DEFAULT, but the new code leaves it uninitialized.

This is a problem for at least ACPI platforms, where acpi_find_gpio()
only does a bunch of *lookupflags |= GPIO_* statements and thus relies
on the variable being initialized.

The variable not being initialized leads to:

1. Potentially the wrong flags getting used
2. The check for conflicting lookup flags in gpiod_configure_flags():
   "multiple pull-up, pull-down or pull-disable enabled, invalid config"
   sometimes triggering, making the GPIO unavailable

Restore the initialization of lookupflags to GPIO_LOOKUP_FLAGS_DEFAULT
to fix this.

Fixes: 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note I'm not working and not reading work email until Monday January 9th.
I hit this while doing some hobby stuff and I decided to send this out
right away to avoid others potentially wasting time debugging this, but
I will not see any replies until Monday January 9th.
---
 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bartosz Golaszewski Dec. 30, 2022, 10:04 a.m. UTC | #1
On Thu, Dec 29, 2022 at 5:45 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Commit 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups") refactors
> fwnode_get_named_gpiod() and gpiod_get_index() into a unified
> gpiod_find_and_request() helper.
>
> The old functions both initialized their local lookupflags variable to
> GPIO_LOOKUP_FLAGS_DEFAULT, but the new code leaves it uninitialized.
>
> This is a problem for at least ACPI platforms, where acpi_find_gpio()
> only does a bunch of *lookupflags |= GPIO_* statements and thus relies
> on the variable being initialized.
>
> The variable not being initialized leads to:
>
> 1. Potentially the wrong flags getting used
> 2. The check for conflicting lookup flags in gpiod_configure_flags():
>    "multiple pull-up, pull-down or pull-disable enabled, invalid config"
>    sometimes triggering, making the GPIO unavailable
>
> Restore the initialization of lookupflags to GPIO_LOOKUP_FLAGS_DEFAULT
> to fix this.
>
> Fixes: 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note I'm not working and not reading work email until Monday January 9th.
> I hit this while doing some hobby stuff and I decided to send this out
> right away to avoid others potentially wasting time debugging this, but
> I will not see any replies until Monday January 9th.
> ---
>  drivers/gpio/gpiolib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5a66d9616d7c..939c776b9488 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -3905,8 +3905,8 @@ static struct gpio_desc *gpiod_find_and_request(struct device *consumer,
>                                                 const char *label,
>                                                 bool platform_lookup_allowed)
>  {
> +       unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT;
>         struct gpio_desc *desc = ERR_PTR(-ENOENT);
> -       unsigned long lookupflags;
>         int ret;
>
>         if (!IS_ERR_OR_NULL(fwnode))
> --
> 2.38.1
>

Queued for fixes, thanks!

Bart
Linux regression tracking (Thorsten Leemhuis) Dec. 30, 2022, 10:11 a.m. UTC | #2
[Note: this mail contains only information for Linux kernel regression
tracking. Mails like these contain '#forregzbot' in the subject to make
then easy to spot and filter out. The author also tried to remove most
or all individuals from the list of recipients to spare them the hassle.]

On 29.12.22 17:45, Hans de Goede wrote:
> Commit 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups") refactors
> fwnode_get_named_gpiod() and gpiod_get_index() into a unified
> gpiod_find_and_request() helper.
> 
> The old functions both initialized their local lookupflags variable to
> GPIO_LOOKUP_FLAGS_DEFAULT, but the new code leaves it uninitialized.
> 
> This is a problem for at least ACPI platforms, where acpi_find_gpio()
> only does a bunch of *lookupflags |= GPIO_* statements and thus relies
> on the variable being initialized.
> 
> The variable not being initialized leads to:
> 
> 1. Potentially the wrong flags getting used
> 2. The check for conflicting lookup flags in gpiod_configure_flags():
>    "multiple pull-up, pull-down or pull-disable enabled, invalid config"
>    sometimes triggering, making the GPIO unavailable
> 
> Restore the initialization of lookupflags to GPIO_LOOKUP_FLAGS_DEFAULT
> to fix this.
> 
> Fixes: 8eb1f71e7acc ("gpiolib: consolidate GPIO lookups")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note I'm not working and not reading work email until Monday January 9th.
> I hit this while doing some hobby stuff and I decided to send this out
> right away to avoid others potentially wasting time debugging this, but
> I will not see any replies until Monday January 9th.

Thanks for the report. To be sure below issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
tracking bot:

#regzbot ^introduced 8eb1f71e7acc
#regzbot title gpiolib: potentially the wrong flags getting used
#regzbot fix: gpiolib: Fix using uninitialized lookup-flags on ACPI
platforms
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5a66d9616d7c..939c776b9488 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3905,8 +3905,8 @@  static struct gpio_desc *gpiod_find_and_request(struct device *consumer,
 						const char *label,
 						bool platform_lookup_allowed)
 {
+	unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT;
 	struct gpio_desc *desc = ERR_PTR(-ENOENT);
-	unsigned long lookupflags;
 	int ret;
 
 	if (!IS_ERR_OR_NULL(fwnode))