linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Bug introduced in the of_get_named_gpiod_flags function.
@ 2018-10-10 16:13 wzab
  2018-10-10 16:39 ` Randy Dunlap
  2018-10-11  8:27 ` Linus Walleij
  0 siblings, 2 replies; 8+ messages in thread
From: wzab @ 2018-10-10 16:13 UTC (permalink / raw)
  To: LKML

Hi,

The function of_get_named_gpiod_flags in older versions of the kernel 
(up to 4.7.10 - https://elixir.bootlin.com/linux/v4.7.10/source/drivers/gpio/gpiolib-of.c#L75 )
contained an important workaround:

/* .of_xlate might decide to not fill in the flags, so clear it. */if (flags)
   *flags = 0; Unfortunately, newer kernels do not contain it. Therefore if the
"xlat" function in the gpiochip driver does not set flags, (like e.g.
the Xilinx AXI GPIO driver: https://github.com/Xilinx/linux-xlnx/blob/c2ba891326bb472da59b6a2da29aca218d337687/drivers/gpio/gpio-xilinx.c#L262 )
the random, unitialized value from the stack in of_find_gpio 
( https://elixir.bootlin.com/linux/v4.18.13/source/drivers/gpio/gpiolib-of.c#L228 )
is used, which results in random settings of e.g., OPEN DRAIN or OPEN SOURCE mode.

I have also reported the problem in the Xilinx forum: https://forums.xilinx.com/t5/Embedded-Linux/Bug-in-of-get-named-gpiod-flags-function-in-the-kernel-random/td-p/897695

With best regards,
Wojciech Zabolotny




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Bug introduced in the of_get_named_gpiod_flags function.
  2018-10-10 16:13 Bug introduced in the of_get_named_gpiod_flags function wzab
@ 2018-10-10 16:39 ` Randy Dunlap
  2018-10-11  8:27 ` Linus Walleij
  1 sibling, 0 replies; 8+ messages in thread
From: Randy Dunlap @ 2018-10-10 16:39 UTC (permalink / raw)
  To: wzab, LKML, Linus Walleij, linux-gpio

[adding linux-gpio + Linus W.]

On 10/10/18 9:13 AM, wzab wrote:
> Hi,
> 
> The function of_get_named_gpiod_flags in older versions of the kernel 
> (up to 4.7.10 - https://elixir.bootlin.com/linux/v4.7.10/source/drivers/gpio/gpiolib-of.c#L75 )
> contained an important workaround:
> 
> /* .of_xlate might decide to not fill in the flags, so clear it. */if (flags)
>    *flags = 0; Unfortunately, newer kernels do not contain it. Therefore if the
> "xlat" function in the gpiochip driver does not set flags, (like e.g.
> the Xilinx AXI GPIO driver: https://github.com/Xilinx/linux-xlnx/blob/c2ba891326bb472da59b6a2da29aca218d337687/drivers/gpio/gpio-xilinx.c#L262 )
> the random, unitialized value from the stack in of_find_gpio 
> ( https://elixir.bootlin.com/linux/v4.18.13/source/drivers/gpio/gpiolib-of.c#L228 )
> is used, which results in random settings of e.g., OPEN DRAIN or OPEN SOURCE mode.
> 
> I have also reported the problem in the Xilinx forum: https://forums.xilinx.com/t5/Embedded-Linux/Bug-in-of-get-named-gpiod-flags-function-in-the-kernel-random/td-p/897695
> 
> With best regards,
> Wojciech Zabolotny
> 
> 
> 


-- 
~Randy

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Bug introduced in the of_get_named_gpiod_flags function.
  2018-10-10 16:13 Bug introduced in the of_get_named_gpiod_flags function wzab
  2018-10-10 16:39 ` Randy Dunlap
@ 2018-10-11  8:27 ` Linus Walleij
  2018-10-12  8:54   ` Michal Simek
  1 sibling, 1 reply; 8+ messages in thread
From: Linus Walleij @ 2018-10-11  8:27 UTC (permalink / raw)
  To: wzab, Randy Dunlap, Masahiro Yamada, open list:GPIO SUBSYSTEM,
	Michal Simek
  Cc: linux-kernel

Hi Wojciech,

(Thanks also Randy for forwarding this!)

On Wed, Oct 10, 2018 at 6:32 PM wzab <wzab@ise.pw.edu.pl> wrote:

> The function of_get_named_gpiod_flags in older versions of the kernel
> (up to 4.7.10 - https://elixir.bootlin.com/linux/v4.7.10/source/drivers/gpio/gpiolib-of.c#L75 )
> contained an important workaround:
>
> /* .of_xlate might decide to not fill in the flags, so clear it. */if (flags)
>    *flags = 0; Unfortunately, newer kernels do not contain it. Therefore if the
> "xlat" function in the gpiochip driver does not set flags, (like e.g.
> the Xilinx AXI GPIO driver: https://github.com/Xilinx/linux-xlnx/blob/c2ba891326bb472da59b6a2da29aca218d337687/drivers/gpio/gpio-xilinx.c#L262 )
> the random, unitialized value from the stack in of_find_gpio
> ( https://elixir.bootlin.com/linux/v4.18.13/source/drivers/gpio/gpiolib-of.c#L228 )
> is used, which results in random settings of e.g., OPEN DRAIN or OPEN SOURCE mode.
>
> I have also reported the problem in the Xilinx forum:
> https://forums.xilinx.com/t5/Embedded-Linux/Bug-in-of-get-named-gpiod-flags-function-in-the-kernel-random/td-p/897695

It seems the commit removing this is:

commit 762c2e46c0591d207289105c8718e4adf29b2b34
"gpio: of: remove of_gpiochip_and_xlate() and struct gg_data"

But I honestly don't see a problem with it.

You are referencing an out-of-tree driver. Use the in-tree gpio-xilinx.c
that does not do any custom xlate and you will be fine.

I looked over the driver doing custom flag translation in the kernel tree,
and they all set flags, so this is not a problem in the upstream kernel.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Bug introduced in the of_get_named_gpiod_flags function.
  2018-10-11  8:27 ` Linus Walleij
@ 2018-10-12  8:54   ` Michal Simek
  2018-10-13 15:53     ` wzabolot
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Simek @ 2018-10-12  8:54 UTC (permalink / raw)
  To: Linus Walleij, wzab, Randy Dunlap, Masahiro Yamada,
	open list:GPIO SUBSYSTEM, Michal Simek
  Cc: linux-kernel

Hi,

On 11.10.2018 10:27, Linus Walleij wrote:
> Hi Wojciech,
> 
> (Thanks also Randy for forwarding this!)
> 
> On Wed, Oct 10, 2018 at 6:32 PM wzab <wzab@ise.pw.edu.pl> wrote:
> 
>> The function of_get_named_gpiod_flags in older versions of the kernel
>> (up to 4.7.10 - https://elixir.bootlin.com/linux/v4.7.10/source/drivers/gpio/gpiolib-of.c#L75 )
>> contained an important workaround:
>>
>> /* .of_xlate might decide to not fill in the flags, so clear it. */if (flags)
>>    *flags = 0; Unfortunately, newer kernels do not contain it. Therefore if the
>> "xlat" function in the gpiochip driver does not set flags, (like e.g.
>> the Xilinx AXI GPIO driver: https://github.com/Xilinx/linux-xlnx/blob/c2ba891326bb472da59b6a2da29aca218d337687/drivers/gpio/gpio-xilinx.c#L262 )
>> the random, unitialized value from the stack in of_find_gpio
>> ( https://elixir.bootlin.com/linux/v4.18.13/source/drivers/gpio/gpiolib-of.c#L228 )
>> is used, which results in random settings of e.g., OPEN DRAIN or OPEN SOURCE mode.
>>
>> I have also reported the problem in the Xilinx forum:
>> https://forums.xilinx.com/t5/Embedded-Linux/Bug-in-of-get-named-gpiod-flags-function-in-the-kernel-random/td-p/897695
> 
> It seems the commit removing this is:
> 
> commit 762c2e46c0591d207289105c8718e4adf29b2b34
> "gpio: of: remove of_gpiochip_and_xlate() and struct gg_data"
> 
> But I honestly don't see a problem with it.
> 
> You are referencing an out-of-tree driver. Use the in-tree gpio-xilinx.c
> that does not do any custom xlate and you will be fine.
> 
> I looked over the driver doing custom flag translation in the kernel tree,
> and they all set flags, so this is not a problem in the upstream kernel.

There was an attempt to sync up xilinx internal gpio driver with
mainline by someone else but not sure what's the status.
If you have issue with xilinx internal patch please talk to us.
If the problem is with mainline please use this mailing list.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Bug introduced in the of_get_named_gpiod_flags function.
  2018-10-12  8:54   ` Michal Simek
@ 2018-10-13 15:53     ` wzabolot
  2018-10-13 16:23       ` Linus Walleij
  0 siblings, 1 reply; 8+ messages in thread
From: wzabolot @ 2018-10-13 15:53 UTC (permalink / raw)
  To: Michal Simek, Linus Walleij, wzab, Randy Dunlap, Masahiro Yamada,
	open list:GPIO SUBSYSTEM
  Cc: linux-kernel

On 10/12/18 10:54 AM, Michal Simek wrote:
> Hi,
>
> On 11.10.2018 10:27, Linus Walleij wrote:
>> Hi Wojciech,
>>
>> (Thanks also Randy for forwarding this!)
>>
>> On Wed, Oct 10, 2018 at 6:32 PM wzab <wzab@ise.pw.edu.pl> wrote:
>>
>>> The function of_get_named_gpiod_flags in older versions of the kernel
>>> (up to 4.7.10 - https://elixir.bootlin.com/linux/v4.7.10/source/drivers/gpio/gpiolib-of.c#L75 )
>>> contained an important workaround:
>>>
>>> /* .of_xlate might decide to not fill in the flags, so clear it. */if (flags)
>>>    *flags = 0; Unfortunately, newer kernels do not contain it. Therefore if the
>>> "xlat" function in the gpiochip driver does not set flags, (like e.g.
>>> the Xilinx AXI GPIO driver: https://github.com/Xilinx/linux-xlnx/blob/c2ba891326bb472da59b6a2da29aca218d337687/drivers/gpio/gpio-xilinx.c#L262 )
>>> the random, unitialized value from the stack in of_find_gpio
>>> ( https://elixir.bootlin.com/linux/v4.18.13/source/drivers/gpio/gpiolib-of.c#L228 )
>>> is used, which results in random settings of e.g., OPEN DRAIN or OPEN SOURCE mode.
>>>
>>> I have also reported the problem in the Xilinx forum:
>>> https://forums.xilinx.com/t5/Embedded-Linux/Bug-in-of-get-named-gpiod-flags-function-in-the-kernel-random/td-p/897695
>> It seems the commit removing this is:
>>
>> commit 762c2e46c0591d207289105c8718e4adf29b2b34
>> "gpio: of: remove of_gpiochip_and_xlate() and struct gg_data"
>>
>> But I honestly don't see a problem with it.
>>
>> You are referencing an out-of-tree driver. Use the in-tree gpio-xilinx.c
>> that does not do any custom xlate and you will be fine.
>>
>> I looked over the driver doing custom flag translation in the kernel tree,
>> and they all set flags, so this is not a problem in the upstream kernel.
> There was an attempt to sync up xilinx internal gpio driver with
> mainline by someone else but not sure what's the status.
> If you have issue with xilinx internal patch please talk to us.
> If the problem is with mainline please use this mailing list.
>
> Thanks,
> Michal

The question is, if there may be any other in-tree GPIO controller
driver that does not initialize those flags?

Anyway the current situation is somehow dangerous.

Maybe the best solution would be to initialize the automatic variable in
the of_find_gpio?

With best regards,

Wojtek

-- 
Wojciech M Zabolotny, PhD
Institute of Electronic Systems
Faculty of Electronics and Information Technology
Warsaw University of Technology


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Bug introduced in the of_get_named_gpiod_flags function.
  2018-10-13 15:53     ` wzabolot
@ 2018-10-13 16:23       ` Linus Walleij
  2018-10-15 13:41         ` Wojciech Zabołotny
  0 siblings, 1 reply; 8+ messages in thread
From: Linus Walleij @ 2018-10-13 16:23 UTC (permalink / raw)
  To: wzabolot
  Cc: Michal Simek, wzab, Randy Dunlap, Masahiro Yamada,
	open list:GPIO SUBSYSTEM, linux-kernel

On Sat, Oct 13, 2018 at 5:53 PM wzabolot@elektron.elka.pw.edu.pl
<wzabolot@elektron.elka.pw.edu.pl> wrote:

> The question is, if there may be any other in-tree GPIO controller
> driver that does not initialize those flags?

So as I said, I looked over them and they all initialize
their flags.

> Anyway the current situation is somehow dangerous.

Not very dangerous compared to much other kernel code.

> Maybe the best solution would be to initialize the automatic variable in
> the of_find_gpio?

Patches welcom, make sure they are tested with mainline
code and drivers.

Yours.
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Bug introduced in the of_get_named_gpiod_flags function.
  2018-10-13 16:23       ` Linus Walleij
@ 2018-10-15 13:41         ` Wojciech Zabołotny
  0 siblings, 0 replies; 8+ messages in thread
From: Wojciech Zabołotny @ 2018-10-15 13:41 UTC (permalink / raw)
  To: Linus Walleij, wzabolot
  Cc: Michal Simek, Randy Dunlap, Masahiro Yamada,
	open list:GPIO SUBSYSTEM, linux-kernel

On 13.10.2018 18:23, Linus Walleij wrote:
> On Sat, Oct 13, 2018 at 5:53 PM wzabolot@elektron.elka.pw.edu.pl
> <wzabolot@elektron.elka.pw.edu.pl> wrote:
>
>> The question is, if there may be any other in-tree GPIO controller
>> driver that does not initialize those flags?
> So as I said, I looked over them and they all initialize
> their flags.
>
>> Anyway the current situation is somehow dangerous.
> Not very dangerous compared to much other kernel code.
>
>> Maybe the best solution would be to initialize the automatic variable in
>> the of_find_gpio?
> Patches welcom, make sure they are tested with mainline
> code and drivers.
>
> Yours.
> Linus Walleij

OK. So I suggested correction of the Xilinx GPIO driver so that it always sets the flags:

static int xgpio_xlate(struct gpio_chip *gc,
		       const struct of_phandle_args *gpiospec, u32 *flags)
{
	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
	struct xgpio_instance *chip = container_of(mm_gc, struct xgpio_instance,
						   mmchip);
        if(flags)
              * flags = 0;

	if (gpiospec->args[1] == chip->offset)
		return gpiospec->args[0];

	return -EINVAL;
}

With best regards,
Wojtek


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Bug introduced in the of_get_named_gpiod_flags function.
@ 2018-10-10 16:30 Wojciech Zabołotny
  0 siblings, 0 replies; 8+ messages in thread
From: Wojciech Zabołotny @ 2018-10-10 16:30 UTC (permalink / raw)
  To: LKML

Hi,

The function of_get_named_gpiod_flags in older versions of the kernel (up to 4.7.10 - https://elixir.bootlin.com/linux/v4.7.10/source/drivers/gpio/gpiolib-of.c#L75 )
contained an important workaround:

/* .of_xlate might decide to not fill in the flags, so clear it. */if (flags)
 *flags = 0;

Unfortunately, newer kernels do not contain it. Therefore if the
"xlat" function in the gpiochip driver does not set flags, (like e.g.
the Xilinx AXI GPIO driver: https://github.com/Xilinx/linux-xlnx/blob/c2ba891326bb472da59b6a2da29aca218d337687/drivers/gpio/gpio-xilinx.c#L262 )
the random, unitialized value from the stack in of_find_gpio ( https://elixir.bootlin.com/linux/v4.18.13/source/drivers/gpio/gpiolib-of.c#L228 )
is used, which results in random settings of e.g., OPEN DRAIN or OPEN SOURCE mode.
I suppose, that either the workaround should be restored, ot the of_flags variable in of_find_gpio should be initialized.

I have also reported the problem in the Xilinx forum: https://forums.xilinx.com/t5/Embedded-Linux/Bug-in-of-get-named-gpiod-flags-function-in-the-kernel-random/td-p/897695

With best regards,
Wojciech Zabolotny




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-10-15 14:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-10 16:13 Bug introduced in the of_get_named_gpiod_flags function wzab
2018-10-10 16:39 ` Randy Dunlap
2018-10-11  8:27 ` Linus Walleij
2018-10-12  8:54   ` Michal Simek
2018-10-13 15:53     ` wzabolot
2018-10-13 16:23       ` Linus Walleij
2018-10-15 13:41         ` Wojciech Zabołotny
2018-10-10 16:30 Wojciech Zabołotny

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).