Re: [PATCH v2] Input: fix potential use-after-free in input_devices_seq_show

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] input: refcount leak and UAF during seq_file iteration
- [Critical] input: invalid pointer dereference in seq_stop on interrupt
--

commit aa10b9792f79b028b976fdcbe93a1680d11f5f5f
Author: Habil Eren Türker <habilerenturker@gmail.com>
Subject: Input: fix potential use-after-free in input_devices_seq_show

This commit attempts to fix a use-after-free in input_devices_seq_show()
by taking a reference to the input device in input_devices_seq_start() and
dropping it in input_devices_seq_stop().

Closes: https://syzkaller.appspot.com/bug?extid=bc6b37960b1d13f68f9f

> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 78c10eea7328d..6d21718a73ff8 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
[ ... ]
> @@ -1124,7 +1125,11 @@ static void *input_devices_seq_start(struct seq_file *seq, loff_t *pos)
>  
>  	state->mutex_acquired = true;
>  
> -	return seq_list_start(&input_dev_list, *pos);
> +	v = seq_list_start(&input_dev_list, *pos);
> +	if (v)
> +		input_get_device(container_of(v, struct input_dev, node));
> +
> +	return v;
>  }
>  
>  static void *input_devices_seq_next(struct seq_file *seq, void *v, loff_t *pos)

[Severity: Critical]
In input_devices_seq_start(), a reference is acquired on the first device. 
However, input_devices_seq_next() advances the iterator without acquiring a 
reference to the next device or dropping the reference to the previous one.

When iteration stops, input_devices_seq_stop() drops a reference on whatever 
device input_devices_seq_next() yielded last.

Can this cause a refcount underflow (and potential use-after-free) on the
last device, while permanently leaking the reference to the first device?

[ ... ]
> @@ -1140,6 +1145,17 @@ static void input_seq_stop(struct seq_file *seq, void *v)
>  		mutex_unlock(&input_mutex);
>  }
>  
> +static void input_devices_seq_stop(struct seq_file *seq, void *v)
> +{
> +	struct input_seq_state *state = seq->private;
> +
> +	if (v)
> +		input_put_device(container_of(v, struct input_dev, node));

[Severity: Critical]
If input_devices_seq_start() is interrupted by a signal while trying to
acquire input_mutex, it returns ERR_PTR(-EINTR). The seq_file core then
passes this error pointer to input_devices_seq_stop().

Because an error pointer is not NULL, the if (v) check will evaluate to true.

Does this pass an invalid, negative pointer to container_of() and
input_put_device(), potentially crashing the kernel? Should this logic verify
!IS_ERR(v) before dereferencing it?

> +
> +	if (state->mutex_acquired)
> +		mutex_unlock(&input_mutex);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928171306.59206-1-habilerenturker@hotmail.com?part=1