Date: Mon, 8 Apr 2013 23:16:42 +0000 (UTC) From: Will Andrews <will@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r249291 - head/sys/dev/firewire Message-ID: <201304082316.r38NGgLZ060737@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
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) /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201304082316.r38NGgLZ060737>