Re: ucom panic

[YONETANI Tomokazu <qhwt+dragonfly-bugs@xxxxxxxxxx>]

On Fri, May 28, 2004 at 07:17:05AM +0900, YONETANI Tomokazu wrote:
> On Thu, May 27, 2004 at 03:26:22PM +0200, Eirik Nygaard wrote:
> > On Thu, May 27, 2004 at 08:15:44PM +0900, YONETANI Tomokazu wrote:
> > > Hello.
> > > After the device layer rollup commit, ucom driver panics when
> > > I disconnect from a tip(1) session. Attached is the backtrace
> > > from the panic. The problem is that ucomclose() is releasing
> > > tp->t_dev too early, and ucomstop() passes it to minor()
> > > without chacking the value, resulting in a NULL-pointer dereference.
> > > I'm not sure how to fix this, but I think you have to push down
> > > the following block in usbclose()
> > > 
> > > 	if (tp->t_dev) {
> > > 		release_dev(tp->t_dev);
> > > 		tp->t_dev = NULL;
> > > 	}
> > > 
> > > down to somewhere after the label `quit:'.
> > 
> > That fix looks right as far as I can tell. Have you tried it out?
> > If not, could you do it, and let me know if it does work so I can commit
> > it?
> 
> I was at work and didn't have time to do so, and unfortunately
> the USB serial adapter is in a different place now. I'll do it
> as soon as I get it back.

Attached patch does stop the panic, but I still get warnings from
ucom when it's unloaded:

Jun  2 17:07:06 gzl /kernel: ucom: Warning: cdevsw_remove() called while 1 devic
e refs still exist! (major 138)

Index: dev/usbmisc/ucom/ucom.c
===================================================================
RCS file: /home/source/dragonfly/cvs/src/sys/dev/usbmisc/ucom/ucom.c,v
retrieving revision 1.15
diff -u -r1.15 ucom.c
--- dev/usbmisc/ucom/ucom.c	19 May 2004 22:52:51 -0000	1.15
+++ dev/usbmisc/ucom/ucom.c	2 Jun 2004 06:11:59 -0000
@@ -476,11 +476,6 @@
 	DPRINTF(("%s: ucomclose: unit = %d\n",
 		USBDEVNAME(sc->sc_dev), UCOMUNIT(dev)));
 
-	if (tp->t_dev) {
-		release_dev(tp->t_dev);
-		tp->t_dev = NULL;
-	}
-
 	if (!ISSET(tp->t_state, TS_ISOPEN))
 		goto quit;
 
@@ -505,7 +500,12 @@
 	if (sc->sc_callback->ucom_close != NULL)
 		sc->sc_callback->ucom_close(sc->sc_parent, sc->sc_portno);
 
-    quit:
+quit:
+	if (tp->t_dev) {
+		release_dev(tp->t_dev);
+		tp->t_dev = NULL;
+	}
+
 	if (--sc->sc_refcnt < 0)
 		usb_detach_wakeup(USBDEV(sc->sc_dev));