Download raw body.
small set of qwx bug fixes
On Mon, Jun 22, 2026 at 12:57:06AM +0200, Stefan Sperling wrote:
> i. Dealloc resources via qwx_stop() if we fail to get RUNNING in qwx_init().
> Cleanup via qwx_stop() was only performed in case of complete success,
> i.e. once IFF_RUNNING is set.
The above change reintroduced an issue where firmware is not getting
stopped properly and uses freed buffers during DMA, triggering mbuf
cluster use-after-free checks.
Below is a revised patch which aims to address this problem, with all
my pending changes rolled in.
Please test this diff instead of previous versions.
I have this queued as 7 separate commits with a rationale for each change
in log messages, to make change history easier to follow.
M sys/dev/ic/qwx.c | 64+ 48-
M sys/dev/pci/if_qwx_pci.c | 2+ 1-
2 files changed, 66 insertions(+), 49 deletions(-)
commit - 510a3c8701b503ed3d6bbf6e2c2c1d66315cb14c
commit + d7ec0f51335bd2d1bafe3b34722ae728b12fba4b
blob - 4c744b205a09dec658faed98adb3e4e7a7f557be
blob + bd67bf6b3fb868108c968c294e3196c01cc7aa60
--- sys/dev/ic/qwx.c
+++ sys/dev/ic/qwx.c
@@ -271,13 +271,16 @@ qwx_init(struct ifnet *ifp)
sc->scan.state = ATH11K_SCAN_IDLE;
sc->vdev_id_11d_scan = QWX_11D_INVALID_VDEV_ID;
- error = qwx_core_init(sc);
- if (error)
- return error;
-
memset(&sc->qrtr_server, 0, sizeof(sc->qrtr_server));
sc->qrtr_server.node = QRTR_NODE_BCAST;
+ /* This flag will be cleared if firmware starts up successfully. */
+ set_bit(ATH11K_FLAG_QMI_FAIL, sc->sc_flags);
+
+ error = qwx_core_init(sc);
+ if (error)
+ return error;
+
/* wait for QRTR init to be done */
while (sc->qrtr_server.node == QRTR_NODE_BCAST) {
error = tsleep_nsec(&sc->qrtr_server, 0, "qwxqrtr",
@@ -309,19 +312,18 @@ qwx_init(struct ifnet *ifp)
sc->sc_dev.dv_xname, ether_sprintf(ic->ic_myaddr),
error);
- ieee80211_media_init(ifp, qwx_media_change,
+ ieee80211_media_init(ifp, ieee80211_media_change,
ieee80211_media_status);
}
if (ifp->if_flags & IFF_UP) {
- refcnt_init(&sc->task_refs);
-
- ifq_clr_oactive(&ifp->if_snd);
-
error = qwx_mac_start(sc);
if (error)
return error;
+ refcnt_init(&sc->task_refs);
+ ifq_clr_oactive(&ifp->if_snd);
+
ifp->if_flags |= IFF_RUNNING;
sc->ops.irq_enable(sc);
ieee80211_begin_scan(ifp);
@@ -414,10 +416,12 @@ qwx_stop(struct ifnet *ifp)
struct qwx_softc *sc = ifp->if_softc;
struct ieee80211com *ic = &sc->sc_ic;
int s = splnet();
+ int was_running;
rw_assert_wrlock(&sc->ioctl_rwl);
- if (ic->ic_opmode == IEEE80211_M_STA &&
+ was_running = (ifp->if_flags & IFF_RUNNING) != 0;
+ if (was_running && ic->ic_opmode == IEEE80211_M_STA &&
ic->ic_state == IEEE80211_S_RUN &&
(ic->ic_bss->ni_flags & IEEE80211_NODE_MFP) &&
ic->ic_bss->ni_port_valid)
@@ -433,7 +437,8 @@ qwx_stop(struct ifnet *ifp)
/* Cancel scheduled tasks and let any stale tasks finish up. */
task_del(systq, &sc->init_task);
qwx_del_task_all(sc);
- refcnt_finalize(&sc->task_refs, "qwxstop");
+ if (was_running)
+ refcnt_finalize(&sc->task_refs, "qwxstop");
ifp->if_timer = sc->sc_tx_timer = 0;
@@ -444,20 +449,21 @@ qwx_stop(struct ifnet *ifp)
sc->bgscan_unref_arg = NULL;
sc->bgscan_unref_arg_size = 0;
- clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags);
-
/*
* Manually run the newstate task's code for switching to INIT state.
* This reconfigures firmware state to stop scanning, or disassociate
* from our current AP, and/or stop the VIF, etc.
*/
- if (ic->ic_state != IEEE80211_S_INIT) {
+ if (was_running && ic->ic_state != IEEE80211_S_INIT) {
sc->ns_nstate = IEEE80211_S_INIT;
sc->ns_arg = -1; /* do not send management frames */
refcnt_init(&sc->task_refs);
refcnt_take(&sc->task_refs);
+ clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags);
qwx_newstate_task(sc);
- if (ic->ic_state != IEEE80211_S_INIT) { /* task code failed */
+ set_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags);
+ if (ic->ic_state != IEEE80211_S_INIT) {
+ /* task code failed */
task_del(systq, &sc->init_task);
sc->sc_newstate(ic, IEEE80211_S_INIT, -1);
}
@@ -472,7 +478,14 @@ qwx_stop(struct ifnet *ifp)
sc->vdev_id_11d_scan = QWX_11D_INVALID_VDEV_ID;
sc->pdevs_active = 0;
- /* power off hardware */
+ /*
+ * If we were running then allow commands to be sent to
+ * firmware during core_deinit().
+ */
+ if (was_running)
+ clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags);
+
+ /* free some DMA allocations and power off hardware */
qwx_core_deinit(sc);
qwx_vif_free_all(sc);
@@ -517,6 +530,10 @@ qwx_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
/* Force reload of firmware image from disk. */
qwx_free_firmware(sc);
err = qwx_init(ifp);
+ if (err) {
+ KASSERT(!(ifp->if_flags & IFF_RUNNING));
+ qwx_stop(ifp);
+ }
}
} else {
if (ifp->if_flags & IFF_RUNNING)
@@ -681,24 +698,6 @@ qwx_watchdog(struct ifnet *ifp)
}
int
-qwx_media_change(struct ifnet *ifp)
-{
- int err;
-
- err = ieee80211_media_change(ifp);
- if (err != ENETRESET)
- return err;
-
- if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) ==
- (IFF_UP | IFF_RUNNING)) {
- qwx_stop(ifp);
- err = qwx_init(ifp);
- }
-
- return err;
-}
-
-int
qwx_queue_setkey_cmd(struct ieee80211com *ic, struct ieee80211_node *ni,
struct ieee80211_key *k, int cmd)
{
@@ -15353,6 +15352,9 @@ qwx_dp_rxdma_ring_buf_setup(struct qwx_softc *sc,
if (rx_ring->rx_data == NULL)
return ENOMEM;
+ rx_ring->bufs_max = num_entries;
+ memset(rx_ring->freemap, 0xff, sizeof(rx_ring->freemap));
+
for (i = 0; i < num_entries; i++) {
struct qwx_rx_data *rx_data = &rx_ring->rx_data[i];
@@ -15361,9 +15363,6 @@ qwx_dp_rxdma_ring_buf_setup(struct qwx_softc *sc,
return ENOMEM;
}
- rx_ring->bufs_max = num_entries;
- memset(rx_ring->freemap, 0xff, sizeof(rx_ring->freemap));
-
return qwx_dp_rxbufs_replenish(sc, dp->mac_id, rx_ring, num_entries,
sc->hw_params.hal_params->rx_buf_rbm);
}
@@ -20786,10 +20785,11 @@ qwx_flush_rx_rings(struct qwx_softc *sc)
void
qwx_core_stop(struct qwx_softc *sc)
{
- if (!test_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags))
+ if (!test_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags) &&
+ !test_bit(ATH11K_FLAG_QMI_FAIL, sc->sc_flags)) {
qwx_qmi_firmware_stop(sc);
-
- qwx_flush_rx_rings(sc);
+ qwx_flush_rx_rings(sc);
+ }
sc->ops.stop(sc);
qwx_wmi_detach(sc);
@@ -20921,7 +20921,8 @@ qwx_core_qmi_firmware_ready(struct qwx_softc *sc)
default:
printf("%s: invalid crypto_mode: %d\n",
sc->sc_dev.dv_xname, sc->crypto_mode);
- return EINVAL;
+ ret = EINVAL;
+ goto err_dp_free;
}
if (sc->frame_mode == ATH11K_HW_TXRX_RAW)
@@ -20970,7 +20971,7 @@ err_firmware_stop:
return ret;
}
-void
+int
qwx_qmi_fw_init_done(struct qwx_softc *sc)
{
int ret = 0;
@@ -20984,10 +20985,15 @@ qwx_qmi_fw_init_done(struct qwx_softc *sc)
clear_bit(ATH11K_FLAG_RECOVERY, sc->sc_flags);
ret = qwx_core_qmi_firmware_ready(sc);
if (ret) {
+ /*
+ * This flags tells qwx_stop() that core is stopped
+ * and ATH11K_FIRMWARE_MODE_OFF was already sent.
+ */
set_bit(ATH11K_FLAG_QMI_FAIL, sc->sc_flags);
- return;
}
}
+
+ return ret;
}
int
@@ -21047,8 +21053,7 @@ qwx_qmi_event_server_arrive(struct qwx_softc *sc)
}
}
- qwx_qmi_fw_init_done(sc);
- return 0;
+ return qwx_qmi_fw_init_done(sc);
}
int
@@ -24323,8 +24328,17 @@ qwx_init_task(void *arg)
struct qwx_softc *sc = arg;
struct ifnet *ifp = &sc->sc_ic.ic_if;
int s = splnet();
- rw_enter_write(&sc->ioctl_rwl);
+ /*
+ * Do not sleep for this lock. The init task is a one-shot
+ * recovery mechanism. If the ioctl handler is busy then
+ * we are being reconfigured or reset already.
+ */
+ if (rw_enter(&sc->ioctl_rwl, RW_WRITE | RW_NOSLEEP) != 0) {
+ splx(s);
+ return;
+ }
+
if (ifp->if_flags & IFF_RUNNING)
qwx_stop(ifp);
@@ -27444,11 +27458,11 @@ qwx_activate(struct device *self, int act)
switch (act) {
case DVACT_QUIESCE:
+ rw_enter_write(&sc->ioctl_rwl);
if (ifp->if_flags & IFF_RUNNING) {
- rw_enter_write(&sc->ioctl_rwl);
qwx_stop(ifp);
- rw_exit(&sc->ioctl_rwl);
}
+ rw_exit(&sc->ioctl_rwl);
break;
case DVACT_RESUME:
err = qwx_hal_srng_init(sc);
@@ -27457,12 +27471,14 @@ qwx_activate(struct device *self, int act)
sc->sc_dev.dv_xname);
break;
case DVACT_WAKEUP:
+ rw_enter_write(&sc->ioctl_rwl);
if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) == IFF_UP) {
err = qwx_init(ifp);
if (err)
printf("%s: could not initialize hardware\n",
sc->sc_dev.dv_xname);
}
+ rw_exit(&sc->ioctl_rwl);
break;
}
blob - a0e416a3cafea6c49646852c11c9fa9045b71ce2
blob + 00da7a38928ecf5dee26f3b73a8eeb10ca0fd04c
--- sys/dev/pci/if_qwx_pci.c
+++ sys/dev/pci/if_qwx_pci.c
@@ -1113,7 +1113,8 @@ unsupported_wcn6855_soc:
memcpy(ifp->if_xname, sc->sc_dev.dv_xname, IFNAMSIZ);
if_attach(ifp);
ieee80211_ifattach(ifp);
- ieee80211_media_init(ifp, qwx_media_change, ieee80211_media_status);
+ ieee80211_media_init(ifp, ieee80211_media_change,
+ ieee80211_media_status);
ic->ic_node_alloc = qwx_node_alloc;
small set of qwx bug fixes