linux-kernel - Re: [BUG, Regression, bisected] USB mouse causes bug on 1st insert, ignored on 2nd insert, lsusb stuck at usbdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Sep 2010 15:57:25 +0200 (CEST)
From:	Jiri Kosina <jkosina@...e.cz>
To:	Phil Turmel <philip@...mel.org>
Cc:	Mat <jackdachef@...il.com>,
	Guillaume Chazarain <guichaz@...il.com>,
	linux-kernel@...r.kernel.org, Greg Kroah-Hartman <gregkh@...e.de>,
	Alan Stern <stern@...land.harvard.edu>,
	Oliver Neukum <oliver@...kum.org>, Alan Ott <alan@...nal11.us>,
	linux-usb@...r.kernel.org, linux-input@...r.kernel.org,
	Andreas Bombe <aeb@...ian.org>,
	Alex Riesen <raa.lkml@...il.com>,
	Gabriel C <nix.or.die@...glemail.com>,
	Heinz Diehl <htd@...tha.org>
Subject: Re: [BUG, Regression, bisected] USB mouse causes bug on 1st insert,
 ignored on 2nd insert, lsusb stuck at usbdev_open

On Tue, 21 Sep 2010, Jiri Kosina wrote:

> > [   22.705849] HID debug: hiddev_open(): hid: ffff88013a7c0000, hiddev: (null), intf: ffff880137d0ac00
> 
> hiddev_open() has been called. usb_find_interface(&hid_driver, minor) 
> returned interface pointer 0xffff880137d0ac00. Which is bogus! Interface 
> 0xffff880137d0ac00 is not registered with hiddev at all, we should have 
> received 0xffff880137d0a400.
> 
> We currently register minors only for those usbhid devices (through 
> usb_register_dev() in hiddev_connect()) which are going to be claimed by 
> hiddev. It doesn't seem to be problematic to me, and I don't undersntand 
> why usb_find_interface() returns wrong interface.
> 
> I have just found out that it's actually CONFIG_USB_DYNAMIC_MINORS which 
> makes the difference. When unset, the problem doesn't trigger, and 
> usb_find_interface() always returns the proper interface. When 
> CONFIG_USB_DYNAMIC_MINORS is being used, the oops happen.
> 
> I'll look into that.

Could you guys please verify whether the patch below fixes the issues you 
were seeing and puts everything back into shape again? Thanks.


diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3f72924..616b449 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1154,6 +1154,7 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 	char buf[64];
 	unsigned int i;
 	int len;
+	int hiddev_minor = 0;
 
 	if (hdev->quirks & HID_QUIRK_HIDDEV_FORCE)
 		connect_mask |= (HID_CONNECT_HIDDEV_FORCE | HID_CONNECT_HIDDEV);
@@ -1168,8 +1169,8 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 				connect_mask & HID_CONNECT_HIDINPUT_FORCE))
 		hdev->claimed |= HID_CLAIMED_INPUT;
 	if ((connect_mask & HID_CONNECT_HIDDEV) && hdev->hiddev_connect &&
-			!hdev->hiddev_connect(hdev,
-				connect_mask & HID_CONNECT_HIDDEV_FORCE))
+			((hiddev_minor = hdev->hiddev_connect(hdev,
+				connect_mask & HID_CONNECT_HIDDEV_FORCE)) >= 0))
 		hdev->claimed |= HID_CLAIMED_HIDDEV;
 	if ((connect_mask & HID_CONNECT_HIDRAW) && !hidraw_connect(hdev))
 		hdev->claimed |= HID_CLAIMED_HIDRAW;
@@ -1189,7 +1190,7 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 		len += sprintf(buf + len, "input");
 	if (hdev->claimed & HID_CLAIMED_HIDDEV)
 		len += sprintf(buf + len, "%shiddev%d", len ? "," : "",
-				hdev->minor);
+				hiddev_minor);
 	if (hdev->claimed & HID_CLAIMED_HIDRAW)
 		len += sprintf(buf + len, "%shidraw%d", len ? "," : "",
 				((struct hidraw *)hdev->hidraw)->minor);
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 599041a..1f0a770 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1201,6 +1201,9 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
 	hid->driver_data = usbhid;
 	usbhid->hid = hid;
 	usbhid->intf = intf;
+	/* Only hiddev-claimed devices will have corresponding minor number.
+	 * We can't leave it to 0, as that is valid minor as well */
+	usbhid->intf->minor = -1;
 	usbhid->ifnum = interface->desc.bInterfaceNumber;
 
 	init_waitqueue_head(&usbhid->wait);
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 681e620..7a81c1f 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -899,7 +899,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
 		kfree(hiddev);
 		return -1;
 	}
-	return 0;
+	return usbhid->intf->minor;
 }
 
 /*

-- 
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/