[PATCH wireless] wifi: mac80211: refuse to make a monitor active when it has no queue

[Devin Wittmayer]

commit 8105f9b8a887 ("mac80211: allocate TXQs for active monitor
interfaces") reserved a TXQ for active monitors, because drivers using TXQ
expect every interface handed to them to have one.  It covers only
interfaces created active: vif.txq is assigned in ieee80211_txq_init()
from ieee80211_if_add(), and cannot be added later.

MONITOR_FLAG_ACTIVE can still be set afterwards.
ieee80211_set_mon_options() refuses that while the interface is up, but
while it is down it just stores the flag.  Such an interface then reaches
the driver with vif->txq NULL, and ath9k resolves its multicast node's
tids through that pointer unchecked:

  BUG: kernel NULL pointer dereference, address: 0000000000000066
  RIP: 0010:ath_tx_node_init+0x49/0x170 [ath9k]
   ath9k_add_interface+0x10c/0x140 [ath9k]
   drv_add_interface+0x54/0x250 [mac80211]
   ieee80211_do_open+0x32f/0x800 [mac80211]

Two commands from a user with CAP_NET_ADMIN reach it:

  iw dev <iface> set monitor active
  ip link set <iface> up

The fault happens with RTNL held, so it is never released and all later
netlink operations block.

Refuse the promotion when there is no queue to give.

Fixes: 79af1f866193 ("mac80211: avoid allocating TXQs that won't be used")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Devin Wittmayer <lucid_duck@xxxxxxxxxxxxx>
---
Reproduces on mac80211_hwsim, no hardware needed: create a monitor, take it
down, make it active and bring it up, and the driver gets a NULL vif->txq.
One created with flags active gets a real one.  With the patch the first is
refused, the second still works, and an interface created active, set to
none and back to active, is still accepted.  Same on an MT7922.

Only drivers advertising NL80211_FEATURE_ACTIVE_MONITOR reach this, since
cfg80211 refuses the flag otherwise.  ath9k, mt7603 and mt76x02 deref
vif->txq unchecked.  mt7615, mt7915, mt7921, mt7925 and mt7996 test it
first.

Where it faults today, refusing costs nothing.  On the drivers that check,
I could not test whether a promoted interface actually works, so that is
where a regression would show.

Reserving a queue for every monitor instead would make the promotion work
rather than refuse it, and would also cover a passive monitor reaching the
driver under NO_VIRTUAL_MONITOR.  That undoes 79af1f866193 deliberately, so
I did not assume it.  Happy to write it if you prefer.

 net/mac80211/cfg.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b6163dcc7e92..523cef0a5526 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -115,6 +115,15 @@ static int ieee80211_set_mon_options(struct ieee80211_sub_if_data *sdata,
 			return -EBUSY;
 	}
 
+	/*
+	 * An active monitor is passed to the driver and needs a TXQ, which is
+	 * reserved with the netdev in ieee80211_if_add() and cannot be added
+	 * later.  An interface created without the flag has none, so refuse to
+	 * set it rather than hand the driver a NULL vif->txq.
+	 */
+	if ((params->flags & MONITOR_FLAG_ACTIVE) && !sdata->vif.txq)
+		return -EOPNOTSUPP;
+
 	/* validate whether MU-MIMO can be configured */
 	if (!ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF) &&
 	    !ieee80211_hw_check(&local->hw, NO_VIRTUAL_MONITOR) &&
-- 
2.55.0