Bug 188921

Summary: Function hid_reset_resume() incorrectly check the return value of hid_post_reset()
Product: Drivers Reporter: bianpan (bianpan2010)
Component: Input DevicesAssignee: drivers_input-devices
Status: RESOLVED CODE_FIX    
Severity: normal CC: dmitry.torokhov
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: linux-4.9-rc6 Subsystem:
Regression: No Bisected commit-id:

Description bianpan 2016-11-25 11:09:01 UTC
In file drivers/hid/usbhid/hid-core.c, function hid_post_reset() returns 0 on success, or 1 on failures. However, in function hid_reset_resume(), it is taken as success as long as the return value of hid_post_reset() is not less than 0 (see the check statement at line 1593). As a result, errors in hid_post_reset() will be ignored. Maybe it is better to check the return value of hid_post_reset() by "== 0" rather than ">= 0". Codes related to this bug are summarised as follows.

hid_reset_resume @@ drivers/hid/usbhid/hid-core.c
1587 static int hid_reset_resume(struct usb_interface *intf)
1588 {
1589     struct hid_device *hid = usb_get_intfdata(intf);
1590     int status;
1591 
1592     status = hid_post_reset(intf);
         // "status >= 0" will always be satisfied. use "status == 0"?
1593     if (status >= 0 && hid->driver && hid->driver->reset_resume) {
1594         int ret = hid->driver->reset_resume(hid);
1595         if (ret < 0)
1596             status = ret;
1597     }
1598     return status;
1599 }

hid_post_reset @@ drivers/hid/usbhid/hid-core.c
   // hid_post_reset() returns 1 on failures, and 0 on success 
1445 static int hid_post_reset(struct usb_interface *intf)
1446 {
         ...
1451     int status;
         ...
1459     rdesc = kmalloc(hid->dev_rsize, GFP_KERNEL);
1460     if (!rdesc) {
1461         dbg_hid("couldn't allocate rdesc memory (post_reset)\n");
1462         return 1;
1463     }
1464     status = hid_get_class_descriptor(dev,
1465                 interface->desc.bInterfaceNumber,
1466                 HID_DT_REPORT, rdesc, hid->dev_rsize);
1467     if (status < 0) {
1468         dbg_hid("reading report descriptor failed (post_reset)\n");
1469         kfree(rdesc);
1470         return 1;
1471     }
         ...
1488     return 0;
1489 }

Thanks very much!
Comment 1 Dmitry Torokhov 2016-12-12 22:55:59 UTC
Fixed with c60fa555b11b25386b355c141d90cbee361c3eec (will be merged soon).