linux-kernel - Re: [PATCH v2] gpio: return NULL from gpiod_get

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: <20170213074506.cf2zrjp2lmhtyoo5@pengutronix.de>
Date:   Mon, 13 Feb 2017 08:45:06 +0100
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Alexandre Courbot <gnurou@...il.com>,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] gpio: return NULL from gpiod_get_optional when
 GPIOLIB is disabled

Hello,

On Sun, Feb 12, 2017 at 05:15:01PM -0800, Dmitry Torokhov wrote:
> On Sun, Feb 12, 2017 at 05:13:55PM -0800, Dmitry Torokhov wrote:
> > Given the intent behind gpiod_get_optional() and friends it does not make
> > sense to return -ENOSYS when GPIOLIB is disabled: the driver is expected to
> > work just fine without gpio so let's behave as if gpio was not found.
> > Otherwise we have to special-case -ENOSYS in drivers.
> > 
> > Note that there was objection that someone might forget to enable GPIOLIB
> > when dealing with a platform that has device that actually specifies
> > optional gpio and we'll break it. I find this unconvincing as that would
> > have to be the *only GPIO* in the system, which is extremely unlikely.
> > 
> > Suggested-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>

I don't like this patch and so I wonder what I wrote that could be
interpreted as suggesting this patch. For now I'd say only

	Nacked-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>

is justified.

My concern is still there. This might break some setups. IMHO it's not
ok to request that a device in a certain configuration only works when
the (optional) Kconfig option GPIOLIB is enabled and silently breaks if
it's not. And you cannot rely on the person who configured the kernel.

When accepting this you will burn debug time of others who see their
device breaking with no or unrelated error messages.

The only reliable way out here is to enable enough of GPIOLIB to only
return NULL in ..._optional when there is no gpio required. You can have
a suggested-by for that.

The semantic of gpiod_get_optional is:

	if there is a gpio:
		give it to me
	else:
		give me a dummy

If the kernel is configured to be unable to answer the question "is
there a gpio?" that is worth a -ENOSYS.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |