HID: logitech-hidpp: Check bounds when deleting force-feedback effects

[PATCH] HID: logitech-hidpp: Check bounds when deleting force-feedback effects

Posted by Günther Noack 1 day, 3 hours ago

Without this bounds check, this might otherwise overwrite index -1.

Triggering this condition requires action both from the USB device and from
userspace, which reduces the scenarios in which it can be exploited.

Cc: Lee Jones <lee@kernel.org>
Signed-off-by: Günther Noack <gnoack@google.com>
---
 drivers/hid/hid-logitech-hidpp.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index d1dea7297712..5f63f1d2303a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2502,12 +2502,15 @@ static void hidpp_ff_work_handler(struct work_struct *w)
 		}
 		break;
 	case HIDPP_FF_DESTROY_EFFECT:
-		if (wd->effect_id >= 0)
-			/* regular effect destroyed */
-			data->effect_ids[wd->params[0]-1] = -1;
-		else if (wd->effect_id >= HIDPP_FF_EFFECTID_AUTOCENTER)
-			/* autocenter spring destroyed */
-			data->slot_autocenter = 0;
+		slot = wd->params[0];
+		if (slot > 0 && slot <= data->num_effects) {
+			if (wd->effect_id >= 0)
+				/* regular effect destroyed */
+				data->effect_ids[slot-1] = -1;
+			else if (wd->effect_id >= HIDPP_FF_EFFECTID_AUTOCENTER)
+				/* autocenter spring destroyed */
+				data->slot_autocenter = 0;
+		}
 		break;
 	case HIDPP_FF_SET_GLOBAL_GAINS:
 		data->gain = (wd->params[0] << 8) + wd->params[1];
-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH] HID: logitech-hidpp: Check bounds when deleting force-feedback effects

Posted by Lee Jones 1 day, 3 hours ago

On Tue, 31 Mar 2026, Günther Noack wrote:

> Without this bounds check, this might otherwise overwrite index -1.
> 
> Triggering this condition requires action both from the USB device and from
> userspace, which reduces the scenarios in which it can be exploited.
> 
> Cc: Lee Jones <lee@kernel.org>
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index d1dea7297712..5f63f1d2303a 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -2502,12 +2502,15 @@ static void hidpp_ff_work_handler(struct work_struct *w)
>  		}
>  		break;
>  	case HIDPP_FF_DESTROY_EFFECT:
> -		if (wd->effect_id >= 0)
> -			/* regular effect destroyed */
> -			data->effect_ids[wd->params[0]-1] = -1;
> -		else if (wd->effect_id >= HIDPP_FF_EFFECTID_AUTOCENTER)
> -			/* autocenter spring destroyed */
> -			data->slot_autocenter = 0;
> +		slot = wd->params[0];
> +		if (slot > 0 && slot <= data->num_effects) {
> +			if (wd->effect_id >= 0)
> +				/* regular effect destroyed */
> +				data->effect_ids[slot-1] = -1;
> +			else if (wd->effect_id >= HIDPP_FF_EFFECTID_AUTOCENTER)
> +				/* autocenter spring destroyed */
> +				data->slot_autocenter = 0;
> +		}

LGTM.

Reviewed-by: Lee Jones <lee@kernel.org>

-- 
Lee Jones [李琼斯]