| Message ID | 20211114025327.146897-1-linux@cpellegrino.de (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | a1091118e0d6d84c2fdb94e6c397ac790bfb9dd6 |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: magicmouse: prevent division by 0 on scroll | expand |
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
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
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.
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;
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(-)