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 |
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
[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 --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))
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(-)