Re: [PATCH] Input: evdev - sanitize event type index when fetching event masks

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Tue, Aug 04, 2026 at 01:54:13AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete Spectre v1 mitigation due to misunderstanding of speculative execution on branches.
> --
> 
> --- Patch [1]: [PATCH] Input: evdev - sanitize event type index when fetching event masks ---
> commit e9eb78cd8dbba1d32ef31f1eec9a5c00e54b5aad
> Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> Input: evdev - sanitize event type index when fetching event masks
> 
> This commit uses array_index_mask_nospec() in evdev_get_mask_cnt() to clamp 
> out-of-bounds event types to 0. It asserts that this is sufficient to prevent 
> speculative out-of-bounds memory accesses in functions like evdev_get_mask() 
> because the returned count is speculatively forced to 0.
> 
> > We do not need additional array_index_nospec() calls in evdev_get_mask()
> > because evdev_get_mask_cnt() speculatively forces the count (and
> > resulting xfer_size) to 0 for out-of-bounds types, preventing any
> > speculative memory access to client evmasks array.
> 
> [Severity: High]
> This is a pre-existing issue, but does relying on the returned count being 0 
> fully prevent speculative out-of-bounds accesses in callers like 
> evdev_get_mask() and evdev_set_mask()?
> 
> When CPU branch predictors mispredict the conditions checking the count, they
> do not wait for the condition to evaluate. 
> 
> For example, if the if (cnt > 0) branch in evdev_get_mask() is mispredicted:
> 
> drivers/input/evdev.c:evdev_get_mask() {
>     ...
>     cnt = evdev_get_mask_cnt(type);
>     ...
>     if (cnt > 0) {
>         mask = client->evmasks[type];
>     ...
> }
> 
> The branch predictor will speculatively execute the block using the 
> original, unclamped type variable, performing an out-of-bounds read on 
> client->evmasks[type]. The retrieved pointer is then accessed by 
> bits_to_user().

No it will not because xfer_size will be 0 and bits_to_user
(copy_to_user) will not dereference the pointer at all.

> 
> A similar pattern exists in evdev_set_mask(), where a mispredicted if (!cnt) 
> branch can lead to using the unclamped type:
> 
> drivers/input/evdev.c:evdev_set_mask() {
>     ...
>     cnt = evdev_get_mask_cnt(type);
>     if (!cnt)
>         return 0;
>     ...
>     oldmask = client->evmasks[type];
>     ...
>     bitmap_free(oldmask);
> }
> 
> The speculative read of oldmask is followed by a dereference in bitmap_free(),
> which creates an exploitable side-channel gadget.

kfree() can not serve as a side-channel gadget. Additionally there are
speculation barriers in copy_from_user() and when taking the spinlock.

> 
> This pattern also appears in __evdev_is_filtered(). A mispredicted bounds 
> check can lead to speculative out-of-bounds accesses for test_bit(type, mask) 
> or mask = client->evmasks[type].

This is internal kernel function and is not user-controlled.

> 
> Do we still need to explicitly use array_index_nospec(type, EV_CNT) inside
> these caller functions to prevent the unclamped type from being used during 
> speculative execution?

So no.

Thanks.

-- 
Dmitry