[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#780055: linux: Kernel crash with ThingM blink(1) mk2 device



Source: linux
Severity: normal
Tags: patch

How to reproduce:
Run
   while true; do blink1-tool --list; done
and insert and remove the blink(1) device a few times. This will crash the
kernel with high probability. blink1-tool can be obtained from [0].

This bug was fixed upstream in kernel 3.18 with commit
67a97845830f79584c9db8849ac723e5d2d57f65, see attached patch. The patch
applies cleanly to the current Jessie linux kernel sources and fixes the
issue for me.

[0] https://github.com/todbot/blink1/

-- System Information:
Debian Release: 8.0
  APT prefers testing-updates
  APT policy: (500, 'testing-updates'), (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 3.16.0-4-amd64 (SMP w/8 CPU cores)
Locale: LANG=en_US.utf8, LC_CTYPE=en_US.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
commit 67a97845830f79584c9db8849ac723e5d2d57f65
Author: Jiri Kosina <jkosina@suse.cz>
Date:   Thu Sep 4 08:56:06 2014 +0200

    HID: thingm: fix workqueue race on remove
    
    thingm_remove_rgb() needs to flush the workqueue after all the LED classes
    have been unregistered, otherwise the removal might race with another LED
    event coming, causing thingm_led_set() to schedule additional work after
    thingm_remove_rgb() has flushed it. This obviously causes oops later, as
    the scheduled work has been freed in the meantime.
    
    In addition to that, move the hid_hw_stop() to an earlier place, so that
    dmesg is not polluted by failure messages about not being able to write
    the LED while the device is being shut down.
    
    Reported-and-tested-by: Dylan Alex Simon <dylan-kernel@dylex.net>
    Signed-off-by: Jiri Kosina <jkosina@suse.cz>

diff --git a/drivers/hid/hid-thingm.c b/drivers/hid/hid-thingm.c
index 134be89..f206398 100644
--- a/drivers/hid/hid-thingm.c
+++ b/drivers/hid/hid-thingm.c
@@ -208,10 +208,10 @@ unregister_red:
 
 static void thingm_remove_rgb(struct thingm_rgb *rgb)
 {
-	flush_work(&rgb->work);
 	led_classdev_unregister(&rgb->red.ldev);
 	led_classdev_unregister(&rgb->green.ldev);
 	led_classdev_unregister(&rgb->blue.ldev);
+	flush_work(&rgb->work);
 }
 
 static int thingm_probe(struct hid_device *hdev, const struct hid_device_id *id)
@@ -286,10 +286,10 @@ static void thingm_remove(struct hid_device *hdev)
 	struct thingm_device *tdev = hid_get_drvdata(hdev);
 	int i;
 
+	hid_hw_stop(hdev);
+
 	for (i = 0; i < tdev->fwinfo->numrgb; ++i)
 		thingm_remove_rgb(tdev->rgb + i);
-
-	hid_hw_stop(hdev);
 }
 
 static const struct hid_device_id thingm_table[] = {

Reply to: