svn commit: r249291 - head/sys/dev/firewire

[Will Andrews]

Author: will
Date: Mon Apr  8 23:16:42 2013
New Revision: 249291
URL: http://svnweb.freebsd.org/changeset/base/249291

Log:
  FireWire: Don't allow a tlabel to reference an xfer after free.
  
  sys/dev/firewire/firewire.c:
  - fw_xfer_unload(): Since we are about to free this xfer, call fw_tl_free()
    to remove the xfer from its tlabel's list, if it has a tlabel.
  - In every occasion when a xfer is removed from a tlabel's list, reset
    xfer->tl to -1 while holding fc->tlabel_lock, so that the xfer isn't
    mis-identified as belonging to a tlabel.
  
  This doesn't fix all the use-after-free problems for M_FWMEM, but is an
  incremental towards that goal.
  
  Reviewed by:	kan, sbruno
  Sponsored by:	Spectra Logic

Modified:
  head/sys/dev/firewire/firewire.c

Modified: head/sys/dev/firewire/firewire.c
==============================================================================
--- head/sys/dev/firewire/firewire.c	Mon Apr  8 23:06:25 2013	(r249290)
+++ head/sys/dev/firewire/firewire.c	Mon Apr  8 23:16:42 2013	(r249291)
@@ -374,6 +374,7 @@ firewire_xfer_timeout(void *arg, int pen
 				"tl=0x%x flag=0x%02x\n", i, xfer->flag);
 			fw_dump_hdr(&xfer->send.hdr, "send");
 			xfer->resp = ETIMEDOUT;
+			xfer->tl = -1;
 			STAILQ_REMOVE_HEAD(&fc->tlabels[i], tlabel);
 			STAILQ_INSERT_TAIL(&xfer_timeout, xfer, tlabel);
 		}
@@ -608,6 +609,7 @@ fw_drain_txq(struct firewire_comm *fc)
 		while ((xfer = STAILQ_FIRST(&fc->tlabels[i])) != NULL) {
 			if (firewire_debug)
 				printf("tl=%d flag=%d\n", i, xfer->flag);
+			xfer->tl = -1;
 			xfer->resp = EAGAIN;
 			STAILQ_REMOVE_HEAD(&fc->tlabels[i], tlabel);
 			STAILQ_INSERT_TAIL(&xfer_drain, xfer, tlabel);
@@ -1044,11 +1046,12 @@ fw_tl_free(struct firewire_comm *fc, str
 	struct fw_xfer *txfer;
 	int s;
 
-	if (xfer->tl < 0)
-		return;
-
 	s = splfw();
 	mtx_lock(&fc->tlabel_lock);
+	if (xfer->tl < 0) {
+		mtx_unlock(&fc->tlabel_lock);
+		return;
+	}
 #if 1	/* make sure the label is allocated */
 	STAILQ_FOREACH(txfer, &fc->tlabels[xfer->tl], tlabel)
 		if(txfer == xfer)
@@ -1067,6 +1070,7 @@ fw_tl_free(struct firewire_comm *fc, str
 #endif
 
 	STAILQ_REMOVE(&fc->tlabels[xfer->tl], xfer, fw_xfer, tlabel);
+	xfer->tl = -1;
 	mtx_unlock(&fc->tlabel_lock);
 	splx(s);
 	return;
@@ -1191,6 +1195,11 @@ fw_xfer_unload(struct fw_xfer* xfer)
 		splx(s);
 	}
 	if (xfer->fc != NULL) {
+		/*
+		 * Ensure that any tlabel owner can't access this
+		 * xfer after it's freed.
+		 */
+		fw_tl_free(xfer->fc, xfer);
 #if 1
 		if(xfer->flag & FWXF_START)
 			/*