[patch]fix race between disconnect and open in usbtouch

[patch]fix race between disconnect and open in usbtouch

[Oliver Neukum]

Hi,

if you kill the URB before you unregister, open can come in between
and resubmit the urb. This patch reverses the order.

	Regards
		Oliver

Signed-off-by: Oliver Neukum <oneukum@suse.de>

---

--- linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c.alt	2008-06-26 15:23:34.000000000 +0200
+++ linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c	2008-06-26 15:23:38.000000000 +0200
@@ -973,8 +973,8 @@ static void usbtouch_disconnect(struct u
 
 	dbg("%s - usbtouch is initialized, cleaning up", __FUNCTION__);
 	usb_set_intfdata(intf, NULL);
-	usb_kill_urb(usbtouch->irq);
 	input_unregister_device(usbtouch->input);
+	usb_kill_urb(usbtouch->irq);
 	usb_free_urb(usbtouch->irq);
 	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
 	kfree(usbtouch);

Re: [patch]fix race between disconnect and open in usbtouch

[Ville Syrjälä]

On Thu, Jun 26, 2008 at 03:32:05PM +0200, Oliver Neukum wrote:
> Hi,
> 
> if you kill the URB before you unregister, open can come in between
> and resubmit the urb. This patch reverses the order.
> 
> 	Regards
> 		Oliver
> 
> Signed-off-by: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
> 
> ---
> 
> --- linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c.alt	2008-06-26 15:23:34.000000000 +0200
> +++ linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c	2008-06-26 15:23:38.000000000 +0200
> @@ -973,8 +973,8 @@ static void usbtouch_disconnect(struct u
>  
>  	dbg("%s - usbtouch is initialized, cleaning up", __FUNCTION__);
>  	usb_set_intfdata(intf, NULL);
> -	usb_kill_urb(usbtouch->irq);
>  	input_unregister_device(usbtouch->input);
> +	usb_kill_urb(usbtouch->irq);
>  	usb_free_urb(usbtouch->irq);
>  	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
>  	kfree(usbtouch);

I think you can just remove that usb_kill_urb() call since
usbtouch_close() has already called it by that time.

-- 
Ville Syrjälä
syrjala-ORSVBvAovxo@public.gmane.org
http://www.sci.fi/~syrjala/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch]fix race between disconnect and open in usbtouch

[Oliver Neukum]

Am Donnerstag 26 Juni 2008 15:41:10 schrieb Ville Syrjälä:
> > --- linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c.alt        2008-06-26 15:23:34.000000000 +0200
> > +++ linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c    2008-06-26 15:23:38.000000000 +0200
> > @@ -973,8 +973,8 @@ static void usbtouch_disconnect(struct u
> >  
> >       dbg("%s - usbtouch is initialized, cleaning up", __FUNCTION__);
> >       usb_set_intfdata(intf, NULL);
> > -     usb_kill_urb(usbtouch->irq);
> >       input_unregister_device(usbtouch->input);
> > +     usb_kill_urb(usbtouch->irq);
> >       usb_free_urb(usbtouch->irq);
> >       usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
> >       kfree(usbtouch);
> 
> I think you can just remove that usb_kill_urb() call since
> usbtouch_close() has already called it by that time.

Are you sure? Would you prepare a patch?

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

Re: [patch]fix race between disconnect and open in usbtouch

[Dmitry Torokhov]

On Thu, Jun 26, 2008 at 03:49:26PM +0200, Oliver Neukum wrote:
> Am Donnerstag 26 Juni 2008 15:41:10 schrieb Ville Syrjälä:
> > > --- linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c.alt        2008-06-26 15:23:34.000000000 +0200
> > > +++ linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c    2008-06-26 15:23:38.000000000 +0200
> > > @@ -973,8 +973,8 @@ static void usbtouch_disconnect(struct u
> > >  
> > >       dbg("%s - usbtouch is initialized, cleaning up", __FUNCTION__);
> > >       usb_set_intfdata(intf, NULL);
> > > -     usb_kill_urb(usbtouch->irq);
> > >       input_unregister_device(usbtouch->input);
> > > +     usb_kill_urb(usbtouch->irq);
> > >       usb_free_urb(usbtouch->irq);
> > >       usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
> > >       kfree(usbtouch);
> > 
> > I think you can just remove that usb_kill_urb() call since
> > usbtouch_close() has already called it by that time.
> 
> Are you sure?

Yes, calling dev->close for opened devices as part of unregister is done
by the input core.

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

Re: [patch]fix race between disconnect and open in usbtouch

[Oliver Neukum]

Am Donnerstag 26 Juni 2008 16:30:11 schrieb Dmitry Torokhov:
> > > I think you can just remove that usb_kill_urb() call since
> > > usbtouch_close() has already called it by that time.
> > 
> > Are you sure?
> 
> Yes, calling dev->close for opened devices as part of unregister is done
> by the input core.

OK, thanks.

	Regards
		Oliver

Re: [patch]fix race between disconnect and open in usbtouch

[Alan Stern]

On Thu, 26 Jun 2008, Ville Syrjälä wrote:

> > --- linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c.alt	2008-06-26 15:23:34.000000000 +0200
> > +++ linux-2.6.26-sierra/drivers/input/touchscreen/usbtouchscreen.c	2008-06-26 15:23:38.000000000 +0200
> > @@ -973,8 +973,8 @@ static void usbtouch_disconnect(struct u
> >  
> >  	dbg("%s - usbtouch is initialized, cleaning up", __FUNCTION__);
> >  	usb_set_intfdata(intf, NULL);
> > -	usb_kill_urb(usbtouch->irq);
> >  	input_unregister_device(usbtouch->input);
> > +	usb_kill_urb(usbtouch->irq);
> >  	usb_free_urb(usbtouch->irq);
> >  	usbtouch_free_buffers(interface_to_usbdev(intf), usbtouch);
> >  	kfree(usbtouch);
> 
> I think you can just remove that usb_kill_urb() call since
> usbtouch_close() has already called it by that time.

How do you know that usbtouch_close has been called at this point?

Alan Stern

--
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

Re: [patch]fix race between disconnect and open in usbtouch

[Dmitry Torokhov]

On Thu, Jun 26, 2008 at 03:32:05PM +0200, Oliver Neukum wrote:
> Hi,
> 
> if you kill the URB before you unregister, open can come in between
> and resubmit the urb. This patch reverses the order.
> 

Hm, we just talked about it - it should be simply removed, not moved
around.

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

Re: [patch]fix race between disconnect and open in usbtouch

[Dmitry Torokhov]

On Thu, Jun 26, 2008 at 01:41:31PM -0400, Dmitry Torokhov wrote:
> On Thu, Jun 26, 2008 at 03:32:05PM +0200, Oliver Neukum wrote:
> > Hi,
> > 
> > if you kill the URB before you unregister, open can come in between
> > and resubmit the urb. This patch reverses the order.
> > 
> 
> Hm, we just talked about it - it should be simply removed, not moved
> around.
> 

Oops, I ma sorry - it looks lke the original mail just now arrived into
my mailbox ;)

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