[PATCH] HID: magicmouse: prevent division by 0 on scroll

[PATCH] HID: magicmouse: prevent division by 0 on scroll

[Claudia Pellegrino]

In hid_magicmouse, if the user has set scroll_speed to a value between
55 and 63 and scrolls seven times in quick succession, the
step_hr variable in the magicmouse_emit_touch function becomes 0.

That causes a division by zero further down in the function when
it does `step_x_hr /= step_hr`.

To reproduce, create `/etc/modprobe.d/hid_magicmouse.conf` with the
following content:

```
options hid_magicmouse scroll_acceleration=1 scroll_speed=55
```

Then reboot, connect a Magic Mouse and scroll seven times quickly.
The system will freeze for a minute, and after that `dmesg` will
confirm that a division by zero occurred.

Enforce a minimum of 1 for the variable so the high resolution
step count can never reach 0 even at maximum scroll acceleration.

Fixes: d4b9f10a0eb6 ("HID: magicmouse: enable high-resolution scroll")

Signed-off-by: Claudia Pellegrino <linux@cpellegrino.de>
---
 drivers/hid/hid-magicmouse.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 686788ebf3e1..d7687ce70614 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -256,8 +256,11 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 		unsigned long now = jiffies;
 		int step_x = msc->touches[id].scroll_x - x;
 		int step_y = msc->touches[id].scroll_y - y;
-		int step_hr = ((64 - (int)scroll_speed) * msc->scroll_accel) /
-			      SCROLL_HR_STEPS;
+		int step_hr =
+			max_t(int,
+			      ((64 - (int)scroll_speed) * msc->scroll_accel) /
+					SCROLL_HR_STEPS,
+			      1);
 		int step_x_hr = msc->touches[id].scroll_x_hr - x;
 		int step_y_hr = msc->touches[id].scroll_y_hr - y;
 
-- 
2.33.1

Re: [PATCH] HID: magicmouse: prevent division by 0 on scroll

[José Expósito]

On Sun, Nov 14, 2021 at 03:53:27AM +0100, Claudia Pellegrino wrote:
> In hid_magicmouse, if the user has set scroll_speed to a value between
> 55 and 63 and scrolls seven times in quick succession, the
> step_hr variable in the magicmouse_emit_touch function becomes 0.
> 
> That causes a division by zero further down in the function when
> it does `step_x_hr /= step_hr`.
> 
> To reproduce, create `/etc/modprobe.d/hid_magicmouse.conf` with the
> following content:
> 
> ```
> options hid_magicmouse scroll_acceleration=1 scroll_speed=55
> ```
> 
> Then reboot, connect a Magic Mouse and scroll seven times quickly.
> The system will freeze for a minute, and after that `dmesg` will
> confirm that a division by zero occurred.
> 
> Enforce a minimum of 1 for the variable so the high resolution
> step count can never reach 0 even at maximum scroll acceleration.
> 
> Fixes: d4b9f10a0eb6 ("HID: magicmouse: enable high-resolution scroll")
> 
> Signed-off-by: Claudia Pellegrino <linux@cpellegrino.de>
> ---
>  drivers/hid/hid-magicmouse.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 686788ebf3e1..d7687ce70614 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -256,8 +256,11 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
>  		unsigned long now = jiffies;
>  		int step_x = msc->touches[id].scroll_x - x;
>  		int step_y = msc->touches[id].scroll_y - y;
> -		int step_hr = ((64 - (int)scroll_speed) * msc->scroll_accel) /
> -			      SCROLL_HR_STEPS;
> +		int step_hr =
> +			max_t(int,
> +			      ((64 - (int)scroll_speed) * msc->scroll_accel) /
> +					SCROLL_HR_STEPS,
> +			      1);
>  		int step_x_hr = msc->touches[id].scroll_x_hr - x;
>  		int step_y_hr = msc->touches[id].scroll_y_hr - y;
>  
> -- 
> 2.33.1
> 

Hi Caludia,

Thanks for ccing me.

I can confirm both that the bug is present and that your patch fixes it.

Tested-by: José Expósito <jose.exposito89@gmail.com>

Best wishes,
Jose

Re: [PATCH] HID: magicmouse: prevent division by 0 on scroll

[Claudia Pellegrino]

Hi José,

> I can confirm both that the bug is present and that your patch fixes it.
Thanks for your testing and feedback!
And kudos for implementing the feature in the first place.


Regards
Claudia

Re: [PATCH] HID: magicmouse: prevent division by 0 on scroll

[Jiri Kosina]

On Sun, 14 Nov 2021, Claudia Pellegrino wrote:

> In hid_magicmouse, if the user has set scroll_speed to a value between
> 55 and 63 and scrolls seven times in quick succession, the
> step_hr variable in the magicmouse_emit_touch function becomes 0.
> 
> That causes a division by zero further down in the function when
> it does `step_x_hr /= step_hr`.
> 
> To reproduce, create `/etc/modprobe.d/hid_magicmouse.conf` with the
> following content:
> 
> ```
> options hid_magicmouse scroll_acceleration=1 scroll_speed=55
> ```
> 
> Then reboot, connect a Magic Mouse and scroll seven times quickly.
> The system will freeze for a minute, and after that `dmesg` will
> confirm that a division by zero occurred.
> 
> Enforce a minimum of 1 for the variable so the high resolution
> step count can never reach 0 even at maximum scroll acceleration.
> 
> Fixes: d4b9f10a0eb6 ("HID: magicmouse: enable high-resolution scroll")
> 
> Signed-off-by: Claudia Pellegrino <linux@cpellegrino.de>

Applied, thank you.

-- 
Jiri Kosina
SUSE Labs