Re: [PATCH 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed

[Andy Shevchenko]

On Mon, Oct 12, 2026 at 04:49:59PM +0300, Adrian Ambrozewicz wrote:
> 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

For all patches, please, act in accordance with the Submitting Patches
documentation on backtraces in the commit messages.

> 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.

...

> -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;
>  }

This can be modified a bit to prepare for this with the suggestion to check
an irq here. So, in the previous patch you may have it as

	char *name = NULL;

	if (irq && irq_has_action(irq))
		name = free_irq(irq, dev_id);

	free_irq_label(name);

After this patch

	char *name = *label;

	if (irq && irq_has_action(irq))
		free_irq(irq, dev_id);

	free_irq_label(name);
	*label = NULL;

...

While it might be more verbose I think it makes the point clearer.

-- 
With Best Regards,
Andy Shevchenko