Re: [PATCH 2/6] gpiolib: free the label of lines released after their chip was removed

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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