Re: [PATCH 2/6] gpiolib: free the label of lines released after their chip was removed
From: Andy Shevchenko
Date: Tue Oct 13 2026 - 13:23:47 EST
On Mon, Oct 12, 2026 at 04:49:56PM +0300, Adrian Ambrozewicz wrote:
> 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.
...
> --- 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);
> }
> }
I have two questions here.
1/ Do we still need to notify user space in the case of the chip gone?
2/ Can we write the above as
if (guard.gc && !test_bit(GPIOD_FLAG_REQUESTED, &flags))
return;
desc_set_label(desc, NULL);
if (guard_gc) {
...current code...
}
--
With Best Regards,
Andy Shevchenko