Re: [patch]race between open and disconnect in usbhid

[Oliver Neukum <oliver@neukum.org>]

Am Montag, 31. März 2008 15:23:50 schrieb Jiri Kosina:
> On Mon, 31 Mar 2008, Oliver Neukum wrote:
> 
> > There is a window:
> > task A					task B
> > spin_lock_irq(&usbhid->inlock);	/* Sync with error handler */
> > usb_set_intfdata(intf, NULL);
> > spin_unlock_irq(&usbhid->inlock);
> > usb_kill_urb(usbhid->urbin);
> > usb_kill_urb(usbhid->urbout);
> > usb_kill_urb(usbhid->urbctrl);
> > 
> > del_timer_sync(&usbhid->io_retry);
> > cancel_work_sync(&usbhid->reset_work);
> > 
> > 						if (!hid->open++) {
> > 							res = usb_autopm_get_interface(usbhid->intf);
> > 							if (res < 0) {
> > 								hid->open--;
> > 								return -EIO;
> > 							}
> > 						}
> > 						if (hid_start_in(hid))
> > 
> > if (hid->claimed & HID_CLAIMED_INPUT)
> > 	hidinput_disconnect(hid);
> > in which an open() to an already disconnected device will submit an URB 
> > to an undead device. In case disconnect() was called by an ioctl, 
> > this'll oops. Fix by introducing a reliable flag an checking it in 
> > hid_start_in().
> 
> Hi Oliver,
> 
> thanks for checking this, good catch.
> 
> > +	char disconnected:1;						/* indicates undead device - no use */
> 
> Wouldn't introduction of new flag (HID_DISCONNECTED) to 
> usbhid_device->iofl be cleaner?

A matter of taste. I don't use atomic ops if I don't have to, but if you
like it better the other way I can redo it.

	Regards
		Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html