* [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space
@ 2026-10-12 13:49 Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu Adrian Ambrozewicz
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 13:49 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij
Cc: Bartosz Golaszewski, Kent Gibson, Paul E. McKenney, Herve Codina,
Jia Jia, Andy Shevchenko, linux-gpio, linux-kernel
Removing a GPIO chip while user space keeps a line request open, which
is what happens when a USB or otherwise hot-pluggable GPIO controller
goes away, leaves a few things behind on current kernels:
- the consumer label of each such line leaks (patch 2), and freeing it
just before the GPIO device is released needs gpiodev_release() to
wait for pending label frees (patch 1, which can also be hit on its
own with line hogs);
- the character device frees the line's IRQ a second time on close,
after gpiochip_remove() already did, which warns when the IRQ number
was reused and was reported as a use-after-free with virtio-gpio
(patch 3), and its IRQs are freed in an order that still lets a
reused IRQ number or a concurrent close() get in the way (patch 6);
- a process blocked in read() on the request keeps gpiochip_remove()
waiting in synchronize_srcu() until it is signalled, and with it
device_shutdown() on power-off (patch 4);
- the IRQ name the character device allocated leaks (patch 5).
All five were reproduced with gpio-sim on v7.3-rc5 under QEMU (x86_64,
4 vCPUs) and are still present in gpio/for-next. The loop: create a
gpio-sim chip, request a line through the character device and hold
it, set "live" to 0 while it is held, then optionally set the chip up
again and request the same line, and close the first request.
v7.3-rc5 (30 cycles) with the series (100)
v2 edges, reuse 30 WARN, 17-30 labels 0 WARN, 0 leaks
v2 edges 0 WARN, 19-29 labels 0
v2 sw debounce 30 WARN, 13-30 labels 0
v1 line event 30 WARN, 14-29 labels 0
v2 no edges 15-30 labels 0
v2 edges, consumer 30 WARN, 15-30 labels, 0
17-24 IRQ names
blocked read() removal waits for the removal completes,
(v2, v1, info watch) reader to be killed read() gets -ENODEV
The counts above are from both a KASAN and a non-KASAN build; KASAN did
not report anything in these runs, the slab use-after-free from the
report needs the descriptor free to race with the close. kmemleak does
not see every leaked object, hence the ranges. The desc_srcu warning
fixed by patch 1 takes srcutree.convert_to_big=1 to show up within a
few dozen cycles of a hogged line (cycle 57 here); with the series,
1000 cycles pass. That warning was also seen on real hardware, on arm64
KCSAN kernels with v7.3-rc5 unbinding a USB GPIO expander whose driver
is not yet upstream, built both as an out-of-tree module and into the
kernel.
Patch 3 on its own leaves two windows, which gpio-sim does not hit: if
the chip disposes of its IRQ mappings in gpiochip_irqchip_remove() and
another device gets one of the numbers before the character device's
notifier runs, the notifier's free_irq() warns; and a close() racing
with gpiochip_remove() is not serialized against gpiod_free_irqs().
Patch 6 calls the notifier right after the chip pointer is cleared and
before gpiod_free_irqs(), and makes the release paths free their IRQs
before unregistering from it. With a test-only chip on the gpiolib
hierarchical irqchip that hands an IRQ number out again during the
removal, or sleeps in irq_bus_lock() to widen the race, 30 out of 30
cycles warn in each case with patches 1-5 and none with patch 6.
Patch 6 is kept apart from patch 3 because it changes the order of
gpiochip_remove() for in-kernel users too: interrupts they leave
requested are now freed after the chip pointer is cleared rather than
before, so the irqchip callbacks that free_irq() calls into run with
the gpiod_*() consumer calls on that chip already failing; the driver
data is still set at that point. Patch 3 is enough for the reported
use-after-free and is the smaller backport. Herve's series [1] moved the
character device's cleanup earlier in gpiochip_remove() in 2024 and was
set aside for a fix covering in-kernel users as well, which became
gpiochip_free_remaining_irqs(); patch 6 keeps that and only lets the
character device go first.
The revocable conversion posted for gpiolib [3] keeps all of these as
they are; patches 2, 4 and 6 would need rebasing over it, patch 6
because its notifier call goes right after the revocable_revoke() that
the conversion puts in gpiochip_remove().
On arm64 (QEMU virt, 4 vCPUs, KASAN), 5 cycles per case, the removal
loop gives the same picture: on v7.3-rc5 the v2 edge, debounce, v1 and
consumer cases warn in every cycle, all cases but the blocked readers
leak labels, and the blocked readers hold up the removal; with the
series none of that happens. With the test chip, the close race warns
in 10 out of 10 cycles on v7.3-rc5 for v2 and v1 alike, and in one v2
cycle KASAN reports the use-after-free of [2], free_irq() from
edge_detector_stop() on a freed irq_desc; with the series there is
neither. (The reused IRQ number only leads to a warning once patch 3
frees IRQs from the notifier, so not on v7.3-rc5.) An x86_64 KCSAN
build, default KCSAN options, shows the same warnings and stalls on
v7.3-rc5 and none with the series; KCSAN itself reported no data race
in gpiolib, also with kcsan.skip_watch=100 for the test chip and the
edge, consumer and blocked-reader cases (one report in 32 boots, in
osq_unlock() while a disk was probed during boot). The test kernels
with patch 6 also had the direction fix mentioned below applied, which
touches none of these paths.
User-visible change: with patch 4, a read() that is blocked on a line
request, on a v1 line event or on a chip file with watched lines when
the chip is removed returns the events still queued, if any, and then
fails with -ENODEV. On v7.3-rc5 it stays blocked until it is signalled
or an event arrives, holding up the removal. A read() started after the
removal already failed with -ENODEV, and poll() already reported
EPOLLHUP | EPOLLERR; neither changes.
For stable: patches 3, 5 and 6 fix commit ec8b6f55b981, which is in
6.12 and later. Patch 4 applies to 6.12.y as is; the SRCU read section
it changes replaced the gdev->sem rwsem in 6.9, and 5.15.y, 6.1.y and
6.6.y have that rwsem from the commit it fixes and need a different
change. There the read handlers hold gdev->sem for reading across the
whole read through call_read_locked(), wait included, and
gpiochip_remove() takes it for writing, so the backport has to drop the
rwsem around wait_event_interruptible_locked(), retake it and check
gdev->chip again, with !gdev->chip added to the wait condition. On
6.6.y the device notifier, called with the rwsem held for writing, then
wakes the reader, which gets -ENODEV once the removal drops the rwsem.
5.15.y and 6.1.y have no device notifier: the removal no longer waits,
but a blocked reader is only woken by an event or a signal, as before.
Two fixes found along the way have nothing to do with chip removal and
are sent on their own: one for the line info keeping a direction that
the driver refused (gpiolib-cdev), one for the event clock flags of a
released line (gpiolib). This series does not depend on them, nor they
on it; all three apply to v7.3-rc5, gpio/for-next and gpio/for-current
in any order.
Direction fix: https://lore.kernel.org/r/20261012-gpiolib-cdev-direction-v1-1-aambrozewicz@nvidia.com
Event clock fix: https://lore.kernel.org/r/20261012-gpiolib-event-clock-v1-1-aambrozewicz@nvidia.com
Cc'd beyond the GPIO maintainers and list: Paul for the SRCU side,
since patch 1 replaces synchronize_srcu() with srcu_barrier() before
cleanup_srcu_struct(), patch 4 moves sleeping waits out of the
gdev->srcu read section, and patch 6 relies on synchronize_srcu() on
gdev->srcu to rule out new IRQ requests through the character device;
Herve for [1], which patch 6 revisits; Jia Jia for the virtio-gpio
use-after-free report [2] that patch 3 addresses; Kent as the author of
the v2 character device; Andy, who acked the commit that patch 2 fixes.
[1] https://lore.kernel.org/all/20240227113426.253232-1-herve.codina@bootlin.com/
[2] https://lore.kernel.org/all/20260817070429.157993-1-physicalmtea@gmail.com/
[3] https://lore.kernel.org/all/20261011121246.162759-1-tzungbi@kernel.org/
Adrian Ambrozewicz (6):
gpiolib: wait for pending label frees before cleaning up desc_srcu
gpiolib: free the label of lines released after their chip was removed
gpiolib: cdev: do not free line IRQs again after the chip is removed
gpiolib: cdev: don't sleep in the SRCU read section in read()
gpiolib: cdev: don't leak the IRQ name when the chip is removed
gpiolib: let the character device free its IRQs first on chip removal
drivers/gpio/gpiolib-cdev.c | 136 +++++++++++++++++++++++++++---------
drivers/gpio/gpiolib.c | 34 +++++++--
2 files changed, 130 insertions(+), 40 deletions(-)
base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu
2026-10-12 13:49 [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
@ 2026-10-12 13:49 ` Adrian Ambrozewicz
2026-10-12 15:46 ` Paul E. McKenney
2026-10-12 20:42 ` Andy Shevchenko
2026-10-12 13:49 ` [PATCH 2/6] gpiolib: free the label of lines released after their chip was removed Adrian Ambrozewicz
` (4 subsequent siblings)
5 siblings, 2 replies; 9+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 13:49 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij
Cc: Bartosz Golaszewski, Kent Gibson, Paul E. McKenney, Herve Codina,
Jia Jia, Andy Shevchenko, linux-gpio, linux-kernel, stable
desc_set_label() frees the old label with call_srcu() on gdev->desc_srcu.
gpiodev_release() calls synchronize_srcu() and then cleanup_srcu_struct()
on it, meaning to run the pending frees first, but synchronize_srcu()
only waits for a grace period, not for callbacks queued before it to be
invoked. Once the srcu_struct uses per-CPU callback lists, a label freed
on one CPU, for instance by gpiochip_free_hogs() at the start of
gpiochip_remove(), can still be waiting for its invocation timer there
when synchronize_srcu() returns on another, and cleanup_srcu_struct()
warns:
WARNING: kernel/rcu/srcutree.c:706 at cleanup_srcu_struct+0x239/0x330, CPU#0: init/1
Call Trace:
<TASK>
gpiodev_release+0x20/0x60
device_release+0x3e/0x90
kobject_put+0x67/0xf0
release_nodes+0x3a/0x60
devres_release_all+0x8a/0xf0
device_unbind_cleanup+0x21/0xa0
device_release_driver_internal+0x1c6/0x210
bus_remove_device+0xea/0x180
device_del+0x152/0x460
platform_device_del+0x20/0x80
platform_device_unregister+0xd/0x30
gpio_sim_device_deactivate+0x26/0x80
gpio_sim_device_config_live_store+0x254/0x3e0
configfs_write_iter+0xc2/0x130
vfs_write+0x252/0x530
ksys_write+0x6c/0xe0
do_syscall_64+0x109/0x5d0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
cleanup_srcu_struct() then flushes the callbacks itself, so the label is
still freed, but the warning is what the comment above that check says
srcu_barrier() is for. Use it.
A gpio-sim chip with one hogged line, set live and back in a loop, warns
within 4 to 60 cycles in three runs on a 4-CPU guest booted with
srcutree.convert_to_big=1; with this change, three runs of 1000 cycles
pass. With the default srcutree parameters the srcu_struct only goes
per-CPU under contention and the loop did not hit it in 3000 cycles; the
warning was first seen that way, once, on a KCSAN arm64 guest unbinding
a USB-attached GPIO controller whose firmware node hogs a line, and
since then on real hardware with v7.3-rc5 arm64 KCSAN kernels, when
unbinding a USB GPIO expander whose driver is not yet upstream, built
both as an out-of-tree module and into the kernel.
Fixes: a86d27693066 ("gpiolib: fix the speed of descriptor label setting with SRCU")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
drivers/gpio/gpiolib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 772c6f04f489..1984f3416437 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -895,7 +895,7 @@ static void gpiodev_release(struct device *dev)
struct gpio_device *gdev = to_gpio_device(dev);
/* Call pending kfree()s for descriptor labels. */
- synchronize_srcu(&gdev->desc_srcu);
+ srcu_barrier(&gdev->desc_srcu);
cleanup_srcu_struct(&gdev->desc_srcu);
ida_free(&gpio_ida, gdev->id);
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/6] gpiolib: free the label of lines released after their chip was removed
2026-10-12 13:49 [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu Adrian Ambrozewicz
@ 2026-10-12 13:49 ` Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 3/6] gpiolib: cdev: do not free line IRQs again after the chip is removed Adrian Ambrozewicz
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 13:49 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij
Cc: Bartosz Golaszewski, Kent Gibson, Paul E. McKenney, Herve Codina,
Jia Jia, Andy Shevchenko, linux-gpio, linux-kernel, stable
gpiod_free_commit() only drops the consumer label while the chip is
still there:
if (guard.gc && test_bit(GPIOD_FLAG_REQUESTED, &flags)) {
...
desc_set_label(desc, NULL);
A line that is still requested when gpiochip_remove() runs, for instance
by a character device line request that user space keeps open, is
released only afterwards, when gdev->chip is already NULL. Its label is
then never freed, as gpiodev_release() frees the descriptor array but
not the labels, and kmemleak reports one object per such line:
unreferenced object 0xff110000057b63c0 (size 32):
comm "gpio-hold", pid 111, jiffies 4294696675
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
3f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ?...............
backtrace (crc 7ff222d9):
__kmalloc_noprof+0x4ee/0x670
desc_set_label+0x32/0x110
gpiod_request_commit+0x1dd/0x320
gpiod_request+0x28/0x100
linereq_create+0x61a/0xce0
gpio_ioctl+0x49c/0xc80
__x64_sys_ioctl+0x134/0x1c0
do_syscall_64+0x109/0x5d0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Commit 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
cleared the label unconditionally; it moved under the chip check with
the removal of gpio_lock. Clear it when the chip is gone as well. The
->free() callback and the line state notification still need the chip
and stay where they are.
The label is now often freed just before the last reference to the GPIO
device goes away, which depends on gpiodev_release() waiting for pending
label frees (previous patch).
Reproduced with gpio-sim: request a line through the character device,
set the chip's configfs "live" attribute to 0 while the request is open,
then close it. Without this change kmemleak reports between 15 and 30
such objects after 30 cycles; with it, none after 100.
Fixes: 35b545332b80 ("gpio: remove gpio_lock")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
drivers/gpio/gpiolib.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1984f3416437..750378f3907a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2689,6 +2689,9 @@ void gpiod_free_commit(struct gpio_desc *desc)
WRITE_ONCE(desc->debounce_period_us, 0);
#endif
gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_RELEASED);
+ } else if (!guard.gc) {
+ /* The chip is gone but the consumer label is still ours. */
+ desc_set_label(desc, NULL);
}
}
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/6] gpiolib: cdev: do not free line IRQs again after the chip is removed
2026-10-12 13:49 [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 2/6] gpiolib: free the label of lines released after their chip was removed Adrian Ambrozewicz
@ 2026-10-12 13:49 ` Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 4/6] gpiolib: cdev: don't sleep in the SRCU read section in read() Adrian Ambrozewicz
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 13:49 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij
Cc: Bartosz Golaszewski, Kent Gibson, Paul E. McKenney, Herve Codina,
Jia Jia, Andy Shevchenko, linux-gpio, linux-kernel, stable
Since commit ec8b6f55b981 ("gpio: free irqs that are still requested
when the chip is being removed"), gpiochip_remove() calls free_irq() for
every action still installed on the lines of the chip. The character
device does not know about this: a line request or v1 line event that
user space keeps open across the removal still holds the IRQ number and
calls free_irq() on it again when it is released.
What that second free_irq() does depends on what happened to the IRQ
number in the meantime. If the chip disposed of its mapping, the
descriptor is gone and the call returns early, or, when it races with
the RCU-deferred free of the descriptor, it touches freed memory, as
reported with virtio-gpio. If the number was handed out again, for
instance because the same chip was set up again, the call finds a live
descriptor without our action and warns:
Trying to free already-free IRQ 28
WARNING: kernel/irq/manage.c:1881 at free_irq+0x193/0x2d0, CPU#2: gpio-hold/111
Call Trace:
<TASK>
linereq_free+0xd2/0x120
linereq_release+0xd/0x20
__fput+0xed/0x2a0
__x64_sys_close+0x38/0x80
do_syscall_64+0x109/0x5d0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
Stop edge detection from the device notifier that gpiochip_remove()
already calls into, rather than when the request is released, and only
free the IRQ there if it still has an action: chips that never mark
their lines as used for IRQs, which gpiochip_lock_as_irq() leaves to
the driver, keep theirs past gpiod_free_irqs(). The release path then
finds no IRQ left to free. The notifier takes the request's
config_mutex, which is otherwise only taken by the ioctls, so that it
does not race with a concurrent reconfiguration of the lines.
Reproduced with gpio-sim: request a line with edge detection, set the
chip's configfs "live" attribute to 0 while the request is open, set it
up again and request the same line, then close the first request. Each
of 30 cycles warns as above, for v2 requests with and without software
debouncing and for v1 line events; with this change, 100 cycles of each
pass without a warning.
Fixes: ec8b6f55b981 ("gpio: free irqs that are still requested when the chip is being removed")
Reported-by: Jia Jia <physicalmtea@gmail.com>
Closes: https://lore.kernel.org/all/20260817070429.157993-1-physicalmtea@gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
drivers/gpio/gpiolib-cdev.c | 52 +++++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 5d53bfcdf726..2c9ec524d38e 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -544,17 +544,6 @@ struct linereq {
GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE | \
GPIO_V2_LINE_EDGE_FLAGS)
-static int linereq_unregistered_notify(struct notifier_block *nb,
- unsigned long action, void *data)
-{
- struct linereq *lr = container_of(nb, struct linereq,
- device_unregistered_nb);
-
- wake_up_poll(&lr->wait, EPOLLIN | EPOLLERR);
-
- return NOTIFY_OK;
-}
-
static void linereq_put_event(struct linereq *lr,
struct gpio_v2_line_event *le)
{
@@ -609,6 +598,17 @@ static inline void free_irq_label(const char *label)
kfree(label);
}
+/*
+ * gpiochip_remove() frees the interrupts still requested on the lines of the
+ * chip being removed, so once the chip is going away the IRQ may no longer be
+ * ours to free.
+ */
+static void free_line_irq(unsigned int irq, void *dev_id)
+{
+ if (irq_has_action(irq))
+ free_irq_label(free_irq(irq, dev_id));
+}
+
#ifdef CONFIG_HTE
static enum hte_return process_hw_ts_thread(void *p)
@@ -975,7 +975,7 @@ static u32 gpio_v2_line_config_debounce_period(struct gpio_v2_line_config *lc,
static void edge_detector_stop(struct line *line)
{
if (line->irq) {
- free_irq_label(free_irq(line->irq, line));
+ free_line_irq(line->irq, line);
line->irq = 0;
}
@@ -992,6 +992,27 @@ static void edge_detector_stop(struct line *line)
/* do not change line->level - see comment in debounced_value() */
}
+static int linereq_unregistered_notify(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct linereq *lr = container_of(nb, struct linereq,
+ device_unregistered_nb);
+ unsigned int i;
+
+ /*
+ * Stop edge detection now rather than when the request is released:
+ * by then the IRQ numbers of the removed chip may be in use again.
+ */
+ scoped_guard(mutex, &lr->config_mutex) {
+ for (i = 0; i < lr->num_lines; i++)
+ edge_detector_stop(&lr->lines[i]);
+ }
+
+ wake_up_poll(&lr->wait, EPOLLIN | EPOLLERR);
+
+ return NOTIFY_OK;
+}
+
static int edge_detector_fifo_init(struct linereq *req)
{
if (kfifo_initialized(&req->events))
@@ -1792,6 +1813,11 @@ static int lineevent_unregistered_notify(struct notifier_block *nb,
struct lineevent_state *le = container_of(nb, struct lineevent_state,
device_unregistered_nb);
+ if (le->irq) {
+ free_line_irq(le->irq, le);
+ le->irq = 0;
+ }
+
wake_up_poll(&le->wait, EPOLLIN | EPOLLERR);
return NOTIFY_OK;
@@ -1872,7 +1898,7 @@ static void lineevent_free(struct lineevent_state *le)
blocking_notifier_chain_unregister(&le->gdev->device_notifier,
&le->device_unregistered_nb);
if (le->irq)
- free_irq_label(free_irq(le->irq, le));
+ free_line_irq(le->irq, le);
if (le->desc)
gpiod_free(le->desc);
kfree(le->label);
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/6] gpiolib: cdev: don't sleep in the SRCU read section in read()
2026-10-12 13:49 [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
` (2 preceding siblings ...)
2026-10-12 13:49 ` [PATCH 3/6] gpiolib: cdev: do not free line IRQs again after the chip is removed Adrian Ambrozewicz
@ 2026-10-12 13:49 ` Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed Adrian Ambrozewicz
2026-10-12 13:50 ` [PATCH 6/6] gpiolib: let the character device free its IRQs first on chip removal Adrian Ambrozewicz
5 siblings, 0 replies; 9+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 13:49 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij
Cc: Bartosz Golaszewski, Kent Gibson, Paul E. McKenney, Herve Codina,
Jia Jia, Andy Shevchenko, linux-gpio, linux-kernel, stable
linereq_read(), lineevent_read() and lineinfo_watch_read() enter the
gdev->srcu read section to check that the chip is still there and stay
in it while they sleep waiting for an event. gpiochip_remove() clears
the chip pointer and then calls synchronize_srcu() on gdev->srcu, which
cannot return while a reader sleeps there. The wake-up meant for that
reader is sent by the device notifier, which runs later in
gpiochip_remove() and would not end the wait anyway, as the wait
condition only looks at the event FIFO. Unless an event arrives, a
process blocked in read() on a line request, a v1 line event or a chip
file with watched lines holds up the removal of the chip until it is
signalled:
task:init state:D stack:0 pid:1 tgid:1 ppid:0
Call Trace:
__synchronize_srcu
gpiochip_remove
release_nodes
devres_release_all
device_unbind_cleanup
device_release_driver_internal
bus_remove_device
device_del
platform_device_unregister
gpio_sim_device_deactivate
gpio_sim_device_config_live_store
configfs_write_iter
vfs_write
and since the device lock is held meanwhile, so does device_shutdown()
on power-off.
Check the chip pointer in a scoped SRCU section only. Nothing after the
check dereferences the chip, the request and its FIFO belong to the
file. Also end the wait once the chip is gone, which the notifiers'
wake-ups now see, and return -ENODEV if there is no event to report.
This dates back to the RW semaphore that the SRCU section replaced,
which these functions also held while sleeping and gpiochip_remove()
took for writing.
Reproduced with gpio-sim: one thread blocked in read() on a line
request while another sets the chip's configfs "live" attribute to 0.
The write does not return until the reader is killed; with this change
it returns at once and read() fails with -ENODEV.
Fixes: bdbbae241a04 ("gpiolib: protect the GPIO device against being dropped while in use by user-space")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
drivers/gpio/gpiolib-cdev.c | 43 ++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 2c9ec524d38e..11d5b112886c 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1520,10 +1520,14 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
ssize_t bytes_read = 0;
int ret;
- guard(srcu)(&lr->gdev->srcu);
-
- if (!rcu_access_pointer(lr->gdev->chip))
- return -ENODEV;
+ /*
+ * Don't stay in the SRCU read section while sleeping: that would stall
+ * gpiochip_remove() until an event arrives.
+ */
+ scoped_guard(srcu, &lr->gdev->srcu) {
+ if (!rcu_access_pointer(lr->gdev->chip))
+ return -ENODEV;
+ }
if (count < sizeof(le))
return -EINVAL;
@@ -1538,9 +1542,12 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
return -EAGAIN;
ret = wait_event_interruptible_locked(lr->wait,
- !kfifo_is_empty(&lr->events));
+ !kfifo_is_empty(&lr->events) ||
+ !rcu_access_pointer(lr->gdev->chip));
if (ret)
return ret;
+ if (kfifo_is_empty(&lr->events))
+ return -ENODEV;
}
if (kfifo_out(&lr->events, &le, 1) != 1) {
@@ -1837,10 +1844,10 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
ssize_t ge_size;
int ret;
- guard(srcu)(&le->gdev->srcu);
-
- if (!rcu_access_pointer(le->gdev->chip))
- return -ENODEV;
+ scoped_guard(srcu, &le->gdev->srcu) {
+ if (!rcu_access_pointer(le->gdev->chip))
+ return -ENODEV;
+ }
/*
* When compatible system call is being used the struct gpioevent_data,
@@ -1868,9 +1875,12 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
return -EAGAIN;
ret = wait_event_interruptible_locked(le->wait,
- !kfifo_is_empty(&le->events));
+ !kfifo_is_empty(&le->events) ||
+ !rcu_access_pointer(le->gdev->chip));
if (ret)
return ret;
+ if (kfifo_is_empty(&le->events))
+ return -ENODEV;
}
if (kfifo_out(&le->events, &ge, 1) != 1) {
@@ -2615,10 +2625,10 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
int ret;
size_t event_size;
- guard(srcu)(&cdev->gdev->srcu);
-
- if (!rcu_access_pointer(cdev->gdev->chip))
- return -ENODEV;
+ scoped_guard(srcu, &cdev->gdev->srcu) {
+ if (!rcu_access_pointer(cdev->gdev->chip))
+ return -ENODEV;
+ }
#ifndef CONFIG_GPIO_CDEV_V1
event_size = sizeof(struct gpio_v2_line_info_changed);
@@ -2636,9 +2646,12 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
return -EAGAIN;
ret = wait_event_interruptible_locked(cdev->wait,
- !kfifo_is_empty(&cdev->events));
+ !kfifo_is_empty(&cdev->events) ||
+ !rcu_access_pointer(cdev->gdev->chip));
if (ret)
return ret;
+ if (kfifo_is_empty(&cdev->events))
+ return -ENODEV;
}
#ifdef CONFIG_GPIO_CDEV_V1
/* must be after kfifo check so watch_abi_version is set */
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed
2026-10-12 13:49 [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
` (3 preceding siblings ...)
2026-10-12 13:49 ` [PATCH 4/6] gpiolib: cdev: don't sleep in the SRCU read section in read() Adrian Ambrozewicz
@ 2026-10-12 13:49 ` Adrian Ambrozewicz
2026-10-12 13:50 ` [PATCH 6/6] gpiolib: let the character device free its IRQs first on chip removal Adrian Ambrozewicz
5 siblings, 0 replies; 9+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 13:49 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij
Cc: Bartosz Golaszewski, Kent Gibson, Paul E. McKenney, Herve Codina,
Jia Jia, Andy Shevchenko, linux-gpio, linux-kernel, stable
The character device requests line IRQs with a copy of the consumer
label as the name and frees that copy with the name free_irq() returns.
Since commit ec8b6f55b981 ("gpio: free irqs that are still requested
when the chip is being removed"), gpiochip_remove() may free the IRQ
first and drops the name it gets back. The character device then does
not get it from its own free_irq() call, which since the previous patch
it does not even make, so the copy leaks:
unreferenced object 0xff1100000435b080 (size 8):
comm "gpio-hold", pid 108, jiffies 4294695819
hex dump (first 8 bytes):
72 65 70 72 6f 00 00 00 repro...
backtrace (crc dc8eac94):
__kmalloc_node_track_caller_noprof+0x50f/0x6a0
kstrdup+0x35/0x90
kstrdup_and_replace+0x19/0xe0
edge_detector_setup+0x5d8/0x940
linereq_create+0x9ec/0xce0
gpio_ioctl+0x49c/0xc80
__x64_sys_ioctl+0x134/0x1c0
do_syscall_64+0x109/0x5d0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Keep the name with the line, or with the v1 line event, and free it
there whoever freed the IRQ.
Reproduced with gpio-sim and a line requested with edge detection and
a consumer label, the chip being removed while the request is open:
kmemleak reports between 17 and 23 such objects after 30 cycles; with
this change, none after 100.
Fixes: ec8b6f55b981 ("gpio: free irqs that are still requested when the chip is being removed")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
drivers/gpio/gpiolib-cdev.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 11d5b112886c..ae19b4c3e9d8 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -399,6 +399,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
* @desc: the GPIO descriptor for this line.
* @req: the corresponding line request
* @irq: the interrupt triggered in response to events on this GPIO
+ * @irq_label: the name @irq was requested with
* @edflags: the edge flags, GPIO_V2_LINE_FLAG_EDGE_RISING and/or
* GPIO_V2_LINE_FLAG_EDGE_FALLING, indicating the edge detection applied
* @timestamp_ns: cache for the timestamp storing it between hardirq and
@@ -422,6 +423,7 @@ struct line {
*/
struct linereq *req;
unsigned int irq;
+ char *irq_label;
/*
* The flags for the active edge detector configuration.
*
@@ -601,12 +603,15 @@ static inline void free_irq_label(const char *label)
/*
* gpiochip_remove() frees the interrupts still requested on the lines of the
* chip being removed, so once the chip is going away the IRQ may no longer be
- * ours to free.
+ * ours to free. The label is kept by the caller as free_irq() is then not the
+ * one returning it.
*/
-static void free_line_irq(unsigned int irq, void *dev_id)
+static void free_line_irq(unsigned int irq, void *dev_id, char **label)
{
if (irq_has_action(irq))
- free_irq_label(free_irq(irq, dev_id));
+ free_irq(irq, dev_id);
+ free_irq_label(*label);
+ *label = NULL;
}
#ifdef CONFIG_HTE
@@ -932,6 +937,7 @@ static int debounce_setup(struct line *line, unsigned int debounce_period_us)
return ret;
}
line->irq = irq;
+ line->irq_label = label;
} else {
ret = hte_edge_setup(line, GPIO_V2_LINE_FLAG_EDGE_BOTH);
if (ret)
@@ -975,7 +981,7 @@ static u32 gpio_v2_line_config_debounce_period(struct gpio_v2_line_config *lc,
static void edge_detector_stop(struct line *line)
{
if (line->irq) {
- free_line_irq(line->irq, line);
+ free_line_irq(line->irq, line, &line->irq_label);
line->irq = 0;
}
@@ -1078,6 +1084,7 @@ static int edge_detector_setup(struct line *line,
}
line->irq = irq;
+ line->irq_label = label;
return 0;
}
@@ -1772,6 +1779,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
* @desc: the GPIO descriptor held by this event
* @eflags: the event flags this line was requested with
* @irq: the interrupt that trigger in response to events on this GPIO
+ * @irq_label: the name @irq was requested with
* @wait: wait queue that handles blocking reads of events
* @device_unregistered_nb: notifier block for receiving gdev unregister events
* @events: KFIFO for the GPIO events
@@ -1785,6 +1793,7 @@ struct lineevent_state {
struct gpio_desc *desc;
u32 eflags;
int irq;
+ char *irq_label;
wait_queue_head_t wait;
struct notifier_block device_unregistered_nb;
DECLARE_KFIFO(events, struct gpioevent_data, 16);
@@ -1821,7 +1830,7 @@ static int lineevent_unregistered_notify(struct notifier_block *nb,
device_unregistered_nb);
if (le->irq) {
- free_line_irq(le->irq, le);
+ free_line_irq(le->irq, le, &le->irq_label);
le->irq = 0;
}
@@ -1908,7 +1917,7 @@ static void lineevent_free(struct lineevent_state *le)
blocking_notifier_chain_unregister(&le->gdev->device_notifier,
&le->device_unregistered_nb);
if (le->irq)
- free_line_irq(le->irq, le);
+ free_line_irq(le->irq, le, &le->irq_label);
if (le->desc)
gpiod_free(le->desc);
kfree(le->label);
@@ -2147,6 +2156,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
}
le->irq = irq;
+ le->irq_label = label;
FD_PREPARE(fdf, O_RDONLY | O_CLOEXEC,
anon_inode_getfile("gpio-event", &lineevent_fileops, le,
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 6/6] gpiolib: let the character device free its IRQs first on chip removal
2026-10-12 13:49 [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
` (4 preceding siblings ...)
2026-10-12 13:49 ` [PATCH 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed Adrian Ambrozewicz
@ 2026-10-12 13:50 ` Adrian Ambrozewicz
5 siblings, 0 replies; 9+ messages in thread
From: Adrian Ambrozewicz @ 2026-10-12 13:50 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij
Cc: Bartosz Golaszewski, Kent Gibson, Paul E. McKenney, Herve Codina,
Jia Jia, Andy Shevchenko, linux-gpio, linux-kernel, stable
gpiochip_remove() force-frees every IRQ still requested on the chip's
lines with gpiochip_free_remaining_irqs() before anything else, then
clears the chip pointer, removes the irqchip, disposing of the IRQ
mappings, and only at the very end, from gpiolib_cdev_unregister(),
calls the device notifier on which line requests and v1 line events
stop edge detection and free their own IRQs. That leaves two windows in
which the character device frees an IRQ it no longer owns:
- The notifier runs after the mappings are gone. If an IRQ number freed
by gpiochip_irqchip_remove() is allocated to another device in the
meantime, irq_has_action() finds that device's action and free_irq()
warns "Trying to free already-free IRQ". IRQs requested through the
character device after gpiochip_free_remaining_irqs() but before the
chip pointer is cleared are still installed when their mappings are
disposed of.
- Releasing a request concurrently with gpiochip_remove() races
gpiod_free_irqs(), which can free the action between the release
path's irq_has_action() check and its free_irq(), and then
gpiochip_irqchip_remove(), which can dispose of the descriptor that
free_irq() is still using.
Call the device notifier from gpiochip_remove() right after the chip
pointer is cleared and the SRCU grace period has elapsed, when no ioctl
can request new IRQs any more, and only free the remaining IRQs after
it. Have the release paths free their IRQs before unregistering from the
notifier, under the lock that the notifier callback also takes, so that
the notifier does not return while a release is still freeing an IRQ.
For v1 line events that needs a new mutex; line requests use their
config_mutex, which now has to be initialized before the first error
path. As the chip pointer is NULL by the time gpiod_free_irqs() runs,
have it call gc->to_irq() directly rather than gpiod_to_irq().
Every IRQ requested through the character device is thus freed exactly
once, by the notifier or by the release path, and before
gpiod_free_irqs() and the removal of the irqchip.
Moving the character device's part of the removal earlier was proposed
by Herve Codina before gpiochip_free_remaining_irqs() existed; with that
in place, only the order of the two is left to fix.
Reproduced with a test-only GPIO chip that uses the gpiolib hierarchical
irqchip, unbound from its driver with lines requested through the
character device. For the first window, the chip hands the IRQ number of
line 0 to a dummy handler while gpiochip_irqchip_remove() disposes of
the mappings, and the requests are closed after the unbind. For the
second, it sleeps in its irq_bus_lock() callback once removal has
started, and the request is closed while gpiochip_remove() is in
free_irq(). Each of 30 cycles warns "Trying to free already-free IRQ" in
both cases, for v2 line requests and v1 line events alike; with this
change none does.
Link: https://lore.kernel.org/all/20240227113426.253232-1-herve.codina@bootlin.com/
Fixes: ec8b6f55b981 ("gpio: free irqs that are still requested when the chip is being removed")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
drivers/gpio/gpiolib-cdev.c | 39 +++++++++++++++++++++++++++----------
drivers/gpio/gpiolib.c | 29 ++++++++++++++++++++++-----
2 files changed, 53 insertions(+), 15 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index ae19b4c3e9d8..7565c9f68060 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1580,15 +1580,24 @@ static void linereq_free(struct linereq *lr)
{
unsigned int i;
+ /*
+ * Stop edge detection while still registered for the notification, so
+ * that gpiochip_remove() does not get past it until the IRQs are freed.
+ */
+ scoped_guard(mutex, &lr->config_mutex) {
+ for (i = 0; i < lr->num_lines; i++) {
+ if (lr->lines[i].desc)
+ edge_detector_stop(&lr->lines[i]);
+ }
+ }
+
if (lr->device_unregistered_nb.notifier_call)
blocking_notifier_chain_unregister(&lr->gdev->device_notifier,
&lr->device_unregistered_nb);
for (i = 0; i < lr->num_lines; i++) {
- if (lr->lines[i].desc) {
- edge_detector_stop(&lr->lines[i]);
+ if (lr->lines[i].desc)
gpiod_free(lr->lines[i].desc);
- }
}
kfifo_free(&lr->events);
kfree(lr->label);
@@ -1663,6 +1672,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
if (!lr)
return -ENOMEM;
lr->num_lines = ulr.num_lines;
+ mutex_init(&lr->config_mutex);
lr->gdev = gpio_device_get(gdev);
@@ -1680,7 +1690,6 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
return -ENOMEM;
}
- mutex_init(&lr->config_mutex);
init_waitqueue_head(&lr->wait);
INIT_KFIFO(lr->events);
lr->event_buffer_size = ulr.event_buffer_size;
@@ -1780,6 +1789,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
* @eflags: the event flags this line was requested with
* @irq: the interrupt that trigger in response to events on this GPIO
* @irq_label: the name @irq was requested with
+ * @irq_lock: serializes freeing @irq on release and on chip removal
* @wait: wait queue that handles blocking reads of events
* @device_unregistered_nb: notifier block for receiving gdev unregister events
* @events: KFIFO for the GPIO events
@@ -1794,6 +1804,8 @@ struct lineevent_state {
u32 eflags;
int irq;
char *irq_label;
+ /* release vs. chip removal notifier */
+ struct mutex irq_lock;
wait_queue_head_t wait;
struct notifier_block device_unregistered_nb;
DECLARE_KFIFO(events, struct gpioevent_data, 16);
@@ -1829,9 +1841,11 @@ static int lineevent_unregistered_notify(struct notifier_block *nb,
struct lineevent_state *le = container_of(nb, struct lineevent_state,
device_unregistered_nb);
- if (le->irq) {
- free_line_irq(le->irq, le, &le->irq_label);
- le->irq = 0;
+ scoped_guard(mutex, &le->irq_lock) {
+ if (le->irq) {
+ free_line_irq(le->irq, le, &le->irq_label);
+ le->irq = 0;
+ }
}
wake_up_poll(&le->wait, EPOLLIN | EPOLLERR);
@@ -1913,11 +1927,16 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
static void lineevent_free(struct lineevent_state *le)
{
+ /* As in linereq_free(), free the IRQ before unregistering. */
+ scoped_guard(mutex, &le->irq_lock) {
+ if (le->irq) {
+ free_line_irq(le->irq, le, &le->irq_label);
+ le->irq = 0;
+ }
+ }
if (le->device_unregistered_nb.notifier_call)
blocking_notifier_chain_unregister(&le->gdev->device_notifier,
&le->device_unregistered_nb);
- if (le->irq)
- free_line_irq(le->irq, le, &le->irq_label);
if (le->desc)
gpiod_free(le->desc);
kfree(le->label);
@@ -2093,6 +2112,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
le = kzalloc_obj(*le);
if (!le)
return -ENOMEM;
+ mutex_init(&le->irq_lock);
le->gdev = gpio_device_get(gdev);
if (eventreq.consumer_label[0] != '\0') {
@@ -2829,5 +2849,4 @@ void gpiolib_cdev_unregister(struct gpio_device *gdev)
{
destroy_workqueue(gdev->line_state_wq);
cdev_device_del(&gdev->chrdev, &gdev->dev);
- blocking_notifier_call_chain(&gdev->device_notifier, 0, NULL);
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 750378f3907a..f12f6eeb578d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -851,11 +851,23 @@ bool gpiochip_line_is_valid(const struct gpio_chip *gc,
}
EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
-static void gpiod_free_irqs(struct gpio_desc *desc)
+static void gpiod_free_irqs(struct gpio_chip *gc, struct gpio_desc *desc)
{
- int irq = gpiod_to_irq(desc);
- struct irq_desc *irqd = irq_to_desc(irq);
+ struct irq_desc *irqd;
void *cookie;
+ int irq;
+
+ /* Called with gdev->chip already NULL, so gpiod_to_irq() can't be used. */
+ if (!gc->to_irq)
+ return;
+
+ irq = gc->to_irq(gc, gpiod_hwgpio(desc));
+ if (irq <= 0)
+ return;
+
+ irqd = irq_to_desc(irq);
+ if (!irqd)
+ return;
for (;;) {
/*
@@ -887,7 +899,7 @@ static void gpiochip_free_remaining_irqs(struct gpio_chip *gc)
struct gpio_desc *desc;
for_each_gpio_desc_with_flag(gc, desc, GPIOD_FLAG_USED_AS_IRQ)
- gpiod_free_irqs(desc);
+ gpiod_free_irqs(gc, desc);
}
static void gpiodev_release(struct device *dev)
@@ -1422,7 +1434,6 @@ void gpiochip_remove(struct gpio_chip *gc)
/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
gpiochip_sysfs_unregister(gc);
gpiochip_free_hogs(gc);
- gpiochip_free_remaining_irqs(gc);
scoped_guard(mutex, &gpio_devices_lock)
list_del_rcu(&gdev->list);
@@ -1431,6 +1442,14 @@ void gpiochip_remove(struct gpio_chip *gc)
/* Numb the device, cancelling all outstanding operations */
rcu_assign_pointer(gdev->chip, NULL);
synchronize_srcu(&gdev->srcu);
+
+ /*
+ * No new interrupts can be requested through the character device now.
+ * Have it free the ones it holds before forcibly freeing the rest, and
+ * both before the irqchip and its mappings go away.
+ */
+ blocking_notifier_call_chain(&gdev->device_notifier, 0, NULL);
+ gpiochip_free_remaining_irqs(gc);
gpio_device_teardown_shared(gdev);
gpiochip_irqchip_remove(gc);
acpi_gpiochip_remove(gc);
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu
2026-10-12 13:49 ` [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu Adrian Ambrozewicz
@ 2026-10-12 15:46 ` Paul E. McKenney
2026-10-12 20:42 ` Andy Shevchenko
1 sibling, 0 replies; 9+ messages in thread
From: Paul E. McKenney @ 2026-10-12 15:46 UTC (permalink / raw)
To: Adrian Ambrozewicz
Cc: Bartosz Golaszewski, Linus Walleij, Bartosz Golaszewski,
Kent Gibson, Herve Codina, Jia Jia, Andy Shevchenko, linux-gpio,
linux-kernel, stable
On Mon, Oct 12, 2026 at 04:49:55PM +0300, Adrian Ambrozewicz wrote:
> desc_set_label() frees the old label with call_srcu() on gdev->desc_srcu.
> gpiodev_release() calls synchronize_srcu() and then cleanup_srcu_struct()
> on it, meaning to run the pending frees first, but synchronize_srcu()
> only waits for a grace period, not for callbacks queued before it to be
> invoked. Once the srcu_struct uses per-CPU callback lists, a label freed
> on one CPU, for instance by gpiochip_free_hogs() at the start of
> gpiochip_remove(), can still be waiting for its invocation timer there
> when synchronize_srcu() returns on another, and cleanup_srcu_struct()
> warns:
>
> WARNING: kernel/rcu/srcutree.c:706 at cleanup_srcu_struct+0x239/0x330, CPU#0: init/1
> Call Trace:
> <TASK>
> gpiodev_release+0x20/0x60
> device_release+0x3e/0x90
> kobject_put+0x67/0xf0
> release_nodes+0x3a/0x60
> devres_release_all+0x8a/0xf0
> device_unbind_cleanup+0x21/0xa0
> device_release_driver_internal+0x1c6/0x210
> bus_remove_device+0xea/0x180
> device_del+0x152/0x460
> platform_device_del+0x20/0x80
> platform_device_unregister+0xd/0x30
> gpio_sim_device_deactivate+0x26/0x80
> gpio_sim_device_config_live_store+0x254/0x3e0
> configfs_write_iter+0xc2/0x130
> vfs_write+0x252/0x530
> ksys_write+0x6c/0xe0
> do_syscall_64+0x109/0x5d0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> cleanup_srcu_struct() then flushes the callbacks itself, so the label is
> still freed, but the warning is what the comment above that check says
> srcu_barrier() is for. Use it.
>
> A gpio-sim chip with one hogged line, set live and back in a loop, warns
> within 4 to 60 cycles in three runs on a 4-CPU guest booted with
> srcutree.convert_to_big=1; with this change, three runs of 1000 cycles
> pass. With the default srcutree parameters the srcu_struct only goes
> per-CPU under contention and the loop did not hit it in 3000 cycles; the
> warning was first seen that way, once, on a KCSAN arm64 guest unbinding
> a USB-attached GPIO controller whose firmware node hogs a line, and
> since then on real hardware with v7.3-rc5 arm64 KCSAN kernels, when
> unbinding a USB GPIO expander whose driver is not yet upstream, built
> both as an out-of-tree module and into the kernel.
>
> Fixes: a86d27693066 ("gpiolib: fix the speed of descriptor label setting with SRCU")
> Cc: stable@vger.kernel.org
> Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
From an SRCU viewpoint:
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
In this, I am assuming that the comment is correct, and that we only
need to wait for pending frees, not readers.
Thanx, Paul
> ---
> drivers/gpio/gpiolib.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 772c6f04f489..1984f3416437 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -895,7 +895,7 @@ static void gpiodev_release(struct device *dev)
> struct gpio_device *gdev = to_gpio_device(dev);
>
> /* Call pending kfree()s for descriptor labels. */
> - synchronize_srcu(&gdev->desc_srcu);
> + srcu_barrier(&gdev->desc_srcu);
> cleanup_srcu_struct(&gdev->desc_srcu);
>
> ida_free(&gpio_ida, gdev->id);
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu
2026-10-12 13:49 ` [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu Adrian Ambrozewicz
2026-10-12 15:46 ` Paul E. McKenney
@ 2026-10-12 20:42 ` Andy Shevchenko
1 sibling, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2026-10-12 20:42 UTC (permalink / raw)
To: Adrian Ambrozewicz
Cc: Bartosz Golaszewski, Linus Walleij, Bartosz Golaszewski,
Kent Gibson, Paul E. McKenney, Herve Codina, Jia Jia, linux-gpio,
linux-kernel, stable
On Mon, Oct 12, 2026 at 04:49:55PM +0300, Adrian Ambrozewicz wrote:
> desc_set_label() frees the old label with call_srcu() on gdev->desc_srcu.
> gpiodev_release() calls synchronize_srcu() and then cleanup_srcu_struct()
> on it, meaning to run the pending frees first, but synchronize_srcu()
> only waits for a grace period, not for callbacks queued before it to be
> invoked. Once the srcu_struct uses per-CPU callback lists, a label freed
> on one CPU, for instance by gpiochip_free_hogs() at the start of
> gpiochip_remove(), can still be waiting for its invocation timer there
> when synchronize_srcu() returns on another, and cleanup_srcu_struct()
> warns:
>
> WARNING: kernel/rcu/srcutree.c:706 at cleanup_srcu_struct+0x239/0x330, CPU#0: init/1
> Call Trace:
> <TASK>
> gpiodev_release+0x20/0x60
> device_release+0x3e/0x90
> kobject_put+0x67/0xf0
> release_nodes+0x3a/0x60
> devres_release_all+0x8a/0xf0
> device_unbind_cleanup+0x21/0xa0
> device_release_driver_internal+0x1c6/0x210
> bus_remove_device+0xea/0x180
> device_del+0x152/0x460
> platform_device_del+0x20/0x80
> platform_device_unregister+0xd/0x30
> gpio_sim_device_deactivate+0x26/0x80
> gpio_sim_device_config_live_store+0x254/0x3e0
> configfs_write_iter+0xc2/0x130
> vfs_write+0x252/0x530
> ksys_write+0x6c/0xe0
> do_syscall_64+0x109/0x5d0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
Please, read Submitting Patches on the backtraces in the commit messages
and act accordingly.
> cleanup_srcu_struct() then flushes the callbacks itself, so the label is
> still freed, but the warning is what the comment above that check says
> srcu_barrier() is for. Use it.
>
> A gpio-sim chip with one hogged line, set live and back in a loop, warns
> within 4 to 60 cycles in three runs on a 4-CPU guest booted with
> srcutree.convert_to_big=1; with this change, three runs of 1000 cycles
> pass. With the default srcutree parameters the srcu_struct only goes
> per-CPU under contention and the loop did not hit it in 3000 cycles; the
> warning was first seen that way, once, on a KCSAN arm64 guest unbinding
> a USB-attached GPIO controller whose firmware node hogs a line, and
> since then on real hardware with v7.3-rc5 arm64 KCSAN kernels, when
> unbinding a USB GPIO expander whose driver is not yet upstream, built
> both as an out-of-tree module and into the kernel.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-12 20:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-12 13:49 [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu Adrian Ambrozewicz
2026-10-12 15:46 ` Paul E. McKenney
2026-10-12 20:42 ` Andy Shevchenko
2026-10-12 13:49 ` [PATCH 2/6] gpiolib: free the label of lines released after their chip was removed Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 3/6] gpiolib: cdev: do not free line IRQs again after the chip is removed Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 4/6] gpiolib: cdev: don't sleep in the SRCU read section in read() Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed Adrian Ambrozewicz
2026-10-12 13:50 ` [PATCH 6/6] gpiolib: let the character device free its IRQs first on chip removal Adrian Ambrozewicz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®