linux-kernel - Re: [PATCH] gpio: sysfs: Try numbered exports if symbolic names fail

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-next>] [day] [month] [year] [list]

Date:   Thu, 10 Dec 2020 09:34:04 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     linux-gpio@...r.kernel.org,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Johan Hovold <johan@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] gpio: sysfs: Try numbered exports if symbolic names fail

On Wed, Dec 09, 2020 at 05:18:21PM +0100, Linus Walleij wrote:
> If a GPIO line cannot be exported using a symbolic name from
> the .names array in the gpiochip, fall back to using the
> "gpioN" naming system instead of just failing.
> 
> Cc: Manivannan Sadhasivam <mani@...nel.org>
> Cc: Johan Hovold <johan@...nel.org>
> Suggested-by: Johan Hovold <johan@...nel.org>
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> ---
>  drivers/gpio/gpiolib-sysfs.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index 728f6c687182..a5a0e9238217 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -627,10 +627,24 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
>  	if (chip->names && chip->names[offset])
>  		ioname = chip->names[offset];
>  
> -	dev = device_create_with_groups(&gpio_class, &gdev->dev,
> -					MKDEV(0, 0), data, gpio_groups,
> -					ioname ? ioname : "gpio%u",
> -					desc_to_gpio(desc));
> +	/*
> +	 * If we have a symbolic name for the GPIO we try to use that
> +	 * for the exported sysfs device/file, as legacy scripts depend
> +	 * on it. If we don't have a symbolic name or if there is a
> +	 * namespace collision, we stick with the "gpioN" name.
> +	 */
> +	dev = NULL;
> +	if (ioname)
> +		dev = device_create_with_groups(&gpio_class, &gdev->dev,
> +						MKDEV(0, 0), data, gpio_groups,
> +						ioname,
> +						desc_to_gpio(desc));
> +	if (IS_ERR_OR_NULL(dev))
> +		dev = device_create_with_groups(&gpio_class, &gdev->dev,
> +						MKDEV(0, 0), data, gpio_groups,
> +						"gpio%u",
> +						desc_to_gpio(desc));

I suggested having the driver set a flag which determines whether to use
the line names in sysfs or not.

The above will trigger a bunch of nasty warnings and backtraces in the
sysfs code (for every gpio line!), which is not something we want for
normal operation. Having the sysfs interface for the same USB device
depend on probe order is not very nice either.

Since the USB GPIO controller do not register any names today (as
gpiolib currently require a flat name space), there's no need to worry
about legacy scripts depending on those either (or rather, the argument
goes the other way since adding names now could break a functioning
script).

Just add a flag to suppress the renaming and we can safely start adding
names to hotpluggable controllers (if the rest of gpiolib can handle
non-unique names).

Johan