Starting in version 1.56, ucomstop started calling ucomstart to work

around an output freezing problem (see the CVS log for details).  This
is the same approach that sio takes to solve that problem.  However,
ucom has a problem that sio doesn't have.

Consider the case where output is pending, and the device is closed.
ttyclose calls tt_close (which indirects to ucomclose) and then calls
ttyflush which calls tt_stop (which indirects to ucomstop).  Since
ucomclose removed all the usb transfer points, sc_oxfer will be NULL
when ucomstop calls ucomstart.  This results in a null pointer
dereference.

Since calling ucomstart in ucomstart solves other problems, we need to
work with this calling sequence.  The easiest way to do that is to
bail early if sc_oxfer is NULL.

Kazuaki ODA-san came up with this patch, and filed a PR.  I had seen
this bug at work and this patch does seem to solve it.  He had no idea
why it worked, but knew that either this patch, or backing out ucom.c
1.56 fixed his panic.  I just did the legwork of chasing down the code
paths that would cause this, and added a comment.  This is obscure
enough to warrant a comment, I think.

Submitted by: Kazuaki ODA-san
PR: 113964
Approved by: re (bmah)

1 files changed, 10 insertions, 0 deletions

diff --git a/sys/dev/usb/ucom.c b/sys/dev/usb/ucom.c
index f80aa04..aaa06d3 100644
--- a/sys/dev/usb/ucom.c
+++ b/sys/dev/usb/ucom.c
@@ -532,6 +532,16 @@ ucomstart(struct tty *tp)
 	if (sc->sc_dying)
 		return;
 
+	/*
+	 * If there's no sc_oxfer, then ucomclose has removed it.  The buffer
+	 * has just been flushed in the ttyflush() in ttyclose().  ttyflush()
+	 * then calls tt_stop().  ucomstop calls ucomstart, so the right thing
+	 * to do here is just abort if sc_oxfer is NULL, as everything else
+	 * is cleaned up elsewhere.
+	 */
+	if (sc->sc_oxfer == NULL)
+		return;
+
 	s = spltty();
 
 	if (tp->t_state & TS_TBLOCK) {