Re: [PATCH] input/evdev: move kill_fasync() outside buffer_lock to fix SOFTIRQ deadlock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving kill_fasync() outside of client->buffer_lock does not resolve the lock ordering violation because the function is still executing under dev->event_lock.
--

commit 803cd2c3ce0ada2c1fe4625fcbef594cb58290eb
Author: Rik van Riel <riel@surriel.com>

input/evdev: move kill_fasync() outside buffer_lock to fix SOFTIRQ deadlock

This commit attempts to fix a SOFTIRQ-safe to SOFTIRQ-unsafe lock inversion
by moving the kill_fasync() call outside of the client->buffer_lock.

> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
[ ... ]
> @@ -280,9 +278,11 @@ static void evdev_pass_values(struct evdev_client *client,
>  
>  	spin_unlock(&client->buffer_lock);
>  
> -	if (wakeup)
> +	if (wakeup) {
> +		kill_fasync(&client->fasync, SIGIO, POLL_IN);
>  		wake_up_interruptible_poll(&client->wait,
>  			EPOLLIN | EPOLLOUT | EPOLLRDNORM | EPOLLWRNORM);
> +	}
>  }

Does this completely resolve the lock ordering violation?

While kill_fasync() is now outside of client->buffer_lock, evdev_pass_values()
is called from evdev_events(), which is invoked by the input core in
input_pass_values().

During this dispatch, the input core strictly holds dev->event_lock:

drivers/input/input.c:input_pass_values() {
    ...
    rcu_read_lock();
    spin_lock_irqsave(&dev->event_lock, flags);
    ...
    handler->events(handle, vals, count);
    ...
    spin_unlock_irqrestore(&dev->event_lock, flags);
}

Since dev->event_lock is also an IRQ/SOFTIRQ-safe spinlock, does this
just shift the problem so we still have a dev->event_lock -> fa_lock
inversion?

Would it make sense to defer the kill_fasync() call entirely outside
the event lock context, perhaps by using irq_work?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513115000.77ec5975@fangorn?part=1