netdev - Re: [PATCH net-next] mdio_bus: Remove unneeded gpiod NULL check

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: <CACRpkdYw5GWv3HRCqnWjDfctLEfzcoFdJ7ib_NVKRgw0+Y9dEg@mail.gmail.com>
Date:   Wed, 2 Aug 2017 13:57:36 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Fabio Estevam <festevam@...il.com>,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Fabio Estevam <fabio.estevam@....com>
Subject: Re: [PATCH net-next] mdio_bus: Remove unneeded gpiod NULL check

On Tue, Jul 18, 2017 at 3:32 PM, Andrew Lunn <andrew@...n.ch> wrote:
> On Tue, Jul 18, 2017 at 09:52:51AM -0300, Fabio Estevam wrote:
>> On Tue, Jul 18, 2017 at 9:48 AM, Sergei Shtylyov
>> <sergei.shtylyov@...entembedded.com> wrote:
>> > On 07/18/2017 03:39 PM, Fabio Estevam wrote:
>> >
>> >>>    Won't this result in kernel WARNING when GPIO is disabled?
>> >
>> >
>> >    GPIO support, I was going to type...
>> >
>> >> Not sure if I understood your point, but gpiod_set_value_cansleep() is
>> >> a no-op when the gpiod is NULL.
>> >
>> >
>> >    Look at the stub in <linux/gpio/consumer.h>, it has WARN_ON(1).
>>
>> This patch does not alter the behavior of the driver with respect to
>> GPIO being disabled, so I still do not understand your concern.
>
> http://elixir.free-electrons.com/linux/latest/source/include/linux/gpio/consumer.h#L345
> static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
> {
>         /* GPIO can never have been requested */
>         WARN_ON(1);
> }
>
> But i would say this is a gpio problem. If GPIO enabled does not care,
> GPIO disabled should also not care.

If someone is using GPIO descriptors with GPIO disabled, i.e. calling
gpiod_get() and friends, this is very likely to be a bug, and what
the driver wants to do is:

 depends on GPIOLIB

or

 select GPIOLIB

in Kconfig. The whole optional thing is mainly a leftover from when it
was possible to have a local implementation of the GPIOLIB API in
some custom header file, noone sane should be doing that anymore,
and if they do, they can very well face the warnings.

If someone is facing a lot of WARN_ON() messages to this, it is a clear
indication that they need to fix their Kconfig and in that case it is proper.

Yours,
Linus Walleij