2021-11-14 02:55:16

by Claudia Pellegrino

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

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 <[email protected]>
---
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



2021-11-14 15:34:15

by José Expósito

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

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 <[email protected]>
> ---
> 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 <[email protected]>

Best wishes,
Jose

2021-11-14 20:19:30

by Claudia Pellegrino

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

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

2021-11-19 14:54:47

by Jiri Kosina

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

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 <[email protected]>

Applied, thank you.

--
Jiri Kosina
SUSE Labs