Hi,
The snippet I quoted was from the Ubuntu 10.04 LTS we're using. Apparently, this issue (among others) was fixed in the latest 2.6 kernel, so please ignore this report.
Regards,
Robert
> -----Original Message-----
> From: Robert Lukassen
> Sent: Thursday, January 13, 2011 1:56 PM
> To: linux-usb@xxxxxxxxxxxxxxx
> Subject: Potential trouble in usbhid
>
> Hi all,
>
> We've experienced some issues with an application that uses
> hiddev devices. This application regularly performs ioctl
> syscalls on a hiddev device (/dev/usb/hiddev0) with the
> intent to get report information. This works fine, until the
> device is physically disconnected.
>
> For context, here's a snippet from drivers/hid/usb/hiddev.c:
>
> -------
> static long hiddev_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg) {
> struct hiddev_list *list = file->private_data;
> struct hiddev *hiddev = list->hiddev;
> struct hid_device *hid = hiddev->hid;
> struct usb_device *dev = hid_to_usb_dev(hid);
> struct hiddev_collection_info cinfo;
> struct hiddev_report_info rinfo;
> struct hiddev_field_info finfo;
> struct hiddev_devinfo dinfo;
> struct hid_report *report;
> struct hid_field *field;
> struct usbhid_device *usbhid = hid->driver_data;
> void __user *user_arg = (void __user *)arg;
> int i, r;
>
> /* Called without BKL by compat methods so no BKL taken */
>
> /* FIXME: Who or what stop this racing with a disconnect ?? */
> if (!hiddev->exist)
> return -EIO;
> -------
>
> We found that in rare circumstances, the deference in the
> third line fails, i.e. list->hiddev does not hold a valid
> pointer. Here's the trace from the kernel:
>
> Jan 13 10:32:31 ehv-nb-001 kernel: [ 7525.637584] EIP:
> 0060:[<f80d4d7c>] EFLAGS: 00010292 CPU: 0 Jan 13 10:32:31
> ehv-nb-001 kernel: [ 7525.637596] EIP is at
> hiddev_ioctl+0x2c/0x680 [usbhid] Jan 13 10:32:31 ehv-nb-001
> kernel: [ 7525.637601] EAX: 00000000 EBX: ee538000 ECX:
> bf8d78a8 EDX: 4018480c Jan 13 10:32:31 ehv-nb-001 kernel: [
> 7525.637607] ESI: ee24dd80 EDI: f08e0000 EBP: f5663f50 ESP:
> f5663ec8 Jan 13 10:32:31 ehv-nb-001 kernel: [ 7525.637612]
> DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 Jan 13 10:32:31
> ehv-nb-001 kernel: [ 7525.637627] 00000000 f5663ed8 bf8d78a8
> 00000000 00000001 ffffffff f3b64300 00000000 Jan 13 10:32:31
> ehv-nb-001 kernel: [ 7525.637641] <0> 00000000 00000000
> 00000000 f6bd9980 00000000 00000000 00000000 f6bd9980 Jan 13
> 10:32:31 ehv-nb-001 kernel: [ 7525.637657] <0> c0167b50
> f5663f0c f5663f0c 00000000 00000000 f5663e70 f5663f28
> c03bb5dd Jan 13 10:32:31 ehv-nb-001 kernel: [ 7525.637687]
> [<c0167b50>] ? autoremove_wake_function+0x0/0x50 Jan 13
> 10:32:31 ehv-nb-001 kernel: [ 7525.637697] [<c03bb5dd>] ?
> tty_ldisc_deref+0xd/0x10 Jan 13 10:32:31 ehv-nb-001 kernel: [
> 7525.637707] [<c02f5c14>] ? security_file_permission+0x14/0x20
> Jan 13 10:32:31 ehv-nb-001 kernel: [ 7525.637717]
> [<c0208a24>] ? rw_verify_area+0x64/0xe0 Jan 13 10:32:31
> ehv-nb-001 kernel: [ 7525.637743] [<f80d4d50>] ?
> hiddev_ioctl+0x0/0x680 [usbhid] Jan 13 10:32:31 ehv-nb-001
> kernel: [ 7525.637756] [<c0217211>] ? vfs_ioctl+0x21/0x90
> Jan 13 10:32:31 ehv-nb-001 kernel: [ 7525.637764]
> [<c0171b96>] ? getnstimeofday+0x56/0x110 Jan 13 10:32:31
> ehv-nb-001 kernel: [ 7525.637771] [<c02174f9>] ?
> do_vfs_ioctl+0x79/0x310 Jan 13 10:32:31 ehv-nb-001 kernel: [
> 7525.637779] [<c03549a9>] ? copy_to_user+0x39/0x130 Jan 13
> 10:32:31 ehv-nb-001 kernel: [ 7525.637786] [<c02177f7>] ?
> sys_ioctl+0x67/0x80 Jan 13 10:32:31 ehv-nb-001 kernel: [
> 7525.637794] [<c01033ec>] ? syscall_call+0x7/0xb Jan 13
> 10:32:31 ehv-nb-001 kernel: [ 7525.638001] ---[ end trace
> beb5a9ffee100cae ]---
>
> hiddev_ioctl+0x2c is a dereference of EAX, which is 00000000.
>
> It seems to be a problem related to the 'unbinding' of hiddev
> from the list, resulting in list->hiddev being 0, while
> 'file' still refers to a valid file. We suspect that the
> 'FIXME' mentioned is also related, except that in our case
> sometimes the 'hiddev' is gone.
>
> We've patched these lines:
>
> ------
> static long hiddev_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg) {
> struct hiddev_list *list = file->private_data;
> struct hiddev *hiddev = list->hiddev;
> struct hid_device *hid;
> struct usb_device *dev;
> struct hiddev_collection_info cinfo;
> struct hiddev_report_info rinfo;
> struct hiddev_field_info finfo;
> struct hiddev_devinfo dinfo;
> struct hid_report *report;
> struct hid_field *field;
> struct usbhid_device *usbhid;
> void __user *user_arg;
> int i, r;
>
> if (hiddev == NULL)
> return -EIO;
>
> hid = hiddev->hid;
>
> if (hid == NULL || &(hid->dev) == NULL ||
> hid->dev.parent == NULL)
> return -EIO;
>
> dev = hid_to_usb_dev(hid);
>
> usbhid = hid->driver_data;
> user_arg = (void __user *)arg;
> ------
>
> This seems to be enough to solve the symptom. Not sure
> whether the same caution should be applied to the
> dereferencing of 'list', or even 'file' for that matter.
>
> Regards,
>
> Robert Lukassen
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
