linux-kernel - [RFC PATCH] gpio: sysfs: make the sysfs export behavior consistent

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

Message-ID: <20240927074221.9985-1-brgl@bgdev.pl>
Date: Fri, 27 Sep 2024 09:42:21 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Linus Walleij <linus.walleij@...aro.org>,
	Kent Gibson <warthog618@...il.com>
Cc: linux-gpio@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: [RFC PATCH] gpio: sysfs: make the sysfs export behavior consistent

From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>

For drivers or board files that set gpio_chip->names, the links to the
GPIO attribute group created on sysfs export will be named after the
line's name set in that array. For lines that are named using device
properties, the names pointer of the gpio_chip struct is never assigned
so they are exported as if they're not named.

The ABI documentation does not mention the former behavior and given
that the majority of modern systems use device-tree, ACPI or other way
of passing GPIO names using device properties - bypassing gc->names -
it's better to make the behavior consistent by always exporting lines as
"gpioXYZ".

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
---
Story time:

I decided to learn rust. I figured I'd best find me a project to work on
that would involve some proper coding but wouldn't have much impact on
anything important when I inevitably get it wrong the first few times.

I decided to write a sysfs-to-libgpiod compatibility layer based on
FUSE. Since Rust is hard, I started prototyping the thing in python
first to at least have the logic nailed down before I tackle the rust
part.

When working on the exporting part, I vagely recalled that when I used
to work with GPIO sysfs somewhere between 2009 and 2012 (still with
board-file based systems), named lines exported with sysfs would appear
under /sys/class/gpio as symbolic links named like the line and not the
usual "gpioXYZ". I realized that this is not the case now.

Quick glance at the sysfs code reveals that I didn't dream it up, but
that behavior is reserved to drivers setting gc->names. This has been
slowly going out of fashion with device-tree and device properties.

We could easily restore the behavior for everybody by taking the name
from the descriptor we already have access to or even just assign
gc->names from descriptors in gpiolib core but first: the sysfs ABI
document does not mention the named links at all and second: given how
this has naturally effectively been phased out over the years, it would
probably cause more harm than good when the exported names suddenly
change for existing users.

I'm proposing to just drop the named links alogether.

Let me know what you think.

 drivers/gpio/gpiolib-sysfs.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 17ed229412af..643620d261f5 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -577,7 +577,7 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 	struct gpio_device *gdev;
 	struct gpiod_data *data;
 	struct device *dev;
-	int status, offset;
+	int status;

 	/* can't export until sysfs is available ... */
 	if (!class_is_registered(&gpio_class)) {
@@ -626,10 +626,6 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 	else
 		data->direction_can_change = false;

-	offset = gpio_chip_hwgpio(desc);
-	if (guard.gc->names && guard.gc->names[offset])
-		ioname = guard.gc->names[offset];
-
 	dev = device_create_with_groups(&gpio_class, &gdev->dev,
 					MKDEV(0, 0), data, gpio_groups,
 					ioname ? ioname : "gpio%u",
-- 
2.43.0