From 528b271d19c1d99c4810f13ed1b36d4c80383306 Mon Sep 17 00:00:00 2001 From: will Date: Mon, 8 Apr 2013 23:16:42 +0000 Subject: 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 --- sys/dev/firewire/firewire.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'sys/dev/firewire/firewire.c') diff --git a/sys/dev/firewire/firewire.c b/sys/dev/firewire/firewire.c index aa0f1aa..f1ebb6d 100644 --- a/sys/dev/firewire/firewire.c +++ b/sys/dev/firewire/firewire.c @@ -374,6 +374,7 @@ firewire_xfer_timeout(void *arg, int pending) "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, struct fw_xfer *xfer) 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, struct fw_xfer *xfer) #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) /* -- cgit v1.1