Re: [PATCH 3/6] gpiolib: cdev: do not free line IRQs again after the chip is removed

[Andy Shevchenko]

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

...

> +/*
> + * 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));

In all cases it also checks for irq to be valid, so embed it here and drop
those checks below.

	if (irq && irq_has_action(irq))

> +}

...

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

This, for instance, will become

	free_line_irq(line->irq, line);
	line->irq = 0;


...

> +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);

Slightly better to have it

	struct linereq *lr = container_of(nb, struct linereq, device_unregistered_nb);

or

	struct linereq *lr =
		container_of(nb, struct linereq, device_unregistered_nb);

> +	unsigned int i;

Not used outside of the loop.

> +	/*
> +	 * 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++)

		for (unsigned int i = 0; i < lr->num_lines; i++)

> +			edge_detector_stop(&lr->lines[i]);
> +	}
> +
> +	wake_up_poll(&lr->wait, EPOLLIN | EPOLLERR);
> +
> +	return NOTIFY_OK;
> +}

-- 
With Best Regards,
Andy Shevchenko