usb/113964: [patch] ucom(4): kernel panic when dropping a connection

[M. Warner Losh]

In message: <200706240650.l5O6o7CZ040728 at freefall.freebsd.org>
            "M. Warner Losh" <imp at bsdimp.com> writes:
: The following reply was made to PR usb/113964; it has been noted by GNATS.
: 
: From: "M. Warner Losh" <imp at bsdimp.com>
: To: kazuaki at aliceblue.jp
: Cc: FreeBSD-gnats-submit at FreeBSD.ORG, re at FreeBSD.ORG
: Subject: Re: usb/113964: [patch] ucom(4): kernel panic when dropping a
:  connection
: Date: Sun, 24 Jun 2007 00:48:08 -0600 (MDT)
: 
:  In message: <467D7AE6.2090302 at aliceblue.jp>
:              Kazuaki ODA <kazuaki at aliceblue.jp> writes:
:  : M. Warner Losh wrote:
:  : > In message: <200706231136.l5NBa61H001748 at eyes.aliceblue.jp>
:  : >             Kazuaki ODA <kazuaki at aliceblue.jp> writes:
:  : > : 	I don't know the proper fix but the following patch is workaround for
:  : > : 	me.
:  : > : 
:  : > : --- ucom.c.patch begins here ---
:  : > : --- sys/dev/usb/ucom.c.orig	2007-06-22 23:45:37.000000000 +0900
:  : > : +++ sys/dev/usb/ucom.c	2007-06-23 17:47:18.000000000 +0900
:  : > : @@ -532,6 +532,9 @@
:  : > :  	if (sc->sc_dying)
:  : > :  		return;
:  : > :  
:  : > : +	if (sc->sc_oxfer == NULL)
:  : > : +		return;
:  : > : +
:  : > :  	s = spltty();
:  : > :  
:  : > :  	if (tp->t_state & TS_TBLOCK) {
:  : > : --- ucom.c.patch ends here ---
:  : > 
:  : > This is a good workaround.  However, why does the tty->t_oproc get
:  : > called after the tty->t_close routine which sets sc->sc_oxfer to NULL?
:  : > 
:  : > Warner
:  : 
:  : Unfortunately, I have no idea.  But it seems the changes of tty 
:  : subsystem and ucom.c rev. 1.56 are related to the issue.
:  : 
:  : * On 5.x-RELEASE, where ucom.c rev. 1.56 has been MFCed, we get no panic.
:  : 
:  : * On 6.x-RELEASE or 7.0-CURRENT, we get a panic.
:  : 
:  : * After backing out ucom.c rev. 1.56, no panic on 6.x-RELEASE or 
:  : 7.0-CURRENT.
:  
:  I think that the problem is the addition of ucomstart() at the end of
:  ucomstop().  I think that the tty layer calls t_stop() as part of the
:  ttyflush() routine.  ttyflush() is called inside of tty_close(), which
:  is called after the tt_close() call in ttyclose().  The addition
:  appears to have been taken from sio, either directly, or at the
:  recommendation of Bruce.  The sio routines are written such that the
:  close doesn't delete data that's referenced later.  In our case, it is
:  the right thing to do in ucomclose(), so we need to guard against it
:  later when the data might be used.  Since ucom chose to have
:  ucomstart() called from ucomstop(), I think it is up to ucom to guard
:  against known calling paths.  I think this is not only a good
:  workaround, therefore, but may be a correct fix on its own merit.

I don't know if we need to do something to discard the characters in
the buffer or not.

Warner