Re: [PATCH v3 3/4] Input: hynitron-cst816x: release gesture keys

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] input: TOCTOU race in cst816x_release_keys() due to lockless double-read
--

commit 0b6af957c2d032170de5ed337fe23bb294db578d
Author: Daniel Golle <daniel@makrotopia.org>

Input: hynitron-cst816x: release gesture keys

This commit fixes an issue where gesture keys stay pressed when the
gesture stops but the finger remains down. It adds logic to explicitly
release keys held by the input core on the first non-gesture report.

> diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
> --- a/drivers/input/touchscreen/hynitron-cst816x.c
> +++ b/drivers/input/touchscreen/hynitron-cst816x.c

[ ... ]

> @@ -168,6 +180,18 @@ static void cst816x_reset(struct cst816x_priv *priv)
>  	msleep(100);
>  }
>  
> +static void cst816x_release_keys(struct cst816x_priv *priv)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < priv->keycodemax; i++) {
> +		if (!test_bit(priv->keycode[i], priv->input->key))
> +			continue;
> +
> +		input_report_key(priv->input, priv->keycode[i], 0);

[Severity: Medium]
Is there a race condition here due to reading priv->keycode array elements
twice?

Because the priv->keycode array is shared with the input core and
cst816x_release_keys() does not hold the event_lock, a concurrent
EVIOCSKEYCODE ioctl could modify the array between the test_bit() check
and the input_report_key() call.

If this occurs, could the driver spuriously release a completely different
keycode than the one it just tested?

Would it be safer to read the array element into a local variable first to
ensure the exact same keycode is both evaluated and reported?

> +	}
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791103511.git.daniel@makrotopia.org?part=3