DF-3638 / fix.diff
--- a/sys/dev/netif/ath/ath/if_ath_tx.c +++ b/sys/dev/netif/ath/ath/if_ath_tx.c @@ -4566,16 +4566,26 @@ ath_tx_retry_clone(struct ath_softc *sc, struct ath_node *an, /* Setup the dma for the new buffer */ error = ath_tx_dmasetup(sc, nbf, nbf->bf_m); if (error != 0) { DPRINTF(sc, ATH_DEBUG_XMIT, "%s: failed to setup dma for clone\n", __func__); /* + * ath_tx_dmasetup() has already freed the mbuf chain on + * failure, and nbf owns the node reference moved over by + * ath_buf_clone(). Drop that reference and detach both + * pointers before recycling the buffer, or the node is + * leaked forever and bf_m dangles at freed memory. + */ + if (nbf->bf_node != NULL) { + ieee80211_node_decref(nbf->bf_node); + nbf->bf_node = NULL; + } + nbf->bf_m = NULL; + /* * Put this at the head of the list, not tail; * that way it doesn't interfere with the * busy buffer logic (which uses the tail of * the list.) */ ATH_TXBUF_LOCK(sc); ath_returnbuf_head(sc, nbf); --- a/sys/dev/netif/ath/ath/if_ath.c +++ b/sys/dev/netif/ath/ath/if_ath.c @@ -3133,6 +3133,12 @@ _ath_getbuf_locked(struct ath_softc *sc, ath_buf_type_t btype) bf->bf_next = NULL; /* XXX just to make sure */ bf->bf_last = NULL; /* XXX again, just to make sure */ bf->bf_comp = NULL; /* XXX again, just to make sure */ + /* + * Defense in depth: never hand out a buffer with stale + * mbuf/node pointers left over from a previous owner + * (matches the ath_freebuf() KASSERT contract). + */ + bf->bf_m = NULL; + bf->bf_node = NULL; bzero(&bf->bf_state, sizeof(bf->bf_state)); |