[PATCH v2 00/11] gpio: Adopt revocable mechanism for UAF prevention

[PATCH v2 00/11] gpio: Adopt revocable mechanism for UAF prevention

[Tzung-Bi Shih]

This series transitions the UAF prevention logic within the GPIO core
(gpiolib) to use the 'revocable' mechanism.

The existing code aims to prevent UAF issues when the underlying GPIO
chip is removed.  This series replaces that custom logic with the
generic 'revocable' API, which is designed to handle such lifecycle
dependencies.  There should be no changes in behavior.

The series applies after:
- https://lore.kernel.org/all/20260129143733.45618-1-tzungbi@kernel.org
- https://lore.kernel.org/all/20260203060210.972243-1-tzungbi@kernel.org

Bartosz: the series was planned to send after -rc1 comes.  But I think
it'd be great to send out for your early review and testing if possible.
The series base on v6.19-rc8, driver-core-next, and gpio/for-next.
Please use the temporary integration testing branch
https://git.kernel.org/pub/scm/linux/kernel/git/tzungbi/chrome-platform.git/log/?h=gpio_rev
if you'd like to.

Tzung-Bi Shih (11):
  gpio: Access `gpio_bus_type` in gpiochip_setup_dev()
  gpio: Remove redundant check for struct gpio_chip
  gpio: sysfs: Remove redundant check for struct gpio_chip
  gpio: Ensure struct gpio_chip for gpiochip_setup_dev()
  gpio: cdev: Don't check struct gpio_chip in gpio_chrdev_open()
=> The first 5 patches are refactors.  They try to make the subsequent
   changes easier or at least clear.

  selftests: gpio: Add gpio-cdev-uaf tests
=> The following patch adds kselftest cases for some classic UAF
   scenarios.

  gpio: Add revocable provider handle for struct gpio_chip
  gpio: cdev: Leverage revocable for accessing struct gpio_chip
  gpio: Remove gpio_chip_guard by using revocable
  gpio: Leverage revocable for accessing struct gpio_chip
=> The following 4 patches start to replace the existing code.

  gpio: Remove unused `chip` and `srcu` in struct gpio_device
=> The last patch removes the unused fields for the custom logic as all
   of them should be transiting to revocable.

---
v2:
- Separate fixes patches from v1.  Some of them have been landed.
- Combine small patches into one as they become simpler after applying
  https://lore.kernel.org/all/20260129143733.45618-1-tzungbi@kernel.org.

v1: https://lore.kernel.org/all/20260116081036.352286-1-tzungbi@kernel.org

 drivers/gpio/gpiolib-cdev.c                   |  97 +++---
 drivers/gpio/gpiolib-cdev.h                   |   3 +-
 drivers/gpio/gpiolib-sysfs.c                  |  50 ++-
 drivers/gpio/gpiolib-sysfs.h                  |  11 +-
 drivers/gpio/gpiolib.c                        | 303 +++++++++---------
 drivers/gpio/gpiolib.h                        |  27 +-
 tools/testing/selftests/gpio/Makefile         |   5 +-
 tools/testing/selftests/gpio/gpio-cdev-uaf.c  | 292 +++++++++++++++++
 tools/testing/selftests/gpio/gpio-cdev-uaf.sh |  63 ++++
 9 files changed, 575 insertions(+), 276 deletions(-)
 create mode 100644 tools/testing/selftests/gpio/gpio-cdev-uaf.c
 create mode 100755 tools/testing/selftests/gpio/gpio-cdev-uaf.sh

-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 01/11] gpio: Access `gpio_bus_type` in gpiochip_setup_dev()

[Tzung-Bi Shih]

To make the intent clear, access `gpio_bus_type` only when it's ready in
gpiochip_setup_dev().

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-7-tzungbi@kernel.org

 drivers/gpio/gpiolib.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 039cd3e56baf..f51f53511ae3 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -902,6 +902,8 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
 	struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
 	int ret;
 
+	gdev->dev.bus = &gpio_bus_type;
+
 	/*
 	 * If fwnode doesn't belong to another device, it's safe to clear its
 	 * initialized flag.
@@ -1078,7 +1080,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 
 	device_initialize(&gdev->dev);
 	gdev->dev.type = &gpio_dev_type;
-	gdev->dev.bus = &gpio_bus_type;
 	gdev->dev.parent = gc->parent;
 	device_set_node(&gdev->dev, gpiochip_choose_fwnode(gc));
 
@@ -1216,8 +1217,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	 * we get a device node entry in sysfs under
 	 * /sys/bus/gpio/devices/gpiochipN/dev that can be used for
 	 * coldplug of device nodes and other udev business.
-	 * We can do this only if gpiolib has been initialized.
-	 * Otherwise, defer until later.
+	 * We can do this only if gpiolib has been initialized
+	 * (i.e., `gpio_bus_type` is ready).  Otherwise, defer until later.
 	 */
 	if (gpiolib_initialized) {
 		ret = gpiochip_setup_dev(gdev);
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 02/11] gpio: Remove redundant check for struct gpio_chip

[Tzung-Bi Shih]

gpiolib_dbg_show() is only called by gpiolib_seq_show() which has
ensured the struct gpio_chip.  Remove the redundant check in
gpiolib_dbg_show().

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-8-tzungbi@kernel.org

 drivers/gpio/gpiolib.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f51f53511ae3..a6dd07be126c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -5308,23 +5308,14 @@ core_initcall(gpiolib_dev_init);
 
 #ifdef CONFIG_DEBUG_FS
 
-static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
+static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 {
 	bool active_low, is_irq, is_out;
 	struct gpio_desc *desc;
 	unsigned int gpio = 0;
-	struct gpio_chip *gc;
 	unsigned long flags;
 	int value;
 
-	guard(srcu)(&gdev->srcu);
-
-	gc = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!gc) {
-		seq_puts(s, "Underlying GPIO chip is gone\n");
-		return;
-	}
-
 	for_each_gpio_desc(gc, desc) {
 		guard(srcu)(&desc->gdev->desc_srcu);
 		flags = READ_ONCE(desc->flags);
@@ -5437,7 +5428,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
 	if (gc->dbg_show)
 		gc->dbg_show(s, gc);
 	else
-		gpiolib_dbg_show(s, gdev);
+		gpiolib_dbg_show(s, gc);
 
 	return 0;
 }
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 03/11] gpio: sysfs: Remove redundant check for struct gpio_chip

[Tzung-Bi Shih]

gpiochip_sysfs_unregister() is only called by gpiochip_remove() where
the struct gpio_chip is ensured.

Remove the redundant check.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-9-tzungbi@kernel.org

 drivers/gpio/gpiolib-sysfs.c | 9 +--------
 drivers/gpio/gpiolib-sysfs.h | 6 ++++--
 drivers/gpio/gpiolib.c       | 2 +-
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index cd553acf3055..8e6b09d8b559 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -1048,11 +1048,10 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
 	return 0;
 }
 
-void gpiochip_sysfs_unregister(struct gpio_device *gdev)
+void gpiochip_sysfs_unregister(struct gpio_device *gdev, struct gpio_chip *chip)
 {
 	struct gpiodev_data *data;
 	struct gpio_desc *desc;
-	struct gpio_chip *chip;
 
 	scoped_guard(mutex, &sysfs_lock) {
 		data = gdev_get_data(gdev);
@@ -1066,12 +1065,6 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 		kfree(data);
 	}
 
-	guard(srcu)(&gdev->srcu);
-
-	chip = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!chip)
-		return;
-
 	/* unregister gpiod class devices owned by sysfs */
 	for_each_gpio_desc_with_flag(chip, desc, GPIOD_FLAG_SYSFS) {
 		gpiod_unexport(desc);
diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
index b794b396d6a5..93debe8e118c 100644
--- a/drivers/gpio/gpiolib-sysfs.h
+++ b/drivers/gpio/gpiolib-sysfs.h
@@ -8,7 +8,8 @@ struct gpio_device;
 #ifdef CONFIG_GPIO_SYSFS
 
 int gpiochip_sysfs_register(struct gpio_device *gdev);
-void gpiochip_sysfs_unregister(struct gpio_device *gdev);
+void gpiochip_sysfs_unregister(struct gpio_device *gdev,
+			       struct gpio_chip *chip);
 
 #else
 
@@ -17,7 +18,8 @@ static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
 	return 0;
 }
 
-static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
+static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev,
+					     struct gpio_chip *chip)
 {
 }
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a6dd07be126c..3137e6f1108a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1281,7 +1281,7 @@ void gpiochip_remove(struct gpio_chip *gc)
 	struct gpio_device *gdev = gc->gpiodev;
 
 	/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
-	gpiochip_sysfs_unregister(gdev);
+	gpiochip_sysfs_unregister(gdev, gc);
 	gpiochip_free_hogs(gc);
 	gpiochip_free_remaining_irqs(gc);
 
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 04/11] gpio: Ensure struct gpio_chip for gpiochip_setup_dev()

[Tzung-Bi Shih]

Ensure struct gpio_chip for gpiochip_setup_dev().  This eliminates a few
checks for struct gpio_chip.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-10-tzungbi@kernel.org

 drivers/gpio/gpiolib-cdev.c  | 12 ++----------
 drivers/gpio/gpiolib-cdev.h  |  3 ++-
 drivers/gpio/gpiolib-sysfs.c | 11 ++---------
 drivers/gpio/gpiolib-sysfs.h |  5 +++--
 drivers/gpio/gpiolib.c       | 24 +++++++++++++++++-------
 5 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 2adc3c070908..b89201578516 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2782,9 +2782,9 @@ static const struct file_operations gpio_fileops = {
 #endif
 };
 
-int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
+int gpiolib_cdev_register(struct gpio_device *gdev, struct gpio_chip *gc,
+			  dev_t devt)
 {
-	struct gpio_chip *gc;
 	int ret;
 
 	cdev_init(&gdev->chrdev, &gpio_fileops);
@@ -2802,14 +2802,6 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
 		return ret;
 	}
 
-	guard(srcu)(&gdev->srcu);
-	gc = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!gc) {
-		cdev_device_del(&gdev->chrdev, &gdev->dev);
-		destroy_workqueue(gdev->line_state_wq);
-		return -ENODEV;
-	}
-
 	gpiochip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
 
 	return 0;
diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
index b42644cbffb8..16ef1e2e96a0 100644
--- a/drivers/gpio/gpiolib-cdev.h
+++ b/drivers/gpio/gpiolib-cdev.h
@@ -7,7 +7,8 @@
 
 struct gpio_device;
 
-int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
+int gpiolib_cdev_register(struct gpio_device *gdev, struct gpio_chip *gc,
+			  dev_t devt);
 void gpiolib_cdev_unregister(struct gpio_device *gdev);
 
 #endif /* GPIOLIB_CDEV_H */
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 8e6b09d8b559..a4427a5cfa85 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -978,10 +978,9 @@ void gpiod_unexport(struct gpio_desc *desc)
 }
 EXPORT_SYMBOL_GPL(gpiod_unexport);
 
-int gpiochip_sysfs_register(struct gpio_device *gdev)
+int gpiochip_sysfs_register(struct gpio_device *gdev, struct gpio_chip *chip)
 {
 	struct gpiodev_data *data;
-	struct gpio_chip *chip;
 	struct device *parent;
 	int err;
 
@@ -994,12 +993,6 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
 	if (!class_is_registered(&gpio_class))
 		return 0;
 
-	guard(srcu)(&gdev->srcu);
-
-	chip = srcu_dereference(gdev->chip, &gdev->srcu);
-	if (!chip)
-		return -ENODEV;
-
 	/*
 	 * For sysfs backward compatibility we need to preserve this
 	 * preferred parenting to the gpio_chip parent field, if set.
@@ -1082,7 +1075,7 @@ static int gpiofind_sysfs_register(struct gpio_chip *gc, const void *data)
 	struct gpio_device *gdev = gc->gpiodev;
 	int ret;
 
-	ret = gpiochip_sysfs_register(gdev);
+	ret = gpiochip_sysfs_register(gdev, gc);
 	if (ret)
 		gpiochip_err(gc, "failed to register the sysfs entry: %d\n", ret);
 
diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
index 93debe8e118c..192b1ee041a6 100644
--- a/drivers/gpio/gpiolib-sysfs.h
+++ b/drivers/gpio/gpiolib-sysfs.h
@@ -7,13 +7,14 @@ struct gpio_device;
 
 #ifdef CONFIG_GPIO_SYSFS
 
-int gpiochip_sysfs_register(struct gpio_device *gdev);
+int gpiochip_sysfs_register(struct gpio_device *gdev, struct gpio_chip *chip);
 void gpiochip_sysfs_unregister(struct gpio_device *gdev,
 			       struct gpio_chip *chip);
 
 #else
 
-static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
+static inline int gpiochip_sysfs_register(struct gpio_device *gdev,
+					  struct gpio_chip *chip)
 {
 	return 0;
 }
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3137e6f1108a..7885dcd1e49d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -882,14 +882,15 @@ static const struct device_type gpio_dev_type = {
 };
 
 #ifdef CONFIG_GPIO_CDEV
-#define gcdev_register(gdev, devt)	gpiolib_cdev_register((gdev), (devt))
+#define gcdev_register(gdev, gc, devt)	\
+		gpiolib_cdev_register((gdev), (gc), (devt))
 #define gcdev_unregister(gdev)		gpiolib_cdev_unregister((gdev))
 #else
 /*
  * gpiolib_cdev_register() indirectly calls device_add(), which is still
  * required even when cdev is not selected.
  */
-#define gcdev_register(gdev, devt)	device_add(&(gdev)->dev)
+#define gcdev_register(gdev, gc, devt)	device_add(&(gdev)->dev)
 #define gcdev_unregister(gdev)		device_del(&(gdev)->dev)
 #endif
 
@@ -897,7 +898,7 @@ static const struct device_type gpio_dev_type = {
  * An initial reference count has been held in gpiochip_add_data_with_key().
  * The caller should drop the reference via gpio_device_put() on errors.
  */
-static int gpiochip_setup_dev(struct gpio_device *gdev)
+static int gpiochip_setup_dev(struct gpio_device *gdev, struct gpio_chip *gc)
 {
 	struct fwnode_handle *fwnode = dev_fwnode(&gdev->dev);
 	int ret;
@@ -911,11 +912,11 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
 	if (fwnode && !fwnode->dev)
 		fwnode_dev_initialized(fwnode, false);
 
-	ret = gcdev_register(gdev, gpio_devt);
+	ret = gcdev_register(gdev, gc, gpio_devt);
 	if (ret)
 		return ret;
 
-	ret = gpiochip_sysfs_register(gdev);
+	ret = gpiochip_sysfs_register(gdev, gc);
 	if (ret)
 		goto err_remove_device;
 
@@ -962,13 +963,22 @@ static void machine_gpiochip_add(struct gpio_chip *gc)
 static void gpiochip_setup_devs(void)
 {
 	struct gpio_device *gdev;
+	struct gpio_chip *gc;
 	int ret;
 
 	guard(srcu)(&gpio_devices_srcu);
 
 	list_for_each_entry_srcu(gdev, &gpio_devices, list,
 				 srcu_read_lock_held(&gpio_devices_srcu)) {
-		ret = gpiochip_setup_dev(gdev);
+		guard(srcu)(&gdev->srcu);
+
+		gc = srcu_dereference(gdev->chip, &gdev->srcu);
+		if (!gc) {
+			dev_err(&gdev->dev, "Underlying GPIO chip is gone\n");
+			continue;
+		}
+
+		ret = gpiochip_setup_dev(gdev, gc);
 		if (ret) {
 			gpio_device_put(gdev);
 			dev_err(&gdev->dev,
@@ -1221,7 +1231,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	 * (i.e., `gpio_bus_type` is ready).  Otherwise, defer until later.
 	 */
 	if (gpiolib_initialized) {
-		ret = gpiochip_setup_dev(gdev);
+		ret = gpiochip_setup_dev(gdev, gc);
 		if (ret)
 			goto err_teardown_shared;
 	}
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 05/11] gpio: cdev: Don't check struct gpio_chip in gpio_chrdev_open()

[Tzung-Bi Shih]

It's harmless even if: chrdev_open() and cdev_device_del() run at the
same time, and gpio_chrdev_open() gets called after the underlying GPIO
chip has gone.  The subsequent file operations check the availability
of struct gpio_chip anyway.

Don't check struct gpio_chip in gpio_chrdev_open().

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-11-tzungbi@kernel.org

 drivers/gpio/gpiolib-cdev.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index b89201578516..aaa5de814468 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2689,12 +2689,6 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	struct gpio_chardev_data *cdev;
 	int ret = -ENOMEM;
 
-	guard(srcu)(&gdev->srcu);
-
-	/* Fail on open if the backing gpiochip is gone */
-	if (!rcu_access_pointer(gdev->chip))
-		return -ENODEV;
-
 	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
 	if (!cdev)
 		return -ENOMEM;
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 06/11] selftests: gpio: Add gpio-cdev-uaf tests

[Tzung-Bi Shih]

Add tests for gpiolib-cdev to make sure accessing to dangling resources
via the opening file descriptor won't crash the system after the
underlying resource providers have gone.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- Remove fdinfo test which is redundant.

v1: https://lore.kernel.org/all/20260116081036.352286-12-tzungbi@kernel.org

 tools/testing/selftests/gpio/Makefile         |   5 +-
 tools/testing/selftests/gpio/gpio-cdev-uaf.c  | 292 ++++++++++++++++++
 tools/testing/selftests/gpio/gpio-cdev-uaf.sh |  63 ++++
 3 files changed, 358 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/gpio/gpio-cdev-uaf.c
 create mode 100755 tools/testing/selftests/gpio/gpio-cdev-uaf.sh

diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index 7bfe315f7001..741ab21e1260 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,8 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0
 
-TEST_PROGS := gpio-mockup.sh gpio-sim.sh gpio-aggregator.sh
+TEST_PROGS := gpio-mockup.sh gpio-sim.sh gpio-aggregator.sh gpio-cdev-uaf.sh
 TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name \
+			   gpio-cdev-uaf
 CFLAGS += -O2 -g -Wall $(KHDR_INCLUDES)
 
 include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-cdev-uaf.c b/tools/testing/selftests/gpio/gpio-cdev-uaf.c
new file mode 100644
index 000000000000..765d3cc4f0ef
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-cdev-uaf.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for UAF tests.
+ *
+ * Copyright 2026 Google LLC
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/gpio.h>
+#include <poll.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#define CONFIGFS_DIR "/sys/kernel/config/gpio-sim"
+#define PROCFS_DIR "/proc"
+
+static void print_usage(void)
+{
+	printf("usage:\n");
+	printf("  gpio-cdev-uaf [chip|handle|event|req] [poll|read|ioctl]\n");
+}
+
+static int _create_chip(const char *name, int create)
+{
+	char path[64];
+
+	snprintf(path, sizeof(path), CONFIGFS_DIR "/%s", name);
+
+	if (create)
+		return mkdir(path, 0755);
+	else
+		return rmdir(path);
+}
+
+static int create_chip(const char *name)
+{
+	return _create_chip(name, 1);
+}
+
+static void remove_chip(const char *name)
+{
+	_create_chip(name, 0);
+}
+
+static int _create_bank(const char *chip_name, const char *name, int create)
+{
+	char path[64];
+
+	snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/%s", chip_name, name);
+
+	if (create)
+		return mkdir(path, 0755);
+	else
+		return rmdir(path);
+}
+
+static int create_bank(const char *chip_name, const char *name)
+{
+	return _create_bank(chip_name, name, 1);
+}
+
+static void remove_bank(const char *chip_name, const char *name)
+{
+	_create_bank(chip_name, name, 0);
+}
+
+static int _enable_chip(const char *name, int enable)
+{
+	char path[64];
+	int fd, ret;
+
+	snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/live", name);
+
+	fd = open(path, O_WRONLY);
+	if (fd == -1)
+		return fd;
+
+	if (enable)
+		ret = write(fd, "1", 1);
+	else
+		ret = write(fd, "0", 1);
+
+	close(fd);
+	return ret == 1 ? 0 : -1;
+}
+
+static int enable_chip(const char *name)
+{
+	return _enable_chip(name, 1);
+}
+
+static void disable_chip(const char *name)
+{
+	_enable_chip(name, 0);
+}
+
+static int open_chip(const char *chip_name, const char *bank_name)
+{
+	char path[64], dev_name[32];
+	int ret, fd;
+
+	ret = create_chip(chip_name);
+	if (ret) {
+		fprintf(stderr, "failed to create chip\n");
+		return ret;
+	}
+
+	ret = create_bank(chip_name, bank_name);
+	if (ret) {
+		fprintf(stderr, "failed to create bank\n");
+		goto err_remove_chip;
+	}
+
+	ret = enable_chip(chip_name);
+	if (ret) {
+		fprintf(stderr, "failed to enable chip\n");
+		goto err_remove_bank;
+	}
+
+	snprintf(path, sizeof(path), CONFIGFS_DIR "/%s/%s/chip_name",
+		 chip_name, bank_name);
+
+	fd = open(path, O_RDONLY);
+	if (fd == -1) {
+		ret = fd;
+		fprintf(stderr, "failed to open %s\n", path);
+		goto err_disable_chip;
+	}
+
+	ret = read(fd, dev_name, sizeof(dev_name) - 1);
+	close(fd);
+	if (ret == -1) {
+		fprintf(stderr, "failed to read %s\n", path);
+		goto err_disable_chip;
+	}
+	dev_name[ret] = '\0';
+	if (ret && dev_name[ret - 1] == '\n')
+		dev_name[ret - 1] = '\0';
+
+	snprintf(path, sizeof(path), "/dev/%s", dev_name);
+
+	fd = open(path, O_RDWR);
+	if (fd == -1) {
+		ret = fd;
+		fprintf(stderr, "failed to open %s\n", path);
+		goto err_disable_chip;
+	}
+
+	return fd;
+err_disable_chip:
+	disable_chip(chip_name);
+err_remove_bank:
+	remove_bank(chip_name, bank_name);
+err_remove_chip:
+	remove_chip(chip_name);
+	return ret;
+}
+
+static void close_chip(const char *chip_name, const char *bank_name)
+{
+	disable_chip(chip_name);
+	remove_bank(chip_name, bank_name);
+	remove_chip(chip_name);
+}
+
+static int test_poll(int fd)
+{
+	struct pollfd pfds;
+
+	pfds.fd = fd;
+	pfds.events = POLLIN;
+	pfds.revents = 0;
+
+	if (poll(&pfds, 1, 0) == -1)
+		return -1;
+
+	return (pfds.revents & ~(POLLHUP | POLLERR)) ? -1 : 0;
+}
+
+static int test_read(int fd)
+{
+	char data;
+
+	if (read(fd, &data, 1) == -1 && errno == ENODEV)
+		return 0;
+	return -1;
+}
+
+static int test_ioctl(int fd)
+{
+	if (ioctl(fd, 0, NULL) == -1 && errno == ENODEV)
+		return 0;
+	return -1;
+}
+
+int main(int argc, char **argv)
+{
+	int cfd, fd, ret;
+	int (*test_func)(int);
+
+	if (argc != 3) {
+		print_usage();
+		return EXIT_FAILURE;
+	}
+
+	if (strcmp(argv[1], "chip") == 0 ||
+	    strcmp(argv[1], "event") == 0 ||
+	    strcmp(argv[1], "req") == 0) {
+		if (strcmp(argv[2], "poll") &&
+		    strcmp(argv[2], "read") &&
+		    strcmp(argv[2], "ioctl")) {
+			fprintf(stderr, "unknown command: %s\n", argv[2]);
+			return EXIT_FAILURE;
+		}
+	} else if (strcmp(argv[1], "handle") == 0) {
+		if (strcmp(argv[2], "ioctl")) {
+			fprintf(stderr, "unknown command: %s\n", argv[2]);
+			return EXIT_FAILURE;
+		}
+	} else {
+		fprintf(stderr, "unknown command: %s\n", argv[1]);
+		return EXIT_FAILURE;
+	}
+
+	if (strcmp(argv[2], "poll") == 0)
+		test_func = test_poll;
+	else if (strcmp(argv[2], "read") == 0)
+		test_func = test_read;
+	else	/* strcmp(argv[2], "ioctl") == 0 */
+		test_func = test_ioctl;
+
+	cfd = open_chip("chip", "bank");
+	if (cfd == -1) {
+		fprintf(stderr, "failed to open chip\n");
+		return EXIT_FAILURE;
+	}
+
+	/* Step 1: Hold a FD to the test target. */
+	if (strcmp(argv[1], "chip") == 0) {
+		fd = cfd;
+	} else if (strcmp(argv[1], "handle") == 0) {
+		struct gpiohandle_request req = {0};
+
+		req.lines = 1;
+		if (ioctl(cfd, GPIO_GET_LINEHANDLE_IOCTL, &req) == -1) {
+			fprintf(stderr, "failed to get handle FD\n");
+			goto err_close_chip;
+		}
+
+		close(cfd);
+		fd = req.fd;
+	} else if (strcmp(argv[1], "event") == 0) {
+		struct gpioevent_request req = {0};
+
+		if (ioctl(cfd, GPIO_GET_LINEEVENT_IOCTL, &req) == -1) {
+			fprintf(stderr, "failed to get event FD\n");
+			goto err_close_chip;
+		}
+
+		close(cfd);
+		fd = req.fd;
+	} else {	/* strcmp(argv[1], "req") == 0 */
+		struct gpio_v2_line_request req = {0};
+
+		req.num_lines = 1;
+		if (ioctl(cfd, GPIO_V2_GET_LINE_IOCTL, &req) == -1) {
+			fprintf(stderr, "failed to get req FD\n");
+			goto err_close_chip;
+		}
+
+		close(cfd);
+		fd = req.fd;
+	}
+
+	/* Step 2: Free the chip. */
+	close_chip("chip", "bank");
+
+	/* Step 3: Access the dangling FD to trigger UAF. */
+	ret = test_func(fd);
+	close(fd);
+	return ret ? EXIT_FAILURE : EXIT_SUCCESS;
+err_close_chip:
+	close(cfd);
+	close_chip("chip", "bank");
+	return EXIT_FAILURE;
+}
diff --git a/tools/testing/selftests/gpio/gpio-cdev-uaf.sh b/tools/testing/selftests/gpio/gpio-cdev-uaf.sh
new file mode 100755
index 000000000000..6e47533019cf
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-cdev-uaf.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2026 Google LLC
+
+BASE_DIR=`dirname $0`
+MODULE="gpio-cdev-uaf"
+
+fail() {
+	echo "$*" >&2
+	echo "GPIO $MODULE test FAIL"
+	exit 1
+}
+
+skip() {
+	echo "$*" >&2
+	echo "GPIO $MODULE test SKIP"
+	exit 4
+}
+
+# Load the gpio-sim module. This will pull in configfs if needed too.
+modprobe gpio-sim || skip "unable to load the gpio-sim module"
+# Make sure configfs is mounted at /sys/kernel/config. Wait a bit if needed.
+for _ in `seq 5`; do
+	mountpoint -q /sys/kernel/config && break
+	mount -t configfs none /sys/kernel/config
+	sleep 0.1
+done
+mountpoint -q /sys/kernel/config || \
+	skip "configfs not mounted at /sys/kernel/config"
+
+echo "1. GPIO"
+
+echo "1.1. poll"
+$BASE_DIR/gpio-cdev-uaf chip poll || fail "failed to test chip poll"
+echo "1.2. read"
+$BASE_DIR/gpio-cdev-uaf chip read || fail "failed to test chip read"
+echo "1.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf chip ioctl || fail "failed to test chip ioctl"
+
+echo "2. linehandle"
+
+echo "2.1. ioctl"
+$BASE_DIR/gpio-cdev-uaf handle ioctl || fail "failed to test handle ioctl"
+
+echo "3. lineevent"
+
+echo "3.1. read"
+$BASE_DIR/gpio-cdev-uaf event read || fail "failed to test event read"
+echo "3.2. poll"
+$BASE_DIR/gpio-cdev-uaf event poll || fail "failed to test event poll"
+echo "3.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf event ioctl || fail "failed to test event ioctl"
+
+echo "4. linereq"
+
+echo "4.1. read"
+$BASE_DIR/gpio-cdev-uaf req read || fail "failed to test req read"
+echo "4.2. poll"
+$BASE_DIR/gpio-cdev-uaf req poll || fail "failed to test req poll"
+echo "4.3. ioctl"
+$BASE_DIR/gpio-cdev-uaf req ioctl || fail "failed to test req ioctl"
+
+echo "GPIO $MODULE test PASS"
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 07/11] gpio: Add revocable provider handle for struct gpio_chip

[Tzung-Bi Shih]

The underlying chip can be removed asynchronously.  `gdev->srcu` is used
to ensure the synchronization before accessing `gdev->chip`.

Revocable encapsulates the details.  Add revocable provider handle for
the corresponding struct gpio_chip in struct gpio_device so that it can
start to hide the synchronization details.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- Change usages accordingly after applying
  https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org.
  - Add __rcu for `chip_rp`.
  - Pass pointer of pointer to revocable_provider_revoke().
- Rebase accordingly after applying
  https://lore.kernel.org/all/20260203060210.972243-1-tzungbi@kernel.org.

v1: https://lore.kernel.org/all/20260116081036.352286-13-tzungbi@kernel.org

 drivers/gpio/gpiolib.c | 20 ++++++++++++++++++--
 drivers/gpio/gpiolib.h |  2 ++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 7885dcd1e49d..fdae10ec3a17 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -22,6 +22,7 @@
 #include <linux/nospec.h>
 #include <linux/of.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/revocable.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/srcu.h>
@@ -1110,6 +1111,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 		goto err_put_device;
 	}
 
+	gdev->chip_rp = revocable_provider_alloc(gc);
+	if (!gdev->chip_rp) {
+		ret = -ENOMEM;
+		goto err_put_device;
+	}
+
 	gdev->can_sleep = gc->can_sleep;
 	rwlock_init(&gdev->line_state_lock);
 	RAW_INIT_NOTIFIER_HEAD(&gdev->line_state_notifier);
@@ -1139,7 +1146,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 			if (base < 0) {
 				ret = base;
 				base = 0;
-				goto err_put_device;
+				goto err_free_rp;
 			}
 
 			/*
@@ -1159,7 +1166,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 		ret = gpiodev_add_to_list_unlocked(gdev);
 		if (ret) {
 			gpiochip_err(gc, "GPIO integer space overlap, cannot add chip\n");
-			goto err_put_device;
+			goto err_free_rp;
 		}
 	}
 
@@ -1256,6 +1263,14 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 	scoped_guard(mutex, &gpio_devices_lock)
 		list_del_rcu(&gdev->list);
 	synchronize_srcu(&gpio_devices_srcu);
+err_free_rp:
+	/*
+	 * Unlike other allocated resources for `gdev` can be freed
+	 * in gpiodev_release().  Call revocable_provider_revoke()
+	 * here as it's designed to be called when the chip is gone
+	 * (i.e., gpiochip_remove()).
+	 */
+	revocable_provider_revoke(&gdev->chip_rp);
 err_put_device:
 	gpio_device_put(gdev);
 	goto err_print_message;
@@ -1302,6 +1317,7 @@ void gpiochip_remove(struct gpio_chip *gc)
 	/* Numb the device, cancelling all outstanding operations */
 	rcu_assign_pointer(gdev->chip, NULL);
 	synchronize_srcu(&gdev->srcu);
+	revocable_provider_revoke(&gdev->chip_rp);
 	gpio_device_teardown_shared(gdev);
 	gpiochip_irqchip_remove(gc);
 	acpi_gpiochip_remove(gc);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 3abb90385829..cd136d5b52e9 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -52,6 +52,7 @@
  * @device_notifier: used to notify character device wait queues about the GPIO
  *                   device being unregistered
  * @srcu: protects the pointer to the underlying GPIO chip
+ * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
  * @pin_ranges: range of pins served by the GPIO driver
  *
  * This state container holds most of the runtime variable data
@@ -79,6 +80,7 @@ struct gpio_device {
 	struct workqueue_struct	*line_state_wq;
 	struct blocking_notifier_head device_notifier;
 	struct srcu_struct	srcu;
+	struct revocable_provider __rcu *chip_rp;
 
 #ifdef CONFIG_PINCTRL
 	/*
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 08/11] gpio: cdev: Leverage revocable for accessing struct gpio_chip

[Tzung-Bi Shih]

Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip.  Leverage revocable for accessing the struct
gpio_chip.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- Change usages accordingly after applying
  https://lore.kernel.org/all/20260129143733.45618-4-tzungbi@kernel.org.
  - Preserve a local storage for `struct revocable`.
- Combine multiple patches (see "v1:").

v1:
- https://lore.kernel.org/all/20260116081036.352286-14-tzungbi@kernel.org
- https://lore.kernel.org/all/20260116081036.352286-15-tzungbi@kernel.org
- https://lore.kernel.org/all/20260116081036.352286-16-tzungbi@kernel.org
- https://lore.kernel.org/all/20260116081036.352286-17-tzungbi@kernel.org
- https://lore.kernel.org/all/20260116081036.352286-18-tzungbi@kernel.org

 drivers/gpio/gpiolib-cdev.c | 70 ++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 36 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index aaa5de814468..ca9c04765df4 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -22,6 +22,7 @@
 #include <linux/overflow.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/poll.h>
+#include <linux/revocable.h>
 #include <linux/seq_file.h>
 #include <linux/spinlock.h>
 #include <linux/string.h>
@@ -210,10 +211,10 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
 	DECLARE_BITMAP(vals, GPIOHANDLES_MAX);
 	unsigned int i;
 	int ret;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&lh->gdev->srcu);
-
-	if (!rcu_access_pointer(lh->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(lh->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	switch (cmd) {
@@ -1432,10 +1433,10 @@ static long linereq_ioctl(struct file *file, unsigned int cmd,
 {
 	struct linereq *lr = file->private_data;
 	void __user *ip = (void __user *)arg;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&lr->gdev->srcu);
-
-	if (!rcu_access_pointer(lr->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(lr->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	switch (cmd) {
@@ -1463,10 +1464,10 @@ static __poll_t linereq_poll(struct file *file,
 {
 	struct linereq *lr = file->private_data;
 	__poll_t events = 0;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&lr->gdev->srcu);
-
-	if (!rcu_access_pointer(lr->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(lr->gdev->chip_rp, gc);
+	if (!gc)
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &lr->wait, wait);
@@ -1485,10 +1486,10 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
 	struct gpio_v2_line_event le;
 	ssize_t bytes_read = 0;
 	int ret;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&lr->gdev->srcu);
-
-	if (!rcu_access_pointer(lr->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(lr->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	if (count < sizeof(le))
@@ -1781,10 +1782,10 @@ static __poll_t lineevent_poll(struct file *file,
 {
 	struct lineevent_state *le = file->private_data;
 	__poll_t events = 0;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&le->gdev->srcu);
-
-	if (!rcu_access_pointer(le->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(le->gdev->chip_rp, gc);
+	if (!gc)
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &le->wait, wait);
@@ -1819,10 +1820,10 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
 	ssize_t bytes_read = 0;
 	ssize_t ge_size;
 	int ret;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&le->gdev->srcu);
-
-	if (!rcu_access_pointer(le->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(le->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	/*
@@ -1901,10 +1902,10 @@ static long lineevent_ioctl(struct file *file, unsigned int cmd,
 	struct lineevent_state *le = file->private_data;
 	void __user *ip = (void __user *)arg;
 	struct gpiohandle_data ghd;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&le->gdev->srcu);
-
-	if (!rcu_access_pointer(le->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(le->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	/*
@@ -2434,11 +2435,11 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	struct gpio_chardev_data *cdev = file->private_data;
 	struct gpio_device *gdev = cdev->gdev;
 	void __user *ip = (void __user *)arg;
-
-	guard(srcu)(&gdev->srcu);
+	struct gpio_chip *gc;
 
 	/* We fail any subsequent ioctl():s when the chip is gone */
-	if (!rcu_access_pointer(gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	/* Fill in the struct and pass to userspace */
@@ -2497,12 +2498,9 @@ static void lineinfo_changed_func(struct work_struct *work)
 		 * Pin functions are in general much more static and while it's
 		 * not 100% bullet-proof, it's good enough for most cases.
 		 */
-		scoped_guard(srcu, &ctx->gdev->srcu) {
-			gc = srcu_dereference(ctx->gdev->chip, &ctx->gdev->srcu);
-			if (gc &&
-			    !pinctrl_gpio_can_use_line(gc, ctx->chg.info.offset))
-				ctx->chg.info.flags |= GPIO_V2_LINE_FLAG_USED;
-		}
+		REVOCABLE_TRY_ACCESS_WITH(ctx->gdev->chip_rp, gc);
+		if (gc && !pinctrl_gpio_can_use_line(gc, ctx->chg.info.offset))
+			ctx->chg.info.flags |= GPIO_V2_LINE_FLAG_USED;
 	}
 
 	ret = kfifo_in_spinlocked(&ctx->cdev->events, &ctx->chg, 1,
@@ -2583,10 +2581,10 @@ static __poll_t lineinfo_watch_poll(struct file *file,
 {
 	struct gpio_chardev_data *cdev = file->private_data;
 	__poll_t events = 0;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&cdev->gdev->srcu);
-
-	if (!rcu_access_pointer(cdev->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(cdev->gdev->chip_rp, gc);
+	if (!gc)
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &cdev->wait, pollt);
@@ -2606,10 +2604,10 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 	ssize_t bytes_read = 0;
 	int ret;
 	size_t event_size;
+	struct gpio_chip *gc;
 
-	guard(srcu)(&cdev->gdev->srcu);
-
-	if (!rcu_access_pointer(cdev->gdev->chip))
+	REVOCABLE_TRY_ACCESS_WITH(cdev->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 #ifndef CONFIG_GPIO_CDEV_V1
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 09/11] gpio: Remove gpio_chip_guard by using revocable

[Tzung-Bi Shih]

Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip.  Leverage revocable for accessing the struct
gpio_chip instead of using gpio_chip_guard.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- Separate from v1 for including gpio_chip_guard only.

v1: https://lore.kernel.org/all/20260116081036.352286-23-tzungbi@kernel.org

 drivers/gpio/gpiolib-cdev.c  |   9 +-
 drivers/gpio/gpiolib-sysfs.c |  30 ++++---
 drivers/gpio/gpiolib.c       | 165 ++++++++++++++++++-----------------
 drivers/gpio/gpiolib.h       |  21 -----
 4 files changed, 109 insertions(+), 116 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index ca9c04765df4..270ec6ad639e 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2215,9 +2215,10 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
 	u32 debounce_period_us;
 	unsigned long dflags;
 	const char *label;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return;
 
 	memset(info, 0, sizeof(*info));
@@ -2251,10 +2252,10 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
 	    test_bit(GPIOD_FLAG_IS_HOGGED, &dflags) ||
 	    test_bit(GPIOD_FLAG_EXPORT, &dflags) ||
 	    test_bit(GPIOD_FLAG_SYSFS, &dflags) ||
-	    !gpiochip_line_is_valid(guard.gc, info->offset)) {
+	    !gpiochip_line_is_valid(gc, info->offset)) {
 		info->flags |= GPIO_V2_LINE_FLAG_USED;
 	} else if (!atomic) {
-		if (!pinctrl_gpio_can_use_line(guard.gc, info->offset))
+		if (!pinctrl_gpio_can_use_line(gc, info->offset))
 			info->flags |= GPIO_V2_LINE_FLAG_USED;
 	}
 
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index a4427a5cfa85..566fdda2c5d9 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -215,9 +215,10 @@ static int gpio_sysfs_request_irq(struct gpiod_data *data, unsigned char flags)
 	struct gpio_desc *desc = data->desc;
 	unsigned long irq_flags;
 	int ret;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	data->irq = gpiod_to_irq(desc);
@@ -244,7 +245,7 @@ static int gpio_sysfs_request_irq(struct gpiod_data *data, unsigned char flags)
 	 * Remove this redundant call (along with the corresponding unlock)
 	 * when those drivers have been fixed.
 	 */
-	ret = gpiochip_lock_as_irq(guard.gc, gpiod_hwgpio(desc));
+	ret = gpiochip_lock_as_irq(gc, gpiod_hwgpio(desc));
 	if (ret < 0)
 		goto err_clr_bits;
 
@@ -258,7 +259,7 @@ static int gpio_sysfs_request_irq(struct gpiod_data *data, unsigned char flags)
 	return 0;
 
 err_unlock:
-	gpiochip_unlock_as_irq(guard.gc, gpiod_hwgpio(desc));
+	gpiochip_unlock_as_irq(gc, gpiod_hwgpio(desc));
 err_clr_bits:
 	clear_bit(GPIOD_FLAG_EDGE_RISING, &desc->flags);
 	clear_bit(GPIOD_FLAG_EDGE_FALLING, &desc->flags);
@@ -273,14 +274,15 @@ static int gpio_sysfs_request_irq(struct gpiod_data *data, unsigned char flags)
 static void gpio_sysfs_free_irq(struct gpiod_data *data)
 {
 	struct gpio_desc *desc = data->desc;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return;
 
 	data->irq_flags = 0;
 	free_irq(data->irq, data);
-	gpiochip_unlock_as_irq(guard.gc, gpiod_hwgpio(desc));
+	gpiochip_unlock_as_irq(gc, gpiod_hwgpio(desc));
 	clear_bit(GPIOD_FLAG_EDGE_RISING, &desc->flags);
 	clear_bit(GPIOD_FLAG_EDGE_FALLING, &desc->flags);
 }
@@ -473,13 +475,14 @@ static DEVICE_ATTR_RO(ngpio);
 static int export_gpio_desc(struct gpio_desc *desc)
 {
 	int offset, ret;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	offset = gpiod_hwgpio(desc);
-	if (!gpiochip_line_is_valid(guard.gc, offset)) {
+	if (!gpiochip_line_is_valid(gc, offset)) {
 		pr_debug_ratelimited("%s: GPIO %d masked\n", __func__,
 				     gpiod_hwgpio(desc));
 		return -EINVAL;
@@ -732,6 +735,7 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 	struct gpio_device *gdev;
 	struct attribute **attrs;
 	int status;
+	struct gpio_chip *gc;
 
 	/* can't export until sysfs is available ... */
 	if (!class_is_registered(&gpio_class)) {
@@ -744,8 +748,8 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 		return -EINVAL;
 	}
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	if (test_and_set_bit(GPIOD_FLAG_EXPORT, &desc->flags))
@@ -769,7 +773,7 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 
 	desc_data->desc = desc;
 	mutex_init(&desc_data->mutex);
-	if (guard.gc->direction_input && guard.gc->direction_output)
+	if (gc->direction_input && gc->direction_output)
 		desc_data->direction_can_change = direction_may_change;
 	else
 		desc_data->direction_can_change = false;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index fdae10ec3a17..e62b00191d59 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -449,13 +449,14 @@ int gpiod_get_direction(struct gpio_desc *desc)
 	unsigned long flags;
 	unsigned int offset;
 	int ret;
+	struct gpio_chip *gc;
 
 	ret = validate_desc(desc, __func__);
 	if (ret <= 0)
 		return -EINVAL;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	offset = gpiod_hwgpio(desc);
@@ -469,7 +470,7 @@ int gpiod_get_direction(struct gpio_desc *desc)
 	    test_bit(GPIOD_FLAG_IS_OUT, &flags))
 		return 0;
 
-	ret = gpiochip_get_direction(guard.gc, offset);
+	ret = gpiochip_get_direction(gc, offset);
 	if (ret < 0)
 		return ret;
 
@@ -2474,31 +2475,32 @@ int gpiod_request_commit(struct gpio_desc *desc, const char *label)
 {
 	unsigned int offset;
 	int ret;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	if (test_and_set_bit(GPIOD_FLAG_REQUESTED, &desc->flags))
 		return -EBUSY;
 
 	offset = gpiod_hwgpio(desc);
-	if (!gpiochip_line_is_valid(guard.gc, offset))
+	if (!gpiochip_line_is_valid(gc, offset))
 		return -EINVAL;
 
 	/* NOTE:  gpio_request() can be called in early boot,
 	 * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
 	 */
 
-	if (guard.gc->request) {
-		ret = guard.gc->request(guard.gc, offset);
+	if (gc->request) {
+		ret = gc->request(gc, offset);
 		if (ret > 0)
 			ret = -EBADE;
 		if (ret)
 			goto out_clear_bit;
 	}
 
-	if (guard.gc->get_direction)
+	if (gc->get_direction)
 		gpiod_get_direction(desc);
 
 	ret = desc_set_label(desc, label ? : "?");
@@ -2535,16 +2537,19 @@ int gpiod_request(struct gpio_desc *desc, const char *label)
 void gpiod_free_commit(struct gpio_desc *desc)
 {
 	unsigned long flags;
+	struct gpio_chip *gc;
 
 	might_sleep();
 
-	CLASS(gpio_chip_guard, guard)(desc);
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
+		return;
 
 	flags = READ_ONCE(desc->flags);
 
-	if (guard.gc && test_bit(GPIOD_FLAG_REQUESTED, &flags)) {
-		if (guard.gc->free)
-			guard.gc->free(guard.gc, gpiod_hwgpio(desc));
+	if (test_bit(GPIOD_FLAG_REQUESTED, &flags)) {
+		if (gc->free)
+			gc->free(gc, gpiod_hwgpio(desc));
 
 		clear_bit(GPIOD_FLAG_ACTIVE_LOW, &flags);
 		clear_bit(GPIOD_FLAG_REQUESTED, &flags);
@@ -2696,15 +2701,16 @@ EXPORT_SYMBOL_GPL(gpiochip_free_own_desc);
 int gpio_do_set_config(struct gpio_desc *desc, unsigned long config)
 {
 	int ret;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
-	if (!guard.gc->set_config)
+	if (!gc->set_config)
 		return -ENOTSUPP;
 
-	ret = guard.gc->set_config(guard.gc, gpiod_hwgpio(desc), config);
+	ret = gc->set_config(gc, gpiod_hwgpio(desc), config);
 	if (ret > 0)
 		ret = -EBADE;
 
@@ -2873,9 +2879,10 @@ EXPORT_SYMBOL_GPL(gpiod_direction_input);
 int gpiod_direction_input_nonotify(struct gpio_desc *desc)
 {
 	int ret = 0, dir;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	/*
@@ -2883,7 +2890,7 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
 	 * the chip is output-only, but you can't specify .direction_input()
 	 * and not support the .get() operation, that doesn't make sense.
 	 */
-	if (!guard.gc->get && guard.gc->direction_input) {
+	if (!gc->get && gc->direction_input) {
 		gpiod_warn(desc,
 			   "%s: missing get() but have direction_input()\n",
 			   __func__);
@@ -2896,11 +2903,10 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
 	 * direction (if .get_direction() is supported) else we silently
 	 * assume we are in input mode after this.
 	 */
-	if (guard.gc->direction_input) {
-		ret = gpiochip_direction_input(guard.gc,
-					       gpiod_hwgpio(desc));
-	} else if (guard.gc->get_direction) {
-		dir = gpiochip_get_direction(guard.gc, gpiod_hwgpio(desc));
+	if (gc->direction_input) {
+		ret = gpiochip_direction_input(gc, gpiod_hwgpio(desc));
+	} else if (gc->get_direction) {
+		dir = gpiochip_get_direction(gc, gpiod_hwgpio(desc));
 		if (dir < 0)
 			return dir;
 
@@ -2940,9 +2946,10 @@ static int gpiochip_set(struct gpio_chip *gc, unsigned int offset, int value)
 static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
 {
 	int val = !!value, ret = 0, dir;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	/*
@@ -2950,21 +2957,19 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
 	 * output-only, but if there is then not even a .set() operation it
 	 * is pretty tricky to drive the output line.
 	 */
-	if (!guard.gc->set && !guard.gc->direction_output) {
+	if (!gc->set && !gc->direction_output) {
 		gpiod_warn(desc,
 			   "%s: missing set() and direction_output() operations\n",
 			   __func__);
 		return -EIO;
 	}
 
-	if (guard.gc->direction_output) {
-		ret = gpiochip_direction_output(guard.gc,
-						gpiod_hwgpio(desc), val);
+	if (gc->direction_output) {
+		ret = gpiochip_direction_output(gc, gpiod_hwgpio(desc), val);
 	} else {
 		/* Check that we are in output mode if we can */
-		if (guard.gc->get_direction) {
-			dir = gpiochip_get_direction(guard.gc,
-						     gpiod_hwgpio(desc));
+		if (gc->get_direction) {
+			dir = gpiochip_get_direction(gc, gpiod_hwgpio(desc));
 			if (dir < 0)
 				return dir;
 
@@ -2979,7 +2984,7 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
 		 * If we can't actively set the direction, we are some
 		 * output-only chip, so just drive the output as desired.
 		 */
-		ret = gpiochip_set(guard.gc, gpiod_hwgpio(desc), val);
+		ret = gpiochip_set(gc, gpiod_hwgpio(desc), val);
 		if (ret)
 			return ret;
 	}
@@ -3117,20 +3122,20 @@ int gpiod_direction_output_nonotify(struct gpio_desc *desc, int value)
 int gpiod_enable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags)
 {
 	int ret;
+	struct gpio_chip *gc;
 
 	VALIDATE_DESC(desc);
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
-	if (!guard.gc->en_hw_timestamp) {
+	if (!gc->en_hw_timestamp) {
 		gpiod_warn(desc, "%s: hw ts not supported\n", __func__);
 		return -ENOTSUPP;
 	}
 
-	ret = guard.gc->en_hw_timestamp(guard.gc,
-					gpiod_hwgpio(desc), flags);
+	ret = gc->en_hw_timestamp(gc, gpiod_hwgpio(desc), flags);
 	if (ret)
 		gpiod_warn(desc, "%s: hw ts request failed\n", __func__);
 
@@ -3150,20 +3155,20 @@ EXPORT_SYMBOL_GPL(gpiod_enable_hw_timestamp_ns);
 int gpiod_disable_hw_timestamp_ns(struct gpio_desc *desc, unsigned long flags)
 {
 	int ret;
+	struct gpio_chip *gc;
 
 	VALIDATE_DESC(desc);
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
-	if (!guard.gc->dis_hw_timestamp) {
+	if (!gc->dis_hw_timestamp) {
 		gpiod_warn(desc, "%s: hw ts not supported\n", __func__);
 		return -ENOTSUPP;
 	}
 
-	ret = guard.gc->dis_hw_timestamp(guard.gc, gpiod_hwgpio(desc),
-					 flags);
+	ret = gc->dis_hw_timestamp(gc, gpiod_hwgpio(desc), flags);
 	if (ret)
 		gpiod_warn(desc, "%s: hw ts release failed\n", __func__);
 
@@ -3423,31 +3428,31 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 		unsigned long *mask, *bits;
 		int first, j;
 
-		CLASS(gpio_chip_guard, guard)(desc_array[i]);
-		if (!guard.gc)
+		REVOCABLE_TRY_ACCESS_WITH(desc_array[i]->gdev->chip_rp, gc);
+		if (!gc)
 			return -ENODEV;
 
-		if (likely(guard.gc->ngpio <= FASTPATH_NGPIO)) {
+		if (likely(gc->ngpio <= FASTPATH_NGPIO)) {
 			mask = fastpath_mask;
 			bits = fastpath_bits;
 		} else {
 			gfp_t flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
 
-			mask = bitmap_alloc(guard.gc->ngpio, flags);
+			mask = bitmap_alloc(gc->ngpio, flags);
 			if (!mask)
 				return -ENOMEM;
 
-			bits = bitmap_alloc(guard.gc->ngpio, flags);
+			bits = bitmap_alloc(gc->ngpio, flags);
 			if (!bits) {
 				bitmap_free(mask);
 				return -ENOMEM;
 			}
 		}
 
-		bitmap_zero(mask, guard.gc->ngpio);
+		bitmap_zero(mask, gc->ngpio);
 
 		if (!can_sleep)
-			WARN_ON(guard.gc->can_sleep);
+			WARN_ON(gc->can_sleep);
 
 		/* collect all inputs belonging to the same chip */
 		first = i;
@@ -3462,9 +3467,9 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				i = find_next_zero_bit(array_info->get_mask,
 						       array_size, i);
 		} while ((i < array_size) &&
-			 gpio_device_chip_cmp(desc_array[i]->gdev, guard.gc));
+			 gpio_device_chip_cmp(desc_array[i]->gdev, gc));
 
-		ret = gpio_chip_get_multiple(guard.gc, mask, bits);
+		ret = gpio_chip_get_multiple(gc, mask, bits);
 		if (ret) {
 			if (mask != fastpath_mask)
 				bitmap_free(mask);
@@ -3613,15 +3618,16 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_value);
 static int gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
 {
 	int ret = 0, offset = gpiod_hwgpio(desc);
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	if (value) {
-		ret = gpiochip_direction_input(guard.gc, offset);
+		ret = gpiochip_direction_input(gc, offset);
 	} else {
-		ret = gpiochip_direction_output(guard.gc, offset, 0);
+		ret = gpiochip_direction_output(gc, offset, 0);
 		if (!ret)
 			set_bit(GPIOD_FLAG_IS_OUT, &desc->flags);
 	}
@@ -3642,17 +3648,18 @@ static int gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
 static int gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value)
 {
 	int ret = 0, offset = gpiod_hwgpio(desc);
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	if (value) {
-		ret = gpiochip_direction_output(guard.gc, offset, 1);
+		ret = gpiochip_direction_output(gc, offset, 1);
 		if (!ret)
 			set_bit(GPIOD_FLAG_IS_OUT, &desc->flags);
 	} else {
-		ret = gpiochip_direction_input(guard.gc, offset);
+		ret = gpiochip_direction_input(gc, offset);
 	}
 	trace_gpio_direction(desc_to_gpio(desc), !value, ret);
 	if (ret < 0)
@@ -3665,15 +3672,17 @@ static int gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value)
 
 static int gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
 {
+	struct gpio_chip *gc;
+
 	if (unlikely(!test_bit(GPIOD_FLAG_IS_OUT, &desc->flags)))
 		return -EPERM;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	trace_gpio_value(desc_to_gpio(desc), 0, value);
-	return gpiochip_set(guard.gc, gpiod_hwgpio(desc), value);
+	return gpiochip_set(gc, gpiod_hwgpio(desc), value);
 }
 
 /*
@@ -3768,31 +3777,31 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 		unsigned long *mask, *bits;
 		int count = 0;
 
-		CLASS(gpio_chip_guard, guard)(desc_array[i]);
-		if (!guard.gc)
+		REVOCABLE_TRY_ACCESS_WITH(desc_array[i]->gdev->chip_rp, gc);
+		if (!gc)
 			return -ENODEV;
 
-		if (likely(guard.gc->ngpio <= FASTPATH_NGPIO)) {
+		if (likely(gc->ngpio <= FASTPATH_NGPIO)) {
 			mask = fastpath_mask;
 			bits = fastpath_bits;
 		} else {
 			gfp_t flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
 
-			mask = bitmap_alloc(guard.gc->ngpio, flags);
+			mask = bitmap_alloc(gc->ngpio, flags);
 			if (!mask)
 				return -ENOMEM;
 
-			bits = bitmap_alloc(guard.gc->ngpio, flags);
+			bits = bitmap_alloc(gc->ngpio, flags);
 			if (!bits) {
 				bitmap_free(mask);
 				return -ENOMEM;
 			}
 		}
 
-		bitmap_zero(mask, guard.gc->ngpio);
+		bitmap_zero(mask, gc->ngpio);
 
 		if (!can_sleep)
-			WARN_ON(guard.gc->can_sleep);
+			WARN_ON(gc->can_sleep);
 
 		do {
 			struct gpio_desc *desc = desc_array[i];
@@ -3831,10 +3840,10 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 				i = find_next_zero_bit(array_info->set_mask,
 						       array_size, i);
 		} while ((i < array_size) &&
-			 gpio_device_chip_cmp(desc_array[i]->gdev, guard.gc));
+			 gpio_device_chip_cmp(desc_array[i]->gdev, gc));
 		/* push collected bits to outputs */
 		if (count != 0) {
-			ret = gpiochip_set_multiple(guard.gc, mask, bits);
+			ret = gpiochip_set_multiple(gc, mask, bits);
 			if (ret)
 				return ret;
 		}
@@ -5050,9 +5059,10 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
 	struct gpio_desc *local_desc;
 	int hwnum;
 	int ret;
+	struct gpio_chip *gc;
 
-	CLASS(gpio_chip_guard, guard)(desc);
-	if (!guard.gc)
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
+	if (!gc)
 		return -ENODEV;
 
 	if (test_and_set_bit(GPIOD_FLAG_IS_HOGGED, &desc->flags))
@@ -5060,8 +5070,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
 
 	hwnum = gpiod_hwgpio(desc);
 
-	local_desc = gpiochip_request_own_desc(guard.gc, hwnum, name,
-					       lflags, dflags);
+	local_desc = gpiochip_request_own_desc(gc, hwnum, name, lflags, dflags);
 	if (IS_ERR(local_desc)) {
 		clear_bit(GPIOD_FLAG_IS_HOGGED, &desc->flags);
 		ret = PTR_ERR(local_desc);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index cd136d5b52e9..ce6273cc74f2 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -224,27 +224,6 @@ struct gpio_desc {
 
 #define gpiod_not_found(desc)		(IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
 
-struct gpio_chip_guard {
-	struct gpio_device *gdev;
-	struct gpio_chip *gc;
-	int idx;
-};
-
-DEFINE_CLASS(gpio_chip_guard,
-	     struct gpio_chip_guard,
-	     srcu_read_unlock(&_T.gdev->srcu, _T.idx),
-	     ({
-		struct gpio_chip_guard _guard;
-
-		_guard.gdev = desc->gdev;
-		_guard.idx = srcu_read_lock(&_guard.gdev->srcu);
-		_guard.gc = srcu_dereference(_guard.gdev->chip,
-					     &_guard.gdev->srcu);
-
-		_guard;
-	     }),
-	     struct gpio_desc *desc)
-
 int gpiod_request(struct gpio_desc *desc, const char *label);
 int gpiod_request_commit(struct gpio_desc *desc, const char *label);
 void gpiod_free(struct gpio_desc *desc);
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 10/11] gpio: Leverage revocable for accessing struct gpio_chip

[Tzung-Bi Shih]

Struct gpio_device now provides a revocable provider to the underlying
struct gpio_chip.  Leverage revocable for accessing the struct
gpio_chip.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- Separate from v1(a) for excluding gpio_chip_guard and combine v1(b).

v1(a):
- https://lore.kernel.org/all/20260116081036.352286-23-tzungbi@kernel.org
v1(b):
- https://lore.kernel.org/all/20260116081036.352286-19-tzungbi@kernel.org

 drivers/gpio/gpiolib.c | 52 +++++++++++++++---------------------------
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e62b00191d59..347ff456858f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -338,7 +338,10 @@ EXPORT_SYMBOL(gpio_device_get_label);
  */
 struct gpio_chip *gpio_device_get_chip(struct gpio_device *gdev)
 {
-	return rcu_dereference_check(gdev->chip, 1);
+	struct gpio_chip *gc;
+
+	REVOCABLE_TRY_ACCESS_WITH(gdev->chip_rp, gc);
+	return gc;
 }
 EXPORT_SYMBOL_GPL(gpio_device_get_chip);
 
@@ -558,9 +561,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
 
 	list_for_each_entry_srcu(gdev, &gpio_devices, list,
 				 srcu_read_lock_held(&gpio_devices_srcu)) {
-		guard(srcu)(&gdev->srcu);
-
-		gc = srcu_dereference(gdev->chip, &gdev->srcu);
+		REVOCABLE_TRY_ACCESS_WITH(gdev->chip_rp, gc);
 		if (!gc)
 			continue;
 
@@ -972,9 +973,7 @@ static void gpiochip_setup_devs(void)
 
 	list_for_each_entry_srcu(gdev, &gpio_devices, list,
 				 srcu_read_lock_held(&gpio_devices_srcu)) {
-		guard(srcu)(&gdev->srcu);
-
-		gc = srcu_dereference(gdev->chip, &gdev->srcu);
+		REVOCABLE_TRY_ACCESS_WITH(gdev->chip_rp, gc);
 		if (!gc) {
 			dev_err(&gdev->dev, "Underlying GPIO chip is gone\n");
 			continue;
@@ -1379,11 +1378,11 @@ struct gpio_device *gpio_device_find(const void *data,
 		if (!device_is_registered(&gdev->dev))
 			continue;
 
-		guard(srcu)(&gdev->srcu);
-
-		gc = srcu_dereference(gdev->chip, &gdev->srcu);
+		REVOCABLE_TRY_ACCESS_WITH(gdev->chip_rp, gc);
+		if (!gc)
+			continue;
 
-		if (gc && match(gc, data))
+		if (match(gc, data))
 			return gpio_device_get(gdev);
 	}
 
@@ -3325,16 +3324,10 @@ static int gpio_chip_get_value(struct gpio_chip *gc, const struct gpio_desc *des
 
 static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
 {
-	struct gpio_device *gdev;
 	struct gpio_chip *gc;
 	int value;
 
-	/* FIXME Unable to use gpio_chip_guard due to const desc. */
-	gdev = desc->gdev;
-
-	guard(srcu)(&gdev->srcu);
-
-	gc = srcu_dereference(gdev->chip, &gdev->srcu);
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
 	if (!gc)
 		return -ENODEV;
 
@@ -3375,9 +3368,10 @@ static int gpio_chip_get_multiple(struct gpio_chip *gc,
 /* The 'other' chip must be protected with its GPIO device's SRCU. */
 static bool gpio_device_chip_cmp(struct gpio_device *gdev, struct gpio_chip *gc)
 {
-	guard(srcu)(&gdev->srcu);
+	struct gpio_chip *chip;
 
-	return gc == srcu_dereference(gdev->chip, &gdev->srcu);
+	REVOCABLE_TRY_ACCESS_WITH(gdev->chip_rp, chip);
+	return chip ? chip == gc : false;
 }
 
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
@@ -3400,9 +3394,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 		if (!can_sleep)
 			WARN_ON(array_info->gdev->can_sleep);
 
-		guard(srcu)(&array_info->gdev->srcu);
-		gc = srcu_dereference(array_info->gdev->chip,
-				      &array_info->gdev->srcu);
+		REVOCABLE_TRY_ACCESS_WITH(array_info->gdev->chip_rp, gc);
 		if (!gc)
 			return -ENODEV;
 
@@ -3749,9 +3741,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 				return -EPERM;
 		}
 
-		guard(srcu)(&array_info->gdev->srcu);
-		gc = srcu_dereference(array_info->gdev->chip,
-				      &array_info->gdev->srcu);
+		REVOCABLE_TRY_ACCESS_WITH(array_info->gdev->chip_rp, gc);
 		if (!gc)
 			return -ENODEV;
 
@@ -4049,7 +4039,6 @@ EXPORT_SYMBOL_GPL(gpiod_is_shared);
  */
 int gpiod_to_irq(const struct gpio_desc *desc)
 {
-	struct gpio_device *gdev;
 	struct gpio_chip *gc;
 	int offset;
 	int ret;
@@ -4058,10 +4047,7 @@ int gpiod_to_irq(const struct gpio_desc *desc)
 	if (ret <= 0)
 		return -EINVAL;
 
-	gdev = desc->gdev;
-	/* FIXME Cannot use gpio_chip_guard due to const desc. */
-	guard(srcu)(&gdev->srcu);
-	gc = srcu_dereference(gdev->chip, &gdev->srcu);
+	REVOCABLE_TRY_ACCESS_WITH(desc->gdev->chip_rp, gc);
 	if (!gc)
 		return -ENODEV;
 
@@ -5440,9 +5426,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
 	if (priv->newline)
 		seq_putc(s, '\n');
 
-	guard(srcu)(&gdev->srcu);
-
-	gc = srcu_dereference(gdev->chip, &gdev->srcu);
+	REVOCABLE_TRY_ACCESS_WITH(gdev->chip_rp, gc);
 	if (!gc) {
 		seq_printf(s, "%s: (dangling chip)\n", dev_name(&gdev->dev));
 		return 0;
-- 
2.53.0.rc2.204.g2597b5adb4-goog

[PATCH v2 11/11] gpio: Remove unused `chip` and `srcu` in struct gpio_device

[Tzung-Bi Shih]

`chip` and `srcu` in struct gpio_device are unused as their usages are
replaced to use revocable.  Remove them.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260116081036.352286-24-tzungbi@kernel.org

 drivers/gpio/gpiolib.c | 26 +-------------------------
 drivers/gpio/gpiolib.h |  4 ----
 2 files changed, 1 insertion(+), 29 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 347ff456858f..bcc7834b2362 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -423,8 +423,6 @@ static int gpiochip_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
 	int ret;
 
-	lockdep_assert_held(&gc->gpiodev->srcu);
-
 	if (WARN_ON(!gc->get_direction))
 		return -EOPNOTSUPP;
 
@@ -875,7 +873,6 @@ static void gpiodev_release(struct device *dev)
 	ida_free(&gpio_ida, gdev->id);
 	kfree_const(gdev->label);
 	kfree(gdev->descs);
-	cleanup_srcu_struct(&gdev->srcu);
 	kfree(gdev);
 }
 
@@ -1076,14 +1073,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 		goto err_free_gdev;
 	gdev->id = ret;
 
-	ret = init_srcu_struct(&gdev->srcu);
-	if (ret)
-		goto err_free_ida;
-	rcu_assign_pointer(gdev->chip, gc);
-
 	ret = init_srcu_struct(&gdev->desc_srcu);
 	if (ret)
-		goto err_cleanup_gdev_srcu;
+		goto err_free_ida;
 
 	ret = dev_set_name(&gdev->dev, GPIOCHIP_NAME "%d", gdev->id);
 	if (ret)
@@ -1277,8 +1269,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 
 err_cleanup_desc_srcu:
 	cleanup_srcu_struct(&gdev->desc_srcu);
-err_cleanup_gdev_srcu:
-	cleanup_srcu_struct(&gdev->srcu);
 err_free_ida:
 	ida_free(&gpio_ida, gdev->id);
 err_free_gdev:
@@ -1315,8 +1305,6 @@ void gpiochip_remove(struct gpio_chip *gc)
 	synchronize_srcu(&gpio_devices_srcu);
 
 	/* Numb the device, cancelling all outstanding operations */
-	rcu_assign_pointer(gdev->chip, NULL);
-	synchronize_srcu(&gdev->srcu);
 	revocable_provider_revoke(&gdev->chip_rp);
 	gpio_device_teardown_shared(gdev);
 	gpiochip_irqchip_remove(gc);
@@ -2822,8 +2810,6 @@ static int gpiochip_direction_input(struct gpio_chip *gc, unsigned int offset)
 {
 	int ret;
 
-	lockdep_assert_held(&gc->gpiodev->srcu);
-
 	if (WARN_ON(!gc->direction_input))
 		return -EOPNOTSUPP;
 
@@ -2839,8 +2825,6 @@ static int gpiochip_direction_output(struct gpio_chip *gc, unsigned int offset,
 {
 	int ret;
 
-	lockdep_assert_held(&gc->gpiodev->srcu);
-
 	if (WARN_ON(!gc->direction_output))
 		return -EOPNOTSUPP;
 
@@ -2930,8 +2914,6 @@ static int gpiochip_set(struct gpio_chip *gc, unsigned int offset, int value)
 {
 	int ret;
 
-	lockdep_assert_held(&gc->gpiodev->srcu);
-
 	if (WARN_ON(unlikely(!gc->set)))
 		return -EOPNOTSUPP;
 
@@ -3285,8 +3267,6 @@ static int gpiochip_get(struct gpio_chip *gc, unsigned int offset)
 {
 	int ret;
 
-	lockdep_assert_held(&gc->gpiodev->srcu);
-
 	/* Make sure this is called after checking for gc->get(). */
 	ret = gc->get(gc, offset);
 	if (ret > 1)
@@ -3340,8 +3320,6 @@ static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
 static int gpio_chip_get_multiple(struct gpio_chip *gc,
 				  unsigned long *mask, unsigned long *bits)
 {
-	lockdep_assert_held(&gc->gpiodev->srcu);
-
 	if (gc->get_multiple) {
 		int ret;
 
@@ -3695,8 +3673,6 @@ static int gpiochip_set_multiple(struct gpio_chip *gc,
 	unsigned int i;
 	int ret;
 
-	lockdep_assert_held(&gc->gpiodev->srcu);
-
 	if (gc->set_multiple) {
 		ret = gc->set_multiple(gc, mask, bits);
 		if (ret > 0)
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index ce6273cc74f2..2179074d2362 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -28,7 +28,6 @@
  * @chrdev: character device for the GPIO device
  * @id: numerical ID number for the GPIO chip
  * @owner: helps prevent removal of modules exporting active GPIOs
- * @chip: pointer to the corresponding gpiochip, holding static
  * data for this device
  * @descs: array of ngpio descriptors.
  * @valid_mask: If not %NULL, holds bitmask of GPIOs which are valid to be
@@ -51,7 +50,6 @@
  *                 process context
  * @device_notifier: used to notify character device wait queues about the GPIO
  *                   device being unregistered
- * @srcu: protects the pointer to the underlying GPIO chip
  * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
  * @pin_ranges: range of pins served by the GPIO driver
  *
@@ -65,7 +63,6 @@ struct gpio_device {
 	struct cdev		chrdev;
 	int			id;
 	struct module		*owner;
-	struct gpio_chip __rcu	*chip;
 	struct gpio_desc	*descs;
 	unsigned long		*valid_mask;
 	struct srcu_struct	desc_srcu;
@@ -79,7 +76,6 @@ struct gpio_device {
 	rwlock_t		line_state_lock;
 	struct workqueue_struct	*line_state_wq;
 	struct blocking_notifier_head device_notifier;
-	struct srcu_struct	srcu;
 	struct revocable_provider __rcu *chip_rp;
 
 #ifdef CONFIG_PINCTRL
-- 
2.53.0.rc2.204.g2597b5adb4-goog

Re: [PATCH v2 08/11] gpio: cdev: Leverage revocable for accessing struct gpio_chip

[Tzung-Bi Shih]

On Tue, Feb 03, 2026 at 06:10:55AM +0000, Tzung-Bi Shih wrote:
> ---
> v2:
> - Change usages accordingly after applying
>   https://lore.kernel.org/all/20260129143733.45618-4-tzungbi@kernel.org.
>   - Preserve a local storage for `struct revocable`.
> - Combine multiple patches (see "v1:").

Forgot to mention it in the changelog:
- v2 fixes a race condition reported in
  https://lore.kernel.org/all/CAMRc=McDaipt85OHm0MksLkuf6E79dY1uNSqqbcJnoQTUs81Pw@mail.gmail.com/
  and analyzed in
  https://lore.kernel.org/all/aXEEUWwkxHZzCnaI@tzungbi-laptop/.
  In v1, the blocking_notifier_chain_unregister() will be skipped if the
  chip has been removed, leading an UAF in gpiolib_cdev_unregister().
  In v2, it won't skip blocking_notifier_chain_unregister().

> 
> v1:
> - https://lore.kernel.org/all/20260116081036.352286-14-tzungbi@kernel.org
> - https://lore.kernel.org/all/20260116081036.352286-15-tzungbi@kernel.org
> - https://lore.kernel.org/all/20260116081036.352286-16-tzungbi@kernel.org
> - https://lore.kernel.org/all/20260116081036.352286-17-tzungbi@kernel.org
> - https://lore.kernel.org/all/20260116081036.352286-18-tzungbi@kernel.org

Re: [PATCH v2 00/11] gpio: Adopt revocable mechanism for UAF prevention

[Bartosz Golaszewski]

On Tue, Feb 3, 2026 at 7:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> This series transitions the UAF prevention logic within the GPIO core
> (gpiolib) to use the 'revocable' mechanism.
>
> The existing code aims to prevent UAF issues when the underlying GPIO
> chip is removed.  This series replaces that custom logic with the
> generic 'revocable' API, which is designed to handle such lifecycle
> dependencies.  There should be no changes in behavior.
>
> The series applies after:
> - https://lore.kernel.org/all/20260129143733.45618-1-tzungbi@kernel.org
> - https://lore.kernel.org/all/20260203060210.972243-1-tzungbi@kernel.org
>
> Bartosz: the series was planned to send after -rc1 comes.  But I think
> it'd be great to send out for your early review and testing if possible.
> The series base on v6.19-rc8, driver-core-next, and gpio/for-next.
> Please use the temporary integration testing branch
> https://git.kernel.org/pub/scm/linux/kernel/git/tzungbi/chrome-platform.git/log/?h=gpio_rev
> if you'd like to.
>

This is not a Reviewed-by yet but FWIW:

Tested-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

With this revision there's no impact on stability or performance and I
haven't seen any KASAN splats or kmemleak reports.

I'll try to review all the patches individually.

Bartosz

Re: [PATCH v2 00/11] gpio: Adopt revocable mechanism for UAF prevention

[Bartosz Golaszewski]

On Tue, Feb 3, 2026 at 7:11 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> This series transitions the UAF prevention logic within the GPIO core
> (gpiolib) to use the 'revocable' mechanism.
>
> The existing code aims to prevent UAF issues when the underlying GPIO
> chip is removed.  This series replaces that custom logic with the
> generic 'revocable' API, which is designed to handle such lifecycle
> dependencies.  There should be no changes in behavior.
>
> The series applies after:
> - https://lore.kernel.org/all/20260129143733.45618-1-tzungbi@kernel.org
> - https://lore.kernel.org/all/20260203060210.972243-1-tzungbi@kernel.org
>
> Bartosz: the series was planned to send after -rc1 comes.  But I think
> it'd be great to send out for your early review and testing if possible.
> The series base on v6.19-rc8, driver-core-next, and gpio/for-next.
> Please use the temporary integration testing branch
> https://git.kernel.org/pub/scm/linux/kernel/git/tzungbi/chrome-platform.git/log/?h=gpio_rev
> if you'd like to.
>

One high-level note: for this to be accepted into GPIO, the revocable
API contract must state very clearly that revocable_try_access() works
from process AND atomic context while also allowing sleeping inside
revocable critical sections. I'm saying this because while we use
naked SRCU, we can rely on the SRCU contract. Once we switch to
revocable, if someone - for instance - comes up with an idea of
replacing the internal primitives with rwsem, GPIO will break.

Bartosz

Re: [PATCH v2 03/11] gpio: sysfs: Remove redundant check for struct gpio_chip

[Bartosz Golaszewski]

On Tue, 3 Feb 2026 07:10:50 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> gpiochip_sysfs_unregister() is only called by gpiochip_remove() where
> the struct gpio_chip is ensured.
>
> Remove the redundant check.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> v2:
> - No changes.
>
> v1: https://lore.kernel.org/all/20260116081036.352286-9-tzungbi@kernel.org
>
>  drivers/gpio/gpiolib-sysfs.c | 9 +--------
>  drivers/gpio/gpiolib-sysfs.h | 6 ++++--
>  drivers/gpio/gpiolib.c       | 2 +-
>  3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index cd553acf3055..8e6b09d8b559 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -1048,11 +1048,10 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
>  	return 0;
>  }
>
> -void gpiochip_sysfs_unregister(struct gpio_device *gdev)
> +void gpiochip_sysfs_unregister(struct gpio_device *gdev, struct gpio_chip *chip)
>  {
>  	struct gpiodev_data *data;
>  	struct gpio_desc *desc;
> -	struct gpio_chip *chip;
>
>  	scoped_guard(mutex, &sysfs_lock) {
>  		data = gdev_get_data(gdev);
> @@ -1066,12 +1065,6 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
>  		kfree(data);
>  	}
>
> -	guard(srcu)(&gdev->srcu);
> -
> -	chip = srcu_dereference(gdev->chip, &gdev->srcu);
> -	if (!chip)
> -		return;
> -
>  	/* unregister gpiod class devices owned by sysfs */
>  	for_each_gpio_desc_with_flag(chip, desc, GPIOD_FLAG_SYSFS) {
>  		gpiod_unexport(desc);
> diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
> index b794b396d6a5..93debe8e118c 100644
> --- a/drivers/gpio/gpiolib-sysfs.h
> +++ b/drivers/gpio/gpiolib-sysfs.h
> @@ -8,7 +8,8 @@ struct gpio_device;
>  #ifdef CONFIG_GPIO_SYSFS
>
>  int gpiochip_sysfs_register(struct gpio_device *gdev);
> -void gpiochip_sysfs_unregister(struct gpio_device *gdev);
> +void gpiochip_sysfs_unregister(struct gpio_device *gdev,
> +			       struct gpio_chip *chip);
>
>  #else
>
> @@ -17,7 +18,8 @@ static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
>  	return 0;
>  }
>
> -static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
> +static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev,
> +					     struct gpio_chip *chip)
>  {
>  }
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index a6dd07be126c..3137e6f1108a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1281,7 +1281,7 @@ void gpiochip_remove(struct gpio_chip *gc)
>  	struct gpio_device *gdev = gc->gpiodev;
>
>  	/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
> -	gpiochip_sysfs_unregister(gdev);
> +	gpiochip_sysfs_unregister(gdev, gc);

I understand the intention here but I really don't like passing both gc and
gdev here. We can get the address of the gpio_device from gpio_chip so why not
do this and pass only variable?

Bartosz

>  	gpiochip_free_hogs(gc);
>  	gpiochip_free_remaining_irqs(gc);
>
> --
> 2.53.0.rc2.204.g2597b5adb4-goog
>
>

Re: [PATCH v2 04/11] gpio: Ensure struct gpio_chip for gpiochip_setup_dev()

[Bartosz Golaszewski]

On Tue, 3 Feb 2026 07:10:51 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> Ensure struct gpio_chip for gpiochip_setup_dev().  This eliminates a few
> checks for struct gpio_chip.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Same comment as patch 3. I don't like seeing both gpio_chip and gpio_device
being passed to the same function. If you already dereferenced gpio_chip, pass
it on its own and get the address of the associated devices from its member
pointer.

Bartosz

Re: [PATCH v2 07/11] gpio: Add revocable provider handle for struct gpio_chip

[Bartosz Golaszewski]

On Tue, 3 Feb 2026 07:10:54 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> The underlying chip can be removed asynchronously.  `gdev->srcu` is used
> to ensure the synchronization before accessing `gdev->chip`.
>
> Revocable encapsulates the details.  Add revocable provider handle for
> the corresponding struct gpio_chip in struct gpio_device so that it can
> start to hide the synchronization details.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---

[snip]

> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> index 3abb90385829..cd136d5b52e9 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -52,6 +52,7 @@
>   * @device_notifier: used to notify character device wait queues about the GPIO
>   *                   device being unregistered
>   * @srcu: protects the pointer to the underlying GPIO chip
> + * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
>   * @pin_ranges: range of pins served by the GPIO driver
>   *
>   * This state container holds most of the runtime variable data
> @@ -79,6 +80,7 @@ struct gpio_device {
>  	struct workqueue_struct	*line_state_wq;
>  	struct blocking_notifier_head device_notifier;
>  	struct srcu_struct	srcu;
> +	struct revocable_provider __rcu *chip_rp;
>

Why __rcu? This doesn't live in a different address space, only the internal
resource it protects does. If anything - this could be __attribute__((noderef))
but even that is questionable as this is an opaque structure.

Bart

>  #ifdef CONFIG_PINCTRL
>  	/*
> --
> 2.53.0.rc2.204.g2597b5adb4-goog
>
>

Re: [PATCH v2 08/11] gpio: cdev: Leverage revocable for accessing struct gpio_chip

[Bartosz Golaszewski]

On Tue, Feb 3, 2026 at 7:12 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> Struct gpio_device now provides a revocable provider to the underlying
> struct gpio_chip.  Leverage revocable for accessing the struct
> gpio_chip.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---

[snip]

> @@ -1432,10 +1433,10 @@ static long linereq_ioctl(struct file *file, unsigned int cmd,
>  {
>         struct linereq *lr = file->private_data;
>         void __user *ip = (void __user *)arg;
> +       struct gpio_chip *gc;
>
> -       guard(srcu)(&lr->gdev->srcu);
> -
> -       if (!rcu_access_pointer(lr->gdev->chip))
> +       REVOCABLE_TRY_ACCESS_WITH(lr->gdev->chip_rp, gc);
> +       if (!gc)
>                 return -ENODEV;
>

If we're doing this, then I'd love to see something that actually
makes the code smaller like:

REVOCABLE_TRY_ACCESS_WITH_OR_RETURN(lr->gdev->chip_rp, gc, -ENODEV);

which would allow dropping the repeated checks.

Bart

Re: [PATCH v2 09/11] gpio: Remove gpio_chip_guard by using revocable

[Bartosz Golaszewski]

On Tue, 3 Feb 2026 07:10:56 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> Struct gpio_device now provides a revocable provider to the underlying
> struct gpio_chip.  Leverage revocable for accessing the struct
> gpio_chip instead of using gpio_chip_guard.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> --

[snip]

> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> index cd136d5b52e9..ce6273cc74f2 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -224,27 +224,6 @@ struct gpio_desc {
>
>  #define gpiod_not_found(desc)		(IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
>
> -struct gpio_chip_guard {
> -	struct gpio_device *gdev;
> -	struct gpio_chip *gc;
> -	int idx;
> -};
> -
> -DEFINE_CLASS(gpio_chip_guard,
> -	     struct gpio_chip_guard,
> -	     srcu_read_unlock(&_T.gdev->srcu, _T.idx),
> -	     ({
> -		struct gpio_chip_guard _guard;
> -
> -		_guard.gdev = desc->gdev;
> -		_guard.idx = srcu_read_lock(&_guard.gdev->srcu);
> -		_guard.gc = srcu_dereference(_guard.gdev->chip,
> -					     &_guard.gdev->srcu);
> -
> -		_guard;
> -	     }),
> -	     struct gpio_desc *desc)
> -

Ah, yes, I'll gladly export this into someone else's header. :)

Bart

Re: [PATCH v2 03/11] gpio: sysfs: Remove redundant check for struct gpio_chip

[Tzung-Bi Shih]

On Wed, Feb 04, 2026 at 04:33:50AM -0600, Bartosz Golaszewski wrote:
> On Tue, 3 Feb 2026 07:10:50 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index a6dd07be126c..3137e6f1108a 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -1281,7 +1281,7 @@ void gpiochip_remove(struct gpio_chip *gc)
> >  	struct gpio_device *gdev = gc->gpiodev;
> >
> >  	/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
> > -	gpiochip_sysfs_unregister(gdev);
> > +	gpiochip_sysfs_unregister(gdev, gc);
> 
> I understand the intention here but I really don't like passing both gc and
> gdev here. We can get the address of the gpio_device from gpio_chip so why not
> do this and pass only variable?

Ack, will fix in the next version.

Re: [PATCH v2 04/11] gpio: Ensure struct gpio_chip for gpiochip_setup_dev()

[Tzung-Bi Shih]

On Wed, Feb 04, 2026 at 04:36:00AM -0600, Bartosz Golaszewski wrote:
> On Tue, 3 Feb 2026 07:10:51 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > Ensure struct gpio_chip for gpiochip_setup_dev().  This eliminates a few
> > checks for struct gpio_chip.
> >
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> 
> Same comment as patch 3. I don't like seeing both gpio_chip and gpio_device
> being passed to the same function. If you already dereferenced gpio_chip, pass
> it on its own and get the address of the associated devices from its member
> pointer.

Ack, will fix in the next version.

Re: [PATCH v2 07/11] gpio: Add revocable provider handle for struct gpio_chip

[Tzung-Bi Shih]

On Wed, Feb 04, 2026 at 07:58:44AM -0500, Bartosz Golaszewski wrote:
> On Tue, 3 Feb 2026 07:10:54 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> > index 3abb90385829..cd136d5b52e9 100644
> > --- a/drivers/gpio/gpiolib.h
> > +++ b/drivers/gpio/gpiolib.h
> > @@ -52,6 +52,7 @@
> >   * @device_notifier: used to notify character device wait queues about the GPIO
> >   *                   device being unregistered
> >   * @srcu: protects the pointer to the underlying GPIO chip
> > + * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
> >   * @pin_ranges: range of pins served by the GPIO driver
> >   *
> >   * This state container holds most of the runtime variable data
> > @@ -79,6 +80,7 @@ struct gpio_device {
> >  	struct workqueue_struct	*line_state_wq;
> >  	struct blocking_notifier_head device_notifier;
> >  	struct srcu_struct	srcu;
> > +	struct revocable_provider __rcu *chip_rp;
> >
> 
> Why __rcu? This doesn't live in a different address space, only the internal
> resource it protects does. If anything - this could be __attribute__((noderef))
> but even that is questionable as this is an opaque structure.

For fixing a race on the pointer itself.  See also [1].

[1] https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org

Re: [PATCH v2 07/11] gpio: Add revocable provider handle for struct gpio_chip

[Bartosz Golaszewski]

On Thu, Feb 5, 2026 at 9:52 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Wed, Feb 04, 2026 at 07:58:44AM -0500, Bartosz Golaszewski wrote:
> > On Tue, 3 Feb 2026 07:10:54 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > > diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> > > index 3abb90385829..cd136d5b52e9 100644
> > > --- a/drivers/gpio/gpiolib.h
> > > +++ b/drivers/gpio/gpiolib.h
> > > @@ -52,6 +52,7 @@
> > >   * @device_notifier: used to notify character device wait queues about the GPIO
> > >   *                   device being unregistered
> > >   * @srcu: protects the pointer to the underlying GPIO chip
> > > + * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
> > >   * @pin_ranges: range of pins served by the GPIO driver
> > >   *
> > >   * This state container holds most of the runtime variable data
> > > @@ -79,6 +80,7 @@ struct gpio_device {
> > >     struct workqueue_struct *line_state_wq;
> > >     struct blocking_notifier_head device_notifier;
> > >     struct srcu_struct      srcu;
> > > +   struct revocable_provider __rcu *chip_rp;
> > >
> >
> > Why __rcu? This doesn't live in a different address space, only the internal
> > resource it protects does. If anything - this could be __attribute__((noderef))
> > but even that is questionable as this is an opaque structure.
>
> For fixing a race on the pointer itself.  See also [1].
>
> [1] https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org

So we're just using a double RCU here? One to protect the resource and
another to protect the protector of the resource? I can't say I'm a
fan of this. I really want to like this interface but is there really
no way to hide the implementation details from the caller? Isn't this
the whole point? As it is: the user still has to care about an
RCU-protected pointer.

Bartosz

Re: [PATCH v2 07/11] gpio: Add revocable provider handle for struct gpio_chip

[Tzung-Bi Shih]

On Thu, Feb 05, 2026 at 05:57:15PM +0100, Bartosz Golaszewski wrote:
> On Thu, Feb 5, 2026 at 9:52 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On Wed, Feb 04, 2026 at 07:58:44AM -0500, Bartosz Golaszewski wrote:
> > > On Tue, 3 Feb 2026 07:10:54 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > > > diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> > > > index 3abb90385829..cd136d5b52e9 100644
> > > > --- a/drivers/gpio/gpiolib.h
> > > > +++ b/drivers/gpio/gpiolib.h
> > > > @@ -52,6 +52,7 @@
> > > >   * @device_notifier: used to notify character device wait queues about the GPIO
> > > >   *                   device being unregistered
> > > >   * @srcu: protects the pointer to the underlying GPIO chip
> > > > + * @chip_rp: revocable provider handle for the corresponding struct gpio_chip.
> > > >   * @pin_ranges: range of pins served by the GPIO driver
> > > >   *
> > > >   * This state container holds most of the runtime variable data
> > > > @@ -79,6 +80,7 @@ struct gpio_device {
> > > >     struct workqueue_struct *line_state_wq;
> > > >     struct blocking_notifier_head device_notifier;
> > > >     struct srcu_struct      srcu;
> > > > +   struct revocable_provider __rcu *chip_rp;
> > > >
> > >
> > > Why __rcu? This doesn't live in a different address space, only the internal
> > > resource it protects does. If anything - this could be __attribute__((noderef))
> > > but even that is questionable as this is an opaque structure.
> >
> > For fixing a race on the pointer itself.  See also [1].
> >
> > [1] https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org
> 
> So we're just using a double RCU here? One to protect the resource and
> another to protect the protector of the resource? I can't say I'm a
> fan of this. I really want to like this interface but is there really
> no way to hide the implementation details from the caller? Isn't this
> the whole point? As it is: the user still has to care about an
> RCU-protected pointer.

Will think about it but I have no better idea for now.

Ideally, I think the user doesn't need to interact with the RCU (even if it's
annotated with __rcu) but revocable APIs should handle it correctly.