* [PATCH v6 1/5] gpio: shared: factor out per-entry teardown helper
2026-10-21 14:23 [PATCH v6 0/5] gpio: shared: replace platform lookup with dummy software nodes Bartosz Golaszewski
@ 2026-10-21 14:23 ` Bartosz Golaszewski
2026-10-21 14:24 ` [PATCH v6 2/5] gpio: shared: factor out per-entry setup helper Bartosz Golaszewski
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-10-21 14:23 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Abel Vesa
Cc: linux-arm-msm, linux-gpio, linux-kernel, Bartosz Golaszewski
Move the body of the loop in gpio_device_teardown_shared() into
gpio_shared_teardown_entry(). This is a preparation for unwinding a
partially completed gpiochip_setup_shared(), which will need to tear
down individual entries. No functional change.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/gpiolib-shared.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 8c9ac9d9330490b17fb3936b8758beb97a308a15..64bb450f0a8461d214a45aa97e2474ff0d1857ce 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -517,6 +517,27 @@ static void gpio_shared_remove_adev(struct auxiliary_device *adev)
auxiliary_device_uninit(adev);
}
+static void gpio_shared_teardown_entry(struct gpio_device *gdev,
+ struct gpio_shared_entry *entry)
+{
+ struct gpio_shared_ref *ref;
+
+ scoped_guard(mutex, &entry->lock)
+ gpiod_free_commit(&gdev->descs[entry->offset]);
+
+ list_for_each_entry(ref, &entry->refs, list) {
+ if (ref->lookup) {
+ gpiod_remove_lookup_table(ref->lookup);
+ kfree(ref->lookup->table[0].key);
+ kfree(ref->lookup->dev_id);
+ kfree(ref->lookup);
+ ref->lookup = NULL;
+ }
+
+ gpio_shared_remove_adev(&ref->adev);
+ }
+}
+
int gpiochip_setup_shared(struct gpio_chip *gc)
{
struct gpio_device *gdev = gc->gpiodev;
@@ -606,26 +627,12 @@ int gpiochip_setup_shared(struct gpio_chip *gc)
void gpio_device_teardown_shared(struct gpio_device *gdev)
{
struct gpio_shared_entry *entry;
- struct gpio_shared_ref *ref;
list_for_each_entry(entry, &gpio_shared_list, list) {
if (!device_match_fwnode(&gdev->dev, entry->fwnode))
continue;
- scoped_guard(mutex, &entry->lock)
- gpiod_free_commit(&gdev->descs[entry->offset]);
-
- list_for_each_entry(ref, &entry->refs, list) {
- if (ref->lookup) {
- gpiod_remove_lookup_table(ref->lookup);
- kfree(ref->lookup->table[0].key);
- kfree(ref->lookup->dev_id);
- kfree(ref->lookup);
- ref->lookup = NULL;
- }
-
- gpio_shared_remove_adev(&ref->adev);
- }
+ gpio_shared_teardown_entry(gdev, entry);
}
}
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v6 2/5] gpio: shared: factor out per-entry setup helper
2026-10-21 14:23 [PATCH v6 0/5] gpio: shared: replace platform lookup with dummy software nodes Bartosz Golaszewski
2026-10-21 14:23 ` [PATCH v6 1/5] gpio: shared: factor out per-entry teardown helper Bartosz Golaszewski
@ 2026-10-21 14:24 ` Bartosz Golaszewski
2026-10-21 14:24 ` [PATCH v6 3/5] gpio: shared: unwind partial failures in gpiochip_setup_shared() Bartosz Golaszewski
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-10-21 14:24 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Abel Vesa
Cc: linux-arm-msm, linux-gpio, linux-kernel, Bartosz Golaszewski
Move the per-entry part of gpiochip_setup_shared() into
gpio_shared_setup_entry(). This is a preparation for making the setup
all-or-nothing, which requires handling the failure of a single entry
in isolation. No functional change.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/gpiolib-shared.c | 112 ++++++++++++++++++++++++------------------
1 file changed, 63 insertions(+), 49 deletions(-)
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 64bb450f0a8461d214a45aa97e2474ff0d1857ce..47b434db72f1ebd35ec36ebdb35f30809f4522f2 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -538,12 +538,71 @@ static void gpio_shared_teardown_entry(struct gpio_device *gdev,
}
}
+static int gpio_shared_setup_entry(struct gpio_chip *gc,
+ struct gpio_shared_entry *entry)
+{
+ struct gpio_device *gdev = gc->gpiodev;
+ struct gpio_shared_ref *ref;
+ struct gpio_desc *desc;
+ int ret;
+
+ scoped_guard(mutex, &entry->lock) {
+#if IS_ENABLED(CONFIG_OF)
+ if (is_of_node(entry->fwnode) && gc->of_xlate) {
+ /*
+ * This is the earliest that we can tranlate the
+ * devicetree offset to the chip offset.
+ */
+ struct of_phandle_args gpiospec = { };
+
+ gpiospec.np = to_of_node(entry->fwnode);
+ gpiospec.args_count = 2;
+ gpiospec.args[0] = entry->offset;
+
+ ret = gc->of_xlate(gc, &gpiospec, NULL);
+ if (ret < 0)
+ return ret;
+
+ entry->offset = ret;
+ }
+#endif /* CONFIG_OF */
+
+ desc = &gdev->descs[entry->offset];
+
+ __set_bit(GPIOD_FLAG_SHARED, &desc->flags);
+ /*
+ * Shared GPIOs are not requested via the normal path. Make
+ * them inaccessible to anyone even before we register the
+ * chip.
+ */
+ ret = gpiod_request_commit(desc, "shared");
+ if (ret)
+ return ret;
+
+ pr_debug("GPIO %u owned by %s is shared by multiple consumers\n",
+ entry->offset, gpio_device_get_label(gdev));
+ }
+
+ list_for_each_entry(ref, &entry->refs, list) {
+ pr_debug("Setting up a shared GPIO entry for %s (con_id: '%s')\n",
+ fwnode_get_name(ref->fwnode) ?: "(no fwnode)",
+ ref->con_id ?: "(none)");
+
+ ret = gpio_shared_make_adev(gdev, entry, ref);
+ if (ret) {
+ gpiod_free_commit(desc);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
int gpiochip_setup_shared(struct gpio_chip *gc)
{
struct gpio_device *gdev = gc->gpiodev;
struct gpio_shared_entry *entry;
struct gpio_shared_ref *ref;
- struct gpio_desc *desc;
int ret;
list_for_each_entry(entry, &gpio_shared_list, list) {
@@ -571,54 +630,9 @@ int gpiochip_setup_shared(struct gpio_chip *gc)
if (list_count_nodes(&entry->refs) <= 1)
continue;
- scoped_guard(mutex, &entry->lock) {
-#if IS_ENABLED(CONFIG_OF)
- if (is_of_node(entry->fwnode) && gc->of_xlate) {
- /*
- * This is the earliest that we can tranlate the
- * devicetree offset to the chip offset.
- */
- struct of_phandle_args gpiospec = { };
-
- gpiospec.np = to_of_node(entry->fwnode);
- gpiospec.args_count = 2;
- gpiospec.args[0] = entry->offset;
-
- ret = gc->of_xlate(gc, &gpiospec, NULL);
- if (ret < 0)
- return ret;
-
- entry->offset = ret;
- }
-#endif /* CONFIG_OF */
-
- desc = &gdev->descs[entry->offset];
-
- __set_bit(GPIOD_FLAG_SHARED, &desc->flags);
- /*
- * Shared GPIOs are not requested via the normal path. Make
- * them inaccessible to anyone even before we register the
- * chip.
- */
- ret = gpiod_request_commit(desc, "shared");
- if (ret)
- return ret;
-
- pr_debug("GPIO %u owned by %s is shared by multiple consumers\n",
- entry->offset, gpio_device_get_label(gdev));
- }
-
- list_for_each_entry(ref, &entry->refs, list) {
- pr_debug("Setting up a shared GPIO entry for %s (con_id: '%s')\n",
- fwnode_get_name(ref->fwnode) ?: "(no fwnode)",
- ref->con_id ?: "(none)");
-
- ret = gpio_shared_make_adev(gdev, entry, ref);
- if (ret) {
- gpiod_free_commit(desc);
- return ret;
- }
- }
+ ret = gpio_shared_setup_entry(gc, entry);
+ if (ret)
+ return ret;
}
return 0;
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v6 3/5] gpio: shared: unwind partial failures in gpiochip_setup_shared()
2026-10-21 14:23 [PATCH v6 0/5] gpio: shared: replace platform lookup with dummy software nodes Bartosz Golaszewski
2026-10-21 14:23 ` [PATCH v6 1/5] gpio: shared: factor out per-entry teardown helper Bartosz Golaszewski
2026-10-21 14:24 ` [PATCH v6 2/5] gpio: shared: factor out per-entry setup helper Bartosz Golaszewski
@ 2026-10-21 14:24 ` Bartosz Golaszewski
2026-10-21 14:24 ` [PATCH v6 4/5] gpio: shared: detach the proxy device from struct gpio_shared_ref Bartosz Golaszewski
2026-10-21 14:24 ` [PATCH v6 5/5] gpio: shared: convert to using software nodes for GPIO shared proxy lookup Bartosz Golaszewski
4 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-10-21 14:24 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Abel Vesa
Cc: linux-arm-msm, linux-gpio, linux-kernel, Bartosz Golaszewski
If gpio_shared_make_adev() fails for one of the references of a shared
GPIO entry, the auxiliary devices already registered for the preceding
references are never removed. Similarly, when setting up an entry fails,
the entries that were successfully set up before it keep their requested
descriptors and proxy devices, as the caller does not call
gpio_device_teardown_shared() when gpiochip_setup_shared() fails.
Make gpiochip_setup_shared() all-or-nothing. Have
gpio_shared_setup_entry() undo its own partial work: free the descriptor
and remove only the auxiliary devices of the references that precede the
failed one, so that no device is removed that was never registered. On
failure of an entry, tear down the previously completed entries in
reverse order using the same helper as the regular removal path.
Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/gpiolib-shared.c | 35 ++++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 47b434db72f1ebd35ec36ebdb35f30809f4522f2..043d5daba7a73062151a089b3ee8ccdac726f221 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -589,13 +589,25 @@ static int gpio_shared_setup_entry(struct gpio_chip *gc,
ref->con_id ?: "(none)");
ret = gpio_shared_make_adev(gdev, entry, ref);
- if (ret) {
- gpiod_free_commit(desc);
- return ret;
- }
+ if (ret)
+ goto err_out;
}
return 0;
+
+err_out:
+ gpiod_free_commit(desc);
+ list_for_each_entry_continue_reverse(ref, &entry->refs, list)
+ gpio_shared_remove_adev(&ref->adev);
+
+ return ret;
+}
+
+static bool gpio_shared_entry_needs_setup(struct gpio_device *gdev,
+ struct gpio_shared_entry *entry)
+{
+ return device_match_fwnode(&gdev->dev, entry->fwnode) &&
+ list_count_nodes(&entry->refs) > 1;
}
int gpiochip_setup_shared(struct gpio_chip *gc)
@@ -624,18 +636,23 @@ int gpiochip_setup_shared(struct gpio_chip *gc)
* exposing shared pins. Find them and create the proxy devices.
*/
list_for_each_entry(entry, &gpio_shared_list, list) {
- if (!device_match_fwnode(&gdev->dev, entry->fwnode))
- continue;
-
- if (list_count_nodes(&entry->refs) <= 1)
+ if (!gpio_shared_entry_needs_setup(gdev, entry))
continue;
ret = gpio_shared_setup_entry(gc, entry);
if (ret)
- return ret;
+ goto err_unwind;
}
return 0;
+
+err_unwind:
+ list_for_each_entry_continue_reverse(entry, &gpio_shared_list, list) {
+ if (gpio_shared_entry_needs_setup(gdev, entry))
+ gpio_shared_teardown_entry(gdev, entry);
+ }
+
+ return ret;
}
void gpio_device_teardown_shared(struct gpio_device *gdev)
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v6 4/5] gpio: shared: detach the proxy device from struct gpio_shared_ref
2026-10-21 14:23 [PATCH v6 0/5] gpio: shared: replace platform lookup with dummy software nodes Bartosz Golaszewski
` (2 preceding siblings ...)
2026-10-21 14:24 ` [PATCH v6 3/5] gpio: shared: unwind partial failures in gpiochip_setup_shared() Bartosz Golaszewski
@ 2026-10-21 14:24 ` Bartosz Golaszewski
2026-10-21 14:43 ` sashiko-bot
2026-10-21 14:24 ` [PATCH v6 5/5] gpio: shared: convert to using software nodes for GPIO shared proxy lookup Bartosz Golaszewski
4 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-10-21 14:24 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Abel Vesa
Cc: linux-arm-msm, linux-gpio, linux-kernel, Bartosz Golaszewski
Sashiko reported a pre-existing issue with the reference-counted
auxiliary device being embedded in struct gpio_shared_ref. We
unconditionally memset() the struct when setting up the GPIO shared
proxy parent while there may still be references to it held by users.
While in practice this would never happen and sashiko seems to have
confused the parent auxiliary device and the separately allocated struct
device embedded in struct gpio_device, it's still a bad pattern and
could lead to memory corruption if a user deliberately took reference to
the auxiliary device via one of the lookup functions and kept it over
the GPIO parent's teardown.
Move the auxiliary device struct out of the shared reference structure
and allocate its memory separately on proxy setup.
Since we're already touching that field: change the name to something
more meaningful: it's more important to point out this is the proxy
device instead of indicating its type in the name.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/gpiolib-shared.c | 63 +++++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 26 deletions(-)
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 043d5daba7a73062151a089b3ee8ccdac726f221..3ed25609cae10081a0ab5276ebcd47aa3c49f76a 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -35,11 +35,10 @@ struct gpio_shared_ref {
/* GPIO flags this consumer uses for the request. */
enum gpiod_flags flags;
char *con_id;
- int dev_id;
/* Protects the auxiliary device struct and the lookup table. */
struct mutex lock;
struct lock_class_key lock_key;
- struct auxiliary_device adev;
+ struct auxiliary_device *proxy_dev;
struct gpiod_lookup_table *lookup;
bool is_reset_gpio;
};
@@ -94,10 +93,6 @@ static struct gpio_shared_ref *gpio_shared_make_ref(struct fwnode_handle *fwnode
return NULL;
}
- ref->dev_id = ida_alloc(&gpio_shared_ida, GFP_KERNEL);
- if (ref->dev_id < 0)
- return NULL;
-
ref->flags = flags;
ref->con_id = no_free_ptr(con_id_cpy);
ref->fwnode = fwnode;
@@ -306,31 +301,43 @@ static int gpio_shared_of_scan(void)
}
#endif /* CONFIG_OF */
-static void gpio_shared_adev_release(struct device *dev)
+static void gpio_shared_proxy_dev_release(struct device *dev)
{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+ ida_free(&gpio_shared_ida, adev->id);
+ kfree(adev);
}
-static int gpio_shared_make_adev(struct gpio_device *gdev,
- struct gpio_shared_entry *entry,
- struct gpio_shared_ref *ref)
+static int gpio_shared_make_proxy_dev(struct gpio_device *gdev,
+ struct gpio_shared_entry *entry,
+ struct gpio_shared_ref *ref)
{
- struct auxiliary_device *adev = &ref->adev;
- int ret;
+ struct auxiliary_device *adev;
+ int ret, id;
- guard(mutex)(&ref->lock);
+ adev = kzalloc_obj(*adev);
+ if (!adev)
+ return -ENOMEM;
- memset(adev, 0, sizeof(*adev));
+ id = ida_alloc(&gpio_shared_ida, GFP_KERNEL);
+ if (id < 0) {
+ kfree(adev);
+ return id;
+ }
- adev->id = ref->dev_id;
+ adev->id = id;
adev->name = "proxy";
adev->dev.parent = gdev->dev.parent;
adev->dev.platform_data = entry;
- adev->dev.release = gpio_shared_adev_release;
+ adev->dev.release = gpio_shared_proxy_dev_release;
ret = auxiliary_device_init(adev);
- if (ret)
+ if (ret) {
+ ida_free(&gpio_shared_ida, id);
+ kfree(adev);
return ret;
+ }
ret = auxiliary_device_add(adev);
if (ret) {
@@ -341,6 +348,10 @@ static int gpio_shared_make_adev(struct gpio_device *gdev,
pr_debug("Created an auxiliary GPIO proxy %s for GPIO device %s\n",
dev_name(&adev->dev), gpio_device_get_label(gdev));
+ scoped_guard(mutex, &ref->lock) {
+ ref->proxy_dev = adev;
+ }
+
return 0;
}
@@ -480,7 +491,7 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, struct fwnode_handle *
char *key __free(kfree) =
kasprintf(GFP_KERNEL,
KBUILD_MODNAME ".proxy.%u",
- ref->adev.id);
+ ref->proxy_dev->id);
if (!key)
return -ENOMEM;
@@ -511,10 +522,11 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, struct fwnode_handle *
return -ENOENT;
}
-static void gpio_shared_remove_adev(struct auxiliary_device *adev)
+static void gpio_shared_remove_proxy_dev(struct gpio_shared_ref *ref)
{
- auxiliary_device_delete(adev);
- auxiliary_device_uninit(adev);
+ auxiliary_device_delete(ref->proxy_dev);
+ auxiliary_device_uninit(ref->proxy_dev);
+ ref->proxy_dev = NULL;
}
static void gpio_shared_teardown_entry(struct gpio_device *gdev,
@@ -534,7 +546,7 @@ static void gpio_shared_teardown_entry(struct gpio_device *gdev,
ref->lookup = NULL;
}
- gpio_shared_remove_adev(&ref->adev);
+ gpio_shared_remove_proxy_dev(ref);
}
}
@@ -588,7 +600,7 @@ static int gpio_shared_setup_entry(struct gpio_chip *gc,
fwnode_get_name(ref->fwnode) ?: "(no fwnode)",
ref->con_id ?: "(none)");
- ret = gpio_shared_make_adev(gdev, entry, ref);
+ ret = gpio_shared_make_proxy_dev(gdev, entry, ref);
if (ret)
goto err_out;
}
@@ -598,7 +610,7 @@ static int gpio_shared_setup_entry(struct gpio_chip *gc,
err_out:
gpiod_free_commit(desc);
list_for_each_entry_continue_reverse(ref, &entry->refs, list)
- gpio_shared_remove_adev(&ref->adev);
+ gpio_shared_remove_proxy_dev(ref);
return ret;
}
@@ -619,7 +631,7 @@ int gpiochip_setup_shared(struct gpio_chip *gc)
list_for_each_entry(entry, &gpio_shared_list, list) {
list_for_each_entry(ref, &entry->refs, list) {
- if (gdev->dev.parent == &ref->adev.dev) {
+ if (ref->proxy_dev && gdev->dev.parent == &ref->proxy_dev->dev) {
/*
* This is a shared GPIO proxy. Mark its
* descriptor as such and return here.
@@ -756,7 +768,6 @@ static void gpio_shared_drop_ref(struct gpio_shared_ref *ref)
mutex_destroy(&ref->lock);
lockdep_unregister_key(&ref->lock_key);
kfree(ref->con_id);
- ida_free(&gpio_shared_ida, ref->dev_id);
fwnode_handle_put(ref->fwnode);
kfree(ref);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v6 4/5] gpio: shared: detach the proxy device from struct gpio_shared_ref
2026-10-21 14:24 ` [PATCH v6 4/5] gpio: shared: detach the proxy device from struct gpio_shared_ref Bartosz Golaszewski
@ 2026-10-21 14:43 ` sashiko-bot
0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-10-21 14:43 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: linux-gpio
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: shared: unlocked proxy_dev teardown in gpio_shared_remove_proxy_dev()
--
commit 744026242eec2478ed3cf7962883c73cf7ffb5cd
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
gpio: shared: detach the proxy device from struct gpio_shared_ref
This commit moves the auxiliary device out of struct gpio_shared_ref in
drivers/gpio/gpiolib-shared.c and allocates it dynamically as proxy_dev
when setting up a shared GPIO proxy device.
> diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
> index 043d5daba7a73..3ed25609cae10 100644
> --- a/drivers/gpio/gpiolib-shared.c
> +++ b/drivers/gpio/gpiolib-shared.c
> @@ -35,11 +35,10 @@ struct gpio_shared_ref {
> /* GPIO flags this consumer uses for the request. */
> enum gpiod_flags flags;
> char *con_id;
> - int dev_id;
> /* Protects the auxiliary device struct and the lookup table. */
> struct mutex lock;
> struct lock_class_key lock_key;
> - struct auxiliary_device adev;
> + struct auxiliary_device *proxy_dev;
> struct gpiod_lookup_table *lookup;
> bool is_reset_gpio;
> };
[ ... ]
> @@ -306,31 +301,43 @@ static int gpio_shared_of_scan(void)
> }
> #endif /* CONFIG_OF */
>
> -static void gpio_shared_adev_release(struct device *dev)
> +static void gpio_shared_proxy_dev_release(struct device *dev)
> {
> + struct auxiliary_device *adev = to_auxiliary_dev(dev);
>
> + ida_free(&gpio_shared_ida, adev->id);
> + kfree(adev);
> }
>
> -static int gpio_shared_make_adev(struct gpio_device *gdev,
> - struct gpio_shared_entry *entry,
> - struct gpio_shared_ref *ref)
> +static int gpio_shared_make_proxy_dev(struct gpio_device *gdev,
> + struct gpio_shared_entry *entry,
> + struct gpio_shared_ref *ref)
> {
> - struct auxiliary_device *adev = &ref->adev;
> - int ret;
> + struct auxiliary_device *adev;
> + int ret, id;
>
> - guard(mutex)(&ref->lock);
> + adev = kzalloc_obj(*adev);
> + if (!adev)
> + return -ENOMEM;
[ ... ]
> pr_debug("Created an auxiliary GPIO proxy %s for GPIO device %s\n",
> dev_name(&adev->dev), gpio_device_get_label(gdev));
>
> + scoped_guard(mutex, &ref->lock) {
> + ref->proxy_dev = adev;
> + }
> +
> return 0;
> }
>
> @@ -480,7 +491,7 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, struct fwnode_handle *
> char *key __free(kfree) =
> kasprintf(GFP_KERNEL,
> KBUILD_MODNAME ".proxy.%u",
> - ref->adev.id);
> + ref->proxy_dev->id);
> if (!key)
> return -ENOMEM;
>
> @@ -511,10 +522,11 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, struct fwnode_handle *
> return -ENOENT;
> }
>
> -static void gpio_shared_remove_adev(struct auxiliary_device *adev)
> +static void gpio_shared_remove_proxy_dev(struct gpio_shared_ref *ref)
> {
> - auxiliary_device_delete(adev);
> - auxiliary_device_uninit(adev);
> + auxiliary_device_delete(ref->proxy_dev);
> + auxiliary_device_uninit(ref->proxy_dev);
> + ref->proxy_dev = NULL;
> }
[Severity: High]
Can tearing down ref->proxy_dev in gpio_shared_remove_proxy_dev() without
holding ref->lock race with concurrent readers and cause a use-after-free or
NULL pointer dereference?
In drivers/gpio/gpiolib-shared.c, struct gpio_shared_ref documents that
ref->lock protects the auxiliary device and the lookup table, and
gpio_shared_make_proxy_dev() assigns ref->proxy_dev under ref->lock.
However, gpio_shared_remove_proxy_dev() and gpio_shared_teardown_entry() do
not take ref->lock, and auxiliary_device_uninit(ref->proxy_dev) drops the
final reference and frees ref->proxy_dev via gpio_shared_proxy_dev_release()
before ref->proxy_dev is set to NULL.
In gpiochip_add_data_with_key(), gpiochip_setup_dev() calls gcdev_register()
while gdev is already visible on gpio_devices. If gpiochip_sysfs_register()
subsequently fails, the error path runs:
gpiochip_add_data_with_key()
-> gpio_device_teardown_shared()
-> gpio_shared_teardown_entry()
-> gpio_shared_remove_proxy_dev()
before gdev is removed from gpio_devices and synchronize_srcu() is called
(and gpiod_free_commit() leaves GPIOD_FLAG_SHARED set on the descriptor).
A concurrent consumer in gpiod_find_and_request() that looked up the
descriptor under gpio_devices_srcu can call gpio_shared_add_proxy_lookup(),
take ref->lock, and dereference ref->proxy_dev->id while
gpio_shared_remove_proxy_dev() is freeing ref->proxy_dev or after it has set
ref->proxy_dev to NULL.
Similarly, if a consumer node references multiple shared GPIOs with the same
fwnode and con_id across two GPIO controllers, requesting the pin on one
registered controller calls gpio_shared_add_proxy_lookup(), which walks
gpio_shared_list and can match the ref belonging to the second controller
while its ref->proxy_dev is still NULL or being torn down in
gpio_shared_remove_proxy_dev().
Concurrent gpiochip_setup_shared() calls for other controllers also walk
gpio_shared_list without ref->lock:
list_for_each_entry(entry, &gpio_shared_list, list) {
list_for_each_entry(ref, &entry->refs, list) {
if (ref->proxy_dev && gdev->dev.parent == &ref->proxy_dev->dev) {
which can read a non-NULL ref->proxy_dev pointer after
auxiliary_device_uninit() has already freed the structure (or after the slab
slot has been reallocated).
Note that the follow-up commit ("gpio: shared: convert to using software
nodes for GPIO shared proxy lookup") still leaves
gpio_shared_remove_proxy_dev() unlocked and frees ref->proxy_dev and
unregisters ref->proxy_swnode before clearing ref->proxy_dev and
ref->proxy_fwnode to NULL.
Should gpio_shared_remove_proxy_dev() and the readers of ref->proxy_dev
serialize access with ref->lock and check for a NULL ref->proxy_dev?
>
> static void gpio_shared_teardown_entry(struct gpio_device *gdev,
> @@ -534,7 +546,7 @@ static void gpio_shared_teardown_entry(struct gpio_device *gdev,
> ref->lookup = NULL;
> }
>
> - gpio_shared_remove_adev(&ref->adev);
> + gpio_shared_remove_proxy_dev(ref);
> }
> }
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021-gpio-shared-fwnode-lookup-v6-0-6af7d8eefd25@oss.qualcomm.com?part=4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v6 5/5] gpio: shared: convert to using software nodes for GPIO shared proxy lookup
2026-10-21 14:23 [PATCH v6 0/5] gpio: shared: replace platform lookup with dummy software nodes Bartosz Golaszewski
` (3 preceding siblings ...)
2026-10-21 14:24 ` [PATCH v6 4/5] gpio: shared: detach the proxy device from struct gpio_shared_ref Bartosz Golaszewski
@ 2026-10-21 14:24 ` Bartosz Golaszewski
2026-10-21 14:59 ` sashiko-bot
4 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-10-21 14:24 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Abel Vesa
Cc: linux-arm-msm, linux-gpio, linux-kernel, Bartosz Golaszewski
Automated management of shared GPIOs only allows to request shared pins
when they're assigned to a real struct device. Firmware node, no-device
lookup currently isn't supported and can't be because we're using
platform lookup tables to point the callers to the shared proxy GPIO
controllers.
Rework the GPIO sharing by using dummy software nodes assigned to the
proxy devices and stored in the reference structures which allow us -
once the correct struct gpio_shared_ref is found - to find the correct
target GPIO device by its firmware node.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/gpiolib-shared.c | 117 ++++++++++++++++++++----------------------
drivers/gpio/gpiolib-shared.h | 15 +++---
drivers/gpio/gpiolib.c | 20 +-------
3 files changed, 65 insertions(+), 87 deletions(-)
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 3ed25609cae10081a0ab5276ebcd47aa3c49f76a..71f16105f7a5c51f657ccf60569209b3d819ade2 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -35,11 +35,12 @@ struct gpio_shared_ref {
/* GPIO flags this consumer uses for the request. */
enum gpiod_flags flags;
char *con_id;
- /* Protects the auxiliary device struct and the lookup table. */
+ /* Protects the auxiliary device struct and fwnode. */
struct mutex lock;
struct lock_class_key lock_key;
struct auxiliary_device *proxy_dev;
- struct gpiod_lookup_table *lookup;
+ struct software_node proxy_swnode;
+ struct fwnode_handle *proxy_fwnode;
bool is_reset_gpio;
};
@@ -313,35 +314,50 @@ static int gpio_shared_make_proxy_dev(struct gpio_device *gdev,
struct gpio_shared_entry *entry,
struct gpio_shared_ref *ref)
{
+ struct software_node *swnode = &ref->proxy_swnode;
struct auxiliary_device *adev;
+ struct fwnode_handle *fwnode;
+ struct device *dev;
int ret, id;
adev = kzalloc_obj(*adev);
if (!adev)
return -ENOMEM;
+
id = ida_alloc(&gpio_shared_ida, GFP_KERNEL);
if (id < 0) {
- kfree(adev);
- return id;
+ ret = id;
+ goto err_free_adev;
}
+ ret = software_node_register(swnode);
+ if (ret)
+ goto err_free_ida;
+
+ fwnode = software_node_fwnode(swnode);
+ if (!fwnode) {
+ ret = -ENODEV;
+ goto err_swnode_unreg;
+ }
+
+ dev = &adev->dev;
adev->id = id;
adev->name = "proxy";
- adev->dev.parent = gdev->dev.parent;
- adev->dev.platform_data = entry;
- adev->dev.release = gpio_shared_proxy_dev_release;
+ dev->parent = gdev->dev.parent;
+ dev->platform_data = entry;
+ dev->release = gpio_shared_proxy_dev_release;
+
+ device_set_node(dev, software_node_fwnode(swnode));
ret = auxiliary_device_init(adev);
- if (ret) {
- ida_free(&gpio_shared_ida, id);
- kfree(adev);
- return ret;
- }
+ if (ret)
+ goto err_swnode_unreg;
ret = auxiliary_device_add(adev);
if (ret) {
auxiliary_device_uninit(adev);
+ software_node_unregister(swnode);
return ret;
}
@@ -350,9 +366,19 @@ static int gpio_shared_make_proxy_dev(struct gpio_device *gdev,
scoped_guard(mutex, &ref->lock) {
ref->proxy_dev = adev;
+ ref->proxy_fwnode = fwnode;
}
return 0;
+
+err_swnode_unreg:
+ software_node_unregister(swnode);
+err_free_ida:
+ ida_free(&gpio_shared_ida, id);
+err_free_adev:
+ kfree(adev);
+
+ return ret;
}
#if IS_ENABLED(CONFIG_RESET_GPIO)
@@ -457,23 +483,19 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer,
}
#endif /* CONFIG_RESET_GPIO */
-int gpio_shared_add_proxy_lookup(struct device *consumer, struct fwnode_handle *fwnode,
- const char *con_id, unsigned long lflags)
+struct gpio_desc *
+gpio_shared_get_proxy(struct device *consumer, struct fwnode_handle *fwnode,
+ const char *con_id)
{
struct gpio_shared_entry *entry;
struct gpio_shared_ref *ref;
- const char *dev_id;
-
- if (!consumer)
- return -EOPNOTSUPP;
-
- dev_id = dev_name(consumer);
list_for_each_entry(entry, &gpio_shared_list, list) {
list_for_each_entry(ref, &entry->refs, list) {
guard(mutex)(&ref->lock);
- if (!ref->fwnode && device_is_compatible(consumer, "reset-gpio")) {
+ if (!ref->fwnode && consumer &&
+ device_is_compatible(consumer, "reset-gpio")) {
if (!gpio_shared_dev_is_reset_gpio(consumer, entry, ref))
continue;
} else if (fwnode != ref->fwnode) {
@@ -484,49 +506,31 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, struct fwnode_handle *
(con_id && ref->con_id && strcmp(con_id, ref->con_id) != 0))
continue;
- /* We've already done that on a previous request. */
- if (ref->lookup)
- return 0;
-
- char *key __free(kfree) =
- kasprintf(GFP_KERNEL,
- KBUILD_MODNAME ".proxy.%u",
- ref->proxy_dev->id);
- if (!key)
- return -ENOMEM;
-
- struct gpiod_lookup_table *lookup __free(kfree) =
- kzalloc_flex(*lookup, table, 2);
- if (!lookup)
- return -ENOMEM;
-
- pr_debug("Adding machine lookup entry for a shared GPIO for consumer %s, with key '%s' and con_id '%s'\n",
- dev_id, key, ref->con_id ?: "none");
-
- lookup->dev_id = kstrdup(dev_id, GFP_KERNEL);
- if (!lookup->dev_id)
- return -ENOMEM;
-
- lookup->table[0] = GPIO_LOOKUP(no_free_ptr(key), 0,
- ref->con_id, lflags);
-
- ref->lookup = no_free_ptr(lookup);
- gpiod_add_lookup_table(ref->lookup);
+ struct gpio_device *gdev __free(gpio_device_put) =
+ gpio_device_find_by_fwnode(ref->proxy_fwnode);
+ if (!gdev)
+ /*
+ * A matching entry is there but the associated
+ * controller has not been registered yet.
+ */
+ return ERR_PTR(-EPROBE_DEFER);
- return 0;
+ return gpio_device_get_desc(gdev, 0);
}
}
/* We warn here because this can only happen if the programmer borked. */
WARN_ON(1);
- return -ENOENT;
+ return ERR_PTR(-ENOENT);
}
static void gpio_shared_remove_proxy_dev(struct gpio_shared_ref *ref)
{
auxiliary_device_delete(ref->proxy_dev);
auxiliary_device_uninit(ref->proxy_dev);
+ software_node_unregister(&ref->proxy_swnode);
ref->proxy_dev = NULL;
+ ref->proxy_fwnode = NULL;
}
static void gpio_shared_teardown_entry(struct gpio_device *gdev,
@@ -537,17 +541,8 @@ static void gpio_shared_teardown_entry(struct gpio_device *gdev,
scoped_guard(mutex, &entry->lock)
gpiod_free_commit(&gdev->descs[entry->offset]);
- list_for_each_entry(ref, &entry->refs, list) {
- if (ref->lookup) {
- gpiod_remove_lookup_table(ref->lookup);
- kfree(ref->lookup->table[0].key);
- kfree(ref->lookup->dev_id);
- kfree(ref->lookup);
- ref->lookup = NULL;
- }
-
+ list_for_each_entry(ref, &entry->refs, list)
gpio_shared_remove_proxy_dev(ref);
- }
}
static int gpio_shared_setup_entry(struct gpio_chip *gc,
diff --git a/drivers/gpio/gpiolib-shared.h b/drivers/gpio/gpiolib-shared.h
index 618756f6c6aafd087df267a4a32732ccc03af38e..27fd2a40c5cb77a19c258e3fbf429d3f6e7d4b32 100644
--- a/drivers/gpio/gpiolib-shared.h
+++ b/drivers/gpio/gpiolib-shared.h
@@ -14,9 +14,9 @@ struct fwnode_handle;
int gpiochip_setup_shared(struct gpio_chip *gc);
void gpio_device_teardown_shared(struct gpio_device *gdev);
-int gpio_shared_add_proxy_lookup(struct device *consumer,
- struct fwnode_handle *fwnode,
- const char *con_id, unsigned long lflags);
+struct gpio_desc *
+gpio_shared_get_proxy(struct device *consumer, struct fwnode_handle *fwnode,
+ const char *con_id);
#else
@@ -27,12 +27,11 @@ static inline int gpiochip_setup_shared(struct gpio_chip *gc)
static inline void gpio_device_teardown_shared(struct gpio_device *gdev) { }
-static inline int gpio_shared_add_proxy_lookup(struct device *consumer,
- struct fwnode_handle *fwnode,
- const char *con_id,
- unsigned long lflags)
+static inline struct gpio_desc *
+gpio_shared_get_proxy(struct device *consumer, struct fwnode_handle *fwnode,
+ const char *con_id)
{
- return 0;
+ return NULL;
}
#endif /* CONFIG_GPIO_SHARED */
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c7f6e4c4055f405562fa1d0dbad181af508273da..409fea3720a6e6c7f173b835f1aa5b747a4495b0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4832,24 +4832,8 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
scoped_guard(srcu, &gpio_devices_srcu) {
desc = gpiod_fwnode_lookup(fwnode, consumer, con_id, idx,
&flags, &lookupflags);
- if (!IS_ERR_OR_NULL(desc) &&
- test_bit(GPIOD_FLAG_SHARED, &desc->flags)) {
- /*
- * We're dealing with a GPIO shared by multiple
- * consumers. This is the moment to add the machine
- * lookup table for the proxy device as previously
- * we only knew the consumer's fwnode.
- */
- ret = gpio_shared_add_proxy_lookup(consumer, fwnode,
- con_id, lookupflags);
- if (ret)
- return ERR_PTR(ret);
-
- /* Trigger platform lookup for shared GPIO proxy. */
- desc = ERR_PTR(-ENOENT);
- /* Trigger it even for fwnode-only gpiod_get(). */
- platform_lookup_allowed = true;
- }
+ if (!IS_ERR_OR_NULL(desc) && test_bit(GPIOD_FLAG_SHARED, &desc->flags))
+ desc = gpio_shared_get_proxy(consumer, fwnode, con_id);
if (gpiod_not_found(desc) && platform_lookup_allowed) {
/*
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v6 5/5] gpio: shared: convert to using software nodes for GPIO shared proxy lookup
2026-10-21 14:24 ` [PATCH v6 5/5] gpio: shared: convert to using software nodes for GPIO shared proxy lookup Bartosz Golaszewski
@ 2026-10-21 14:59 ` sashiko-bot
0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-10-21 14:59 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: linux-gpio
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpio: shared: false-positive NULL fwnode match in gpio_shared_get_proxy()
- [High] gpio: shared: unlocked proxy teardown and NULL fwnode match in lookup
--
commit c9587b1290500bb02f98bc321f73e9b020833797
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
gpio: shared: convert to using software nodes for GPIO shared proxy lookup
This commit replaces platform lookup tables with software nodes for shared
GPIO proxy devices so that shared GPIOs can be looked up via their proxy
device firmware node in gpio_shared_get_proxy().
> diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
> index 3ed25609cae10..71f16105f7a5c 100644
> --- a/drivers/gpio/gpiolib-shared.c
> +++ b/drivers/gpio/gpiolib-shared.c
[ ... ]
> @@ -457,23 +483,19 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer,
> }
> #endif /* CONFIG_RESET_GPIO */
>
> -int gpio_shared_add_proxy_lookup(struct device *consumer, struct fwnode_handle *fwnode,
> - const char *con_id, unsigned long lflags)
> +struct gpio_desc *
> +gpio_shared_get_proxy(struct device *consumer, struct fwnode_handle *fwnode,
> + const char *con_id)
> {
> struct gpio_shared_entry *entry;
> struct gpio_shared_ref *ref;
> - const char *dev_id;
> -
> - if (!consumer)
> - return -EOPNOTSUPP;
> -
> - dev_id = dev_name(consumer);
>
> list_for_each_entry(entry, &gpio_shared_list, list) {
> list_for_each_entry(ref, &entry->refs, list) {
> guard(mutex)(&ref->lock);
>
> - if (!ref->fwnode && device_is_compatible(consumer, "reset-gpio")) {
> + if (!ref->fwnode && consumer &&
> + device_is_compatible(consumer, "reset-gpio")) {
> if (!gpio_shared_dev_is_reset_gpio(consumer, entry, ref))
> continue;
> } else if (fwnode != ref->fwnode) {
> @@ -484,49 +506,31 @@ int gpio_shared_add_proxy_lookup(struct device *consumer, struct fwnode_handle *
> (con_id && ref->con_id && strcmp(con_id, ref->con_id) != 0))
> continue;
>
> - /* We've already done that on a previous request. */
> - if (ref->lookup)
> - return 0;
> -
> - char *key __free(kfree) =
> - kasprintf(GFP_KERNEL,
> - KBUILD_MODNAME ".proxy.%u",
> - ref->proxy_dev->id);
> - if (!key)
> - return -ENOMEM;
> -
> - struct gpiod_lookup_table *lookup __free(kfree) =
> - kzalloc_flex(*lookup, table, 2);
> - if (!lookup)
> - return -ENOMEM;
> -
> - pr_debug("Adding machine lookup entry for a shared GPIO for consumer %s, with key '%s' and con_id '%s'\n",
> - dev_id, key, ref->con_id ?: "none");
> -
> - lookup->dev_id = kstrdup(dev_id, GFP_KERNEL);
> - if (!lookup->dev_id)
> - return -ENOMEM;
> -
> - lookup->table[0] = GPIO_LOOKUP(no_free_ptr(key), 0,
> - ref->con_id, lflags);
> -
> - ref->lookup = no_free_ptr(lookup);
> - gpiod_add_lookup_table(ref->lookup);
> + struct gpio_device *gdev __free(gpio_device_put) =
> + gpio_device_find_by_fwnode(ref->proxy_fwnode);
> + if (!gdev)
> + /*
> + * A matching entry is there but the associated
> + * controller has not been registered yet.
> + */
> + return ERR_PTR(-EPROBE_DEFER);
>
> - return 0;
> + return gpio_device_get_desc(gdev, 0);
[Severity: High]
What happens in gpio_shared_get_proxy() if ref->proxy_fwnode is null when
passed to gpio_device_find_by_fwnode()?
In drivers/gpio/gpiolib.c, gpio_device_find_by_fwnode() uses
gpio_chip_match_by_fwnode():
static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
{
struct device *dev = &gc->gpiodev->dev;
struct fwnode_handle *node = dev_fwnode(dev);
if (IS_ERR(fwnode))
return 0;
if (device_match_fwnode(dev, fwnode))
return 1;
return node && node->secondary == fwnode;
}
When fwnode is null, both IS_ERR(fwnode) and device_match_fwnode(dev, fwnode)
return 0, so the final check evaluates to:
node && node->secondary == NULL
This matches the first registered gpio_device that has a primary fwnode and
no secondary fwnode, causing gpio_shared_get_proxy() to return descriptor 0
of an unrelated controller instead of returning ERR_PTR(-EPROBE_DEFER).
Because gpio_shared_get_proxy() scans gpio_shared_list matching only
(consumer, fwnode, con_id) without checking that the entry matches the
descriptor or index returned by gpiod_fwnode_lookup(), this can be triggered
when a consumer has a multi-element GPIO property scanned by
gpio_shared_of_traverse(): if the second controller is registered while the
first controller in gpio_shared_list is not yet registered, requesting the
second GPIO matches the first entry where ref->proxy_fwnode is still null.
It can also happen if gpiod_find_and_request() resolves desc in
gpiod_fwnode_lookup() under gpio_devices_srcu while
gpio_device_teardown_shared() -> gpio_shared_remove_proxy_dev() clears
ref->proxy_fwnode.
Should gpio_shared_get_proxy() verify that the entry matches the looked-up
descriptor (or index) and check that ref->proxy_fwnode is non-null before
calling gpio_device_find_by_fwnode()?
> }
> }
>
> /* We warn here because this can only happen if the programmer borked. */
> WARN_ON(1);
> - return -ENOENT;
> + return ERR_PTR(-ENOENT);
> }
>
> static void gpio_shared_remove_proxy_dev(struct gpio_shared_ref *ref)
> {
> auxiliary_device_delete(ref->proxy_dev);
> auxiliary_device_uninit(ref->proxy_dev);
> + software_node_unregister(&ref->proxy_swnode);
> ref->proxy_dev = NULL;
> + ref->proxy_fwnode = NULL;
> }
[Severity: High]
Could gpio_shared_remove_proxy_dev() race with concurrent readers of
ref->proxy_dev and ref->proxy_fwnode?
In struct gpio_shared_ref, ref->lock is documented as protecting proxy_dev
and proxy_fwnode, and both gpio_shared_make_proxy_dev() and
gpio_shared_get_proxy() acquire ref->lock.
However, gpio_shared_remove_proxy_dev() does not acquire ref->lock, and it
destroys ref->proxy_dev and unregisters ref->proxy_swnode (freeing the
backing swnode for ref->proxy_fwnode) before clearing ref->proxy_dev and
ref->proxy_fwnode.
Similarly, gpiochip_setup_shared() reads ref->proxy_dev without holding
ref->lock:
list_for_each_entry(entry, &gpio_shared_list, list) {
list_for_each_entry(ref, &entry->refs, list) {
if (ref->proxy_dev && gdev->dev.parent == &ref->proxy_dev->dev) {
This appears reachable in several ways:
1. In drivers/gpio/gpiolib.c:gpiochip_add_data_with_key(), if
gpiochip_setup_dev() fails after gcdev_register() has registered gdev->dev,
err_teardown_shared runs gpio_device_teardown_shared() ->
gpio_shared_remove_proxy_dev() before err_remove_from_list removes gdev from
gpio_devices and calls synchronize_srcu(&gpio_devices_srcu):
if (gpiolib_initialized) {
ret = gpiochip_setup_dev(gc);
if (ret)
goto err_teardown_shared;
}
return 0;
err_teardown_shared:
gpio_device_teardown_shared(gdev);
...
err_remove_from_list:
scoped_guard(mutex, &gpio_devices_lock)
list_del_rcu(&gdev->list);
synchronize_srcu(&gpio_devices_srcu);
2. A consumer with multiple shared GPIOs under the same con_id can call
gpio_shared_get_proxy() and visit an entry whose controller is concurrently
being torn down in gpiochip_remove().
3. Another GPIO chip can concurrently run gpiochip_setup_shared() while a
shared GPIO controller is being torn down or unwinding a failed setup.
Should gpio_shared_remove_proxy_dev() and gpiochip_setup_shared() guard
accesses to ref->proxy_dev and ref->proxy_fwnode with ref->lock, and clear
the pointers before releasing the underlying objects?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021-gpio-shared-fwnode-lookup-v6-0-6af7d8eefd25@oss.qualcomm.com?part=5
^ permalink raw reply [flat|nested] 8+ messages in thread