From: Claudio Jeker <claudio@openbsd.org>
Subject: Re: diff: fix mwx(4) firmware download mbuf corruption & tx cleanup race
To: le unium <uniumfr@gmail.com>
Cc: tech@openbsd.org
Date: Tue, 13 Oct 2026 09:14:16 +0200

On Tue, Oct 13, 2026 at 05:59:51AM +0530, le unium wrote:
> hi claudio and tech@,
> 
> while testing mwx(4) on an mt7921 card, the driver hit two panics during the
> firmware download stage at boot:
> 
> 1) pool_cache_item_magic_check: mbufpl cpu free list modified
> 2) kernel: protection fault trap in m_tag_delete_chain

Could you share a dmesg and the stack traces of these panics?
I wonder why I'm not seeing those panics.
 
> with the diff applied, mwx attaches on the mt7921, scans on both 2.4ghz and
> 5ghz, completes wpa2-psk 4w handshakes and doesnt break over 802.11a/ofdm.

Thanks for the diff. On first look it looks like your diff changes too
many things at once and it is unclear what is actually the fix and what
was changed as a test. It will take me some time to review each
hunk.

> --- sys/dev/pci/if_mwx.c.orig	Tue Oct 13 05:41:37 2026
> +++ sys/dev/pci/if_mwx.c	Tue Oct 13 05:49:20 2026
> @@ -2457,7 +2457,7 @@ mwx_dma_alloc(struct mwx_softc *sc)
>  	if ((rv = mwx_queue_alloc(sc, &sc->sc_txmcuq, 256,
>  	    txq_mcu_regbase)) != 0)
>  		return rv;
> -	if ((rv = mwx_queue_alloc(sc, &sc->sc_txfwdlq, 16 /* XXX */,
> +	if ((rv = mwx_queue_alloc(sc, &sc->sc_txfwdlq, 64 /* XXX */,
>  	    MT_TX_FWDL_RING_BASE)) != 0)
>  		return rv;
>  
> @@ -2653,15 +2653,20 @@ mwx_dma_txwi_enqueue(struct mwx_softc *sc, struct mwx_
>  void
>  mwx_dma_tx_cleanup(struct mwx_softc *sc, struct mwx_queue *q)
>  {
> -	struct mwx_queue_data *md;
>  	struct mwx_desc *desc;
> -	int idx, last;
> +	struct mbuf *m;
> +	struct mwx_queue_data *md;
> +	int idx, last, s;
>  
> +	s = splnet();
> +
>  	idx = q->mq_cons;
>  	last = mwx_read(sc, q->mq_regbase + MT_DMA_DMA_IDX);
>  
> -	if (idx == last)
> +	if (idx == last) {
> +		splx(s);
>  		return;
> +	}
>  
>  	bus_dmamap_sync(sc->sc_dmat, q->mq_map, 0, q->mq_map->dm_mapsize,
>  	     BUS_DMASYNC_PREWRITE);
> @@ -2671,11 +2676,13 @@ mwx_dma_tx_cleanup(struct mwx_softc *sc, struct mwx_qu
>  		desc = &q->mq_desc[idx];
>  
>  		if (md->md_mbuf != NULL) {
> +			m = md->md_mbuf;
> +			md->md_mbuf = NULL;
> +
>  			bus_dmamap_sync(sc->sc_dmat, md->md_map, 0,
>  			    md->md_map->dm_mapsize, BUS_DMASYNC_POSTWRITE);
>  			bus_dmamap_unload(sc->sc_dmat, md->md_map);
> -			m_freem(md->md_mbuf);
> -			md->md_mbuf = NULL;
> +			m_freem(m);
>  		}
>  		if (md->md_txwi != NULL) {
>  			/* nothing here, cleanup via mwx_mac_tx_free() */
> @@ -2704,6 +2711,8 @@ mwx_dma_tx_cleanup(struct mwx_softc *sc, struct mwx_qu
>  		q->mq_wakeme = 0;
>  		wakeup(q);
>  	}
> +
> +	splx(s);
>  }
>  
>  void
> @@ -2883,10 +2892,9 @@ mwx_mcu_alloc_msg(size_t len)
>  		}
>  	}
>  
> -	m_align(m, len + headspace);
> -	m->m_pkthdr.len = m->m_len = len + headspace;
> -	memset(mtod(m, caddr_t), 0, m->m_len);
> -	m_adj(m, headspace);
> +	m->m_data += headspace;
> +	m->m_len = m->m_pkthdr.len = len;
> +	memset(mtod(m, caddr_t), 0, len);
>  
>  	return m;
>  }
> @@ -2929,6 +2937,9 @@ mwx_mcu_send_mbuf(struct mwx_softc *sc, uint32_t cmd, 
>  	tot_len = txd_len + len;
>  	KASSERT(m_leadingspace(m) >= txd_len);
>  	m = m_prepend(m, txd_len, M_DONTWAIT);
> +	if (m == NULL)
> +		return ENOBUFS;
> +	m->m_pkthdr.len = tot_len;
>  	txd = mtod(m, uint32_t *);
>  	memset(txd, 0, txd_len);
>  
> @@ -3671,7 +3682,7 @@ mwx_load_firmware(struct mwx_softc *sc)
>  			goto out;
>  		}
>  		rv = mwx_mcu_send_firmware(sc, MCU_CMD_FW_SCATTER,
> -		    dl, len, 4096);
> +		    dl, len, 4096 - sizeof(struct mwx_mcu_txd));
>  		if (rv != 0) {
>  			DPRINTF("%s: failed to send patch\n", DEVNAME(sc));
>  			goto out;
> @@ -3741,7 +3752,7 @@ out:
>  		}
>  
>  		rv = mwx_mcu_send_firmware(sc, MCU_CMD_FW_SCATTER,
> -		    fwbuf + offset, len, 4096);
> +		    fwbuf + offset, len, 4096 - sizeof(struct mwx_mcu_txd));
>  		if (rv != 0) {
>  			DPRINTF("%s: failed to send firmware\n", DEVNAME(sc));
>  			goto fail;
> @@ -3972,7 +3983,7 @@ mwx_mcu_send_firmware(struct mwx_softc *sc, int cmd, u
>      size_t len, size_t max_len)
>  {
>  	size_t cur_len;
> -	int rv;
> +	int i, rv;
>  
>  	while (len > 0) {
>  		cur_len = len;
> @@ -3986,7 +3997,14 @@ mwx_mcu_send_firmware(struct mwx_softc *sc, int cmd, u
>  		data += cur_len;
>  		len -= cur_len;
>  
> -		mwx_dma_tx_cleanup(sc, &sc->sc_txfwdlq);
> +		for (i = 0; i < 1000; i++) {
> +			mwx_dma_tx_cleanup(sc, &sc->sc_txfwdlq);
> +			if (sc->sc_txfwdlq.mq_cons == sc->sc_txfwdlq.mq_prod)
> +				break;
> +			delay(10);
> +		}
> +		if (i >= 1000)
> +			return ETIMEDOUT;
>  	}
>  
>  	return 0;


-- 
:wq Claudio