Re: [PATCH] usb: fix possible oops in serial option driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 30 May 2012, Johan Hovold wrote:

> On Fri, May 25, 2012 at 01:46:48PM -0400, Alan Stern wrote:
> > On Fri, 25 May 2012, Johan Hovold wrote:
> > 
> > > > Well tty-core shouldn't handle disconnect (which is usb-serial-
> > > > specific)
> > > 
> > > Just to clarify: the tty layer could learn to deal with disconnect
> > > (useful also for cdc-acm for example), but currently it is handled in
> > > usb-serial core so moving it there first for dtr_rts makes sense.
> > 
> > The tty layer may not know how to handle disconnect, but it certainly 
> > does know about handling tty_unregister_device().  Since that routine 
> > gets called by the usb-serial core as part of disconnect processing, 
> > what more is really needed?
> 
> You're right, and this is indeed a bug in the tty layer. In fact, there
> are three different issues interacting here which are related to the
> oops Jan discovered:
> 
> 	1. hangup race in tty

Alan Cox will have to respond to this one.  I'm not sufficiently 
familiar with the details of when the tty core is supposed to invoke 
its various callbacks.

> 	2. access to interface after disconnect
> 	3. io after disconnect (e.g. during close)

...

> 3. io after disconnect (e.g. during close)
> ------------------------------------------
> You asked in reply to my patch why dtr_rts was special and needed to be
> protected against disconnect. It's treated with extra care by several
> drivers since Oliver introduced the disconnected flag:
> 
> 	http://www.mail-archive.com/linux-usb@xxxxxxxxxxxxxxx/msg00207.html
> 
> It used to be handled in driver close() but was later factored out to to
> dtr_rts(). My patch simply refactored this protection to usb-serial
> core.
> 
> But as far as I can see, with the modifications that has since been done
> to the tty layer, if the tty layer handled hangup in cases such as the
> above we could get rid of this special handling as well?

It does seem that way.  Or even if the tty layer handled deregistration 
properly, never mind hangup.

> In fact, it seems to me we can even get rid of the disconnected checks
> in close as vhangup will either call driver close (shutdown) or not
> return until an ongoing close has finished. This should be enough to
> uphold the rule: "You are not allowed any IO to a device after returning
> from this [disconnect] callback" which was what the disconnected checks
> in close was introduced for, right?

Yes -- provided Alan Cox agrees.

Alan Stern

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux