Re: [PATCH] HID: pidff: validate effect type usage indexes

[Tomasz Pakuła]

On Mon, 2026-11-02 at 19:58 +0800, Yifan Zhang wrote:
> pidff_find_special_keys() records usage positions as one-based values, but
> pidff_find_effects() uses those values directly as array indexes.  The
> Create New Effect and Set Effect fields are independently allocated and
> need not have the same number of usages, so a malformed descriptor can
> make the lookup leave the Set Effect usage array.
> 
> KASAN reports:
> 
>   BUG: KASAN: slab-out-of-bounds in hid_pidff_init_with_quirks
>   Read of size 4
>   hid_pidff_init_with_quirks
>   universal_pidff_probe
>   hid_device_probe
>   uhid_device_add_worker
> 
> Convert the stored position back to a zero-based index and validate it
> against both fields before comparing the usages.
> 
> The reproducer is rejected without a KASAN report with this change.
> 
> Fixes: 224ee88fe395 ("Input: add force feedback driver for PID devices")
> Signed-off-by: Yifan Zhang <yfzhang23@xxxxxxxxxxxxxx>
> ---
>  drivers/hid/usbhid/hid-pidff.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
> index 22951b7ecd174..5e4a937237d55 100644
> --- a/drivers/hid/usbhid/hid-pidff.c
> +++ b/drivers/hid/usbhid/hid-pidff.c
> @@ -1373,9 +1373,16 @@ static int pidff_find_effects(struct pidff_device *pidff,
>  
>  	for (i = 0; i < ARRAY_SIZE(pidff_effect_types); i++) {
>  		int pidff_type = pidff->type_id[i];
> +		int usage_index;
>  
> -		if (pidff->set_effect_type->usage[pidff_type].hid !=
> -		    pidff->create_new_effect_type->usage[pidff_type].hid) {
> +		if (!pidff_type)
> +			continue;
> +
> +		usage_index = pidff_type - 1;
> +		if (usage_index >= pidff->set_effect_type->maxusage ||
> +		    usage_index >= pidff->create_new_effect_type->maxusage ||
> +		    pidff->set_effect_type->usage[usage_index].hid !=
> +		    pidff->create_new_effect_type->usage[usage_index].hid) {
>  			hid_err(pidff->hid,
>  				"effect type number %d is invalid\n", i);
>  			return -1;

Thanks very much! Now that I look at it, find_effects was never
overhauled and I don't quite like how it does things here so I'll redo
the function + include a fix for this issue.