HID: magicmouse: Fix an error handling path in magicmouse_probe()

Started by Christophe JAILLET on Dec 28, 2021 · 4 messages from 3 authors · Last reply Jan 6, 2022

Subsystems (2)

Maintainers (3)

STALE1773d

open the first message on its own page

Message details and related content
If the timer introduced by the commit below is started, then it must be
deleted in the error handling of the probe. Otherwise it would trigger
once the driver is no more.

Fixes: 0b91b4e4dae6 ("HID: magicmouse: Report battery level over USB")
Signed-off-by: Christophe JAILLET <redacted>
---
 drivers/hid/hid-magicmouse.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index eba1e8087bfd..b8b08f0a8c54 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -873,6 +873,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 
 	return 0;
 err_stop_hw:
+	del_timer_sync(&msc->battery_timer);
 	hid_hw_stop(hdev);
 	return ret;
 }
-- 
2.32.0
Message details and related content
On Tue, Dec 28, 2021 at 10:09:17PM +0100, Christophe JAILLET wrote:
quoted hunk
If the timer introduced by the commit below is started, then it must be
deleted in the error handling of the probe. Otherwise it would trigger
once the driver is no more.

Fixes: 0b91b4e4dae6 ("HID: magicmouse: Report battery level over USB")
Signed-off-by: Christophe JAILLET <redacted>
---
 drivers/hid/hid-magicmouse.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index eba1e8087bfd..b8b08f0a8c54 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -873,6 +873,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 
 	return 0;
 err_stop_hw:
+	del_timer_sync(&msc->battery_timer);
 	hid_hw_stop(hdev);
 	return ret;
 }
-- 
2.32.0
My bad, thanks for catching it!

Tested-by: José Expósito <redacted>
Message details and related content
Le 29/12/2021 à 08:50, José Expósito a écrit :
On Tue, Dec 28, 2021 at 10:09:17PM +0100, Christophe JAILLET wrote:
quoted
If the timer introduced by the commit below is started, then it must be
deleted in the error handling of the probe. Otherwise it would trigger
once the driver is no more.

Fixes: 0b91b4e4dae6 ("HID: magicmouse: Report battery level over USB")
Signed-off-by: Christophe JAILLET <redacted>
---
  drivers/hid/hid-magicmouse.c | 1 +
  1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index eba1e8087bfd..b8b08f0a8c54 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -873,6 +873,7 @@ static int magicmouse_probe(struct hid_device *hdev,
  
  	return 0;
  err_stop_hw:
+	del_timer_sync(&msc->battery_timer);
  	hid_hw_stop(hdev);
  	return ret;
  }
-- 
2.32.0
My bad, thanks for catching it!

Tested-by: José Expósito <redacted>
Hi, just in case, I got a reply from syzbot that this patch fixes:

https://syzkaller.appspot.com/bug?id=ae4e9aaf5651e1d6895071208c7844d4fdfbe30c

If it is the same issue, we can add:
Reported-by: <redacted>


I've not found it with syzbot, but with a coccinelle script which tries to spot things that are in the remove function and should also be in the error handling path of the probe.

However, if it help syzbot, I don't care mentioning it.

CJ
Message details and related content
On Tue, 28 Dec 2021, Christophe JAILLET wrote:
quoted hunk
If the timer introduced by the commit below is started, then it must be
deleted in the error handling of the probe. Otherwise it would trigger
once the driver is no more.

Fixes: 0b91b4e4dae6 ("HID: magicmouse: Report battery level over USB")
Signed-off-by: Christophe JAILLET <redacted>
---
 drivers/hid/hid-magicmouse.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index eba1e8087bfd..b8b08f0a8c54 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -873,6 +873,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 
 	return 0;
 err_stop_hw:
+	del_timer_sync(&msc->battery_timer);
 	hid_hw_stop(hdev);
 	return ret;
 }
Applied, thanks.

-- 
Jiri Kosina
SUSE Labs
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help