Re: [PATCH wireless-next v2 0/8] Adding NO_STA flag and reworking RX link resolution

On 8/3/2026 5:30 AM, Benjamin Berg wrote:
> From: Benjamin Berg <[email protected]>
> 
> Hi,
> 
> This patchset refactors the RX link resolution a bit to fix some issues
> where mac80211 might accept frames on the wrong link and incorrectly
> translate the address. It also adds a new NL80211_ATTR_FRAME_CMD_NO_STA
> flag so that userspace can know whether address translation was done by
> the kernel on RX and can also prevent address translation for management
> frames during TX.
> 
> This together should be enough to fix the existing issues in hostapd
> where stations that are still associated try to authenticate again but
> hostapd for example ends up sending the frame to an old link address.
> 
> Benjamin
> 
> v2:
>  * Remove unused variable in rtw89
> 
> Changes in v1:
>  * Add NAN_DATA interface handling
>  * Handle NO_STA flag correctly for NAN
>  * Fixed forwarding of station parameter
>  * Rebase and other small fixes
> 
> Changes in RFCv2:
>  * Port other drivers to new API (untested)
>  * Fix a checkpatch warning
> 
> Benjamin Berg (8):
>   wifi: iwlwifi: use link_sta internally to the driver
>   wifi: mac80211: change public RX API to use link stations
>   wifi: mac80211: refactor RX link_id and station handling
>   wifi: mac80211: rework RX packet handling
>   wifi: cfg80211: add attribute for TX/RX denoting there is no station
>   wifi: mac80211: report to cfg80211 when no STA is known for a frame
>   wifi: mac80211: pass station to ieee80211_tx_skb_tid
>   wifi: mac80211: pass error station if non-STA transmit was requested
> 
>  drivers/net/wireless/ath/ath11k/dp_rx.c       |   2 +-
>  drivers/net/wireless/ath/ath12k/dp_mon.c      |   2 -
>  drivers/net/wireless/ath/ath12k/dp_rx.c       |  20 +-
>  drivers/net/wireless/ath/ath12k/hw.h          |  20 +-
>  drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c |  20 +-
>  drivers/net/wireless/ath/ath12k/wifi7/dp_rx.h |  12 +-
>  drivers/net/wireless/ath/ath12k/wifi7/hw.c    |   6 +-
>  drivers/net/wireless/ath/ath12k/wmi.c         |   6 -
>  drivers/net/wireless/intel/iwlwifi/mld/agg.c  |  17 +-
>  drivers/net/wireless/intel/iwlwifi/mld/agg.h  |   4 +-
>  drivers/net/wireless/intel/iwlwifi/mld/rx.c   |  50 +-
>  drivers/net/wireless/intel/iwlwifi/mld/rx.h   |   2 +-
>  .../wireless/intel/iwlwifi/mld/tests/agg.c    |   7 +-
>  drivers/net/wireless/intel/iwlwifi/mvm/rx.c   |   2 +-
>  drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c |   6 +-
>  drivers/net/wireless/mediatek/mt76/mac80211.c |  22 +-
>  drivers/net/wireless/realtek/rtw89/core.c     |   7 -
>  .../wireless/virtual/mac80211_hwsim_main.c    |   3 -
>  include/net/cfg80211.h                        |   4 +
>  include/net/mac80211.h                        |  25 +-
>  include/uapi/linux/nl80211.h                  |   7 +
>  net/mac80211/agg-tx.c                         |   4 +-
>  net/mac80211/ap.c                             |   3 -
>  net/mac80211/ht.c                             |   2 +-
>  net/mac80211/ieee80211_i.h                    |  11 +-
>  net/mac80211/iface.c                          |   7 +-
>  net/mac80211/mlme.c                           |  10 +-
>  net/mac80211/offchannel.c                     |  13 +-
>  net/mac80211/rx.c                             | 447 ++++++++++--------
>  net/mac80211/scan.c                           |  10 +-
>  net/mac80211/tdls.c                           |   2 +-
>  net/mac80211/tx.c                             |  10 +-
>  net/wireless/nl80211.c                        |   8 +-
>  33 files changed, 413 insertions(+), 358 deletions(-)
> 

My review agent had some comments. Since it is late in my day I'm just going
to throw them over the fence so you can look at them.

In my workflow I have one agent perform a code review and then have another
agent try to refute the findings. I'm passing on the ones that were confirmed
(not refuted).


  Bug 1 — Patch 2/8 — drivers/net/wireless/ath/ath12k/dp_rx.c (OOB + skipped
  link 0)

  Confirmed in git show c95532877bc5. The call site does:
  u8 link_id = ath12k_hw_get_rx_link_id(dp->hw_params, peer, rxcb, status);
  if (link_id > 0)
      link_pubsta = rcu_dereference(pubsta->link[link_id]);

  ath12k_hw_get_rx_link_id() is declared int in hw.h and returns -1 on no-ops.
  Storing -1 into u8 gives 255. The guard 255 > 0 is true, causing
  pubsta->link[255] — an out-of-bounds access (ieee80211_sta.link[] is
  IEEE80211_MLD_MAX_NUM_LINKS wide). Additionally > 0 drops the valid link-0
  case.

  Fix:
  int link_id = ath12k_hw_get_rx_link_id(...);
  if (link_id >= 0)
      link_pubsta = rcu_dereference(pubsta->link[link_id]);

  ---
  Bug 2 — Patch 3/8 — net/mac80211/rx.c, ieee80211_rx_data_set_sta() (negative
  array index)

  Confirmed in git show 004192addab3. Inside the link_id < 0 branch:
  if (ieee80211_vif_is_mld(&rx->sdata->vif) && sta && !sta->sta.valid_links) {
      rx->link = rcu_dereference(rx->sdata->link[sta->deflink.link_id]);
      rx->link_sta = rcu_dereference(rx->sta->link[link_id]);  /* BUG: link_id
  is negative */
  }

  link_id < 0 (typically -1) used to index a fixed-size array is undefined
  behaviour / memory corruption. The non-MLO station intent is clearly to use
  the deflink.

  Fix:
  rx->link_sta = &sta->deflink;

  ---
  Bug 3 — Patch 8/8 — net/mac80211/offchannel.c, ieee80211_mgmt_tx() (inverted
  ternary)

  Confirmed in git show 022fe418cbd0. sta is initialised to NULL at function
  entry. When params->no_sta=true, the if (!params->no_sta) block is skipped, so
  sta is still NULL at the ieee80211_tx_skb_tid call:

  ieee80211_tx_skb_tid(sdata, skb,
                       params->no_sta ? sta : ERR_PTR(-ENOENT),
                       7, link_id);

  - no_sta=true → passes NULL (which means "do station lookup") — wrong
  - no_sta=false → passes ERR_PTR(-ENOENT) (which means "suppress lookup") —
  wrong

  Fix:
  ieee80211_tx_skb_tid(sdata, skb,
                       params->no_sta ? ERR_PTR(-ENOENT) : NULL,
                       7, link_id);


The full set of comments from the first agent follows:

## Per-Patch Analysis

### Patch 1/8 — wifi: iwlwifi: use link_sta internally
**Verdict: Reviewed-by, no issues**

Clean mechanical refactor converting `ieee80211_sta *sta` →
`ieee80211_link_sta *link_sta` inside the iwlwifi/mld driver. The
`link_sta->link_id` replacement for `rx_status->link_id` in
`iwl_mld_rx_mgmt_prot` is strictly better — it removes the fallback-to-zero
path when `link_valid` was false. KUnit test update (`&sta->deflink`) is correct.

---

### Patch 2/8 — wifi: mac80211: change public RX API to use link stations
**Verdict: NEEDS FIX — two bugs in ath12k**

The API change itself is well-designed. The FIXME emulation block is
acceptable as a transitional measure. However, the ath12k `dp_rx.c` adaptation
has a pair of related bugs:

**Bug 1: Type mismatch causes out-of-bounds array access**

```c
/* ath12k/dp_rx.c, ath12k_dp_rx_deliver_msdu() */
u8 link_id =
    ath12k_hw_get_rx_link_id(dp->hw_params, peer, rxcb, status);

if (link_id > 0)
    link_pubsta = rcu_dereference(pubsta->link[link_id]);
```

`ath12k_hw_get_rx_link_id()` returns `int` with `-1` as the failure sentinel.
Storing that into `u8` gives `255` (wrap). The check `255 > 0` is true, so the
code then dereferences `pubsta->link[255]` — a massive out-of-bounds access.
`ieee80211_sta.link[]` is only `IEEE80211_MLD_MAX_NUM_LINKS` entries wide.

**Bug 2: Valid link ID 0 is silently dropped**

MLO link IDs start at 0. The check `> 0` skips link 0, leaving `link_pubsta`
as NULL for a valid link-0 frame. It should be `>= 0`.

**Fix:**
```c
int link_id =
    ath12k_hw_get_rx_link_id(dp->hw_params, peer, rxcb, status);

if (link_id >= 0)
    link_pubsta = rcu_dereference(pubsta->link[link_id]);
```

The `wifi7/dp_rx.c` `get_link_id_*` implementations returning `int` are
correct. The bug is purely at the call site.

---

### Patch 3/8 — wifi: mac80211: refactor RX link_id and station handling
**Verdict: NEEDS FIX — negative array index**

The removal of `link_valid` and making `link_id` mac80211-internal is the
right direction. Most of the changes are clean. However, there is a negative
array index bug introduced in `ieee80211_rx_data_set_sta()`:

```c
if (link_id < 0) {
    if (ieee80211_vif_is_mld(&rx->sdata->vif) &&
        sta && !sta->sta.valid_links) {
        rx->link =
            rcu_dereference(rx->sdata->link[sta->deflink.link_id]);
        rx->link_sta = rcu_dereference(rx->sta->link[link_id]);  /* BUG */
    } else {
        rx->link = &rx->sdata->deflink;
    }
}
```

`link_id < 0` (e.g., `-1`) and `rx->sta->link` is a fixed-size array. Indexing
with a negative value is undefined behaviour and will corrupt memory or crash.
The intent here is clearly to set `link_sta` for a non-MLO station
(`!sta->sta.valid_links`), so the fix is:

```c
rx->link_sta = &sta->deflink;
```

or equivalently `rcu_dereference(rx->sta->link[sta->deflink.link_id])`.

Other changes in this patch worth noting:

- `scan.c`: Removing the `link_valid` guard and using `rx_status->link_id`
unconditionally is valid because by the time scan processing is invoked,
mac80211's internal rx path has already written `link_id` into the status.
- `iface.c` / `mlme.c`: Same reasoning applies — safe after patch 4 guarantees
`link_id` is always set.
- `ieee80211_rx_is_valid_sta_link_id()` fix for non-MLO stations is correct.

---

### Patch 4/8 — wifi: mac80211: rework RX packet handling
**Verdict: Reviewed-by, minor observation**

This is the most complex patch. The rewrite of
`__ieee80211_rx_handle_packet()` is well-structured. The new `rx_data_pending`
pattern correctly replaces the `prev` pointer approach with clearer semantics.
The separation of data frames (addressed to known stations) from management
frames (interface iteration) is cleaner.

The new `ieee80211_rx_data_set_link_sta()` helper is a clean abstraction; the
WARN_ON_ONCE guard and early return on missing `link` are appropriate.

One observation on the MLD "no station" fallback:

```c
if (!found)
    link = &sdata->deflink;  /* fallback if no freq match */
```

When no link matches the received frequency on an MLD VIF, the frame is still
delivered on deflink. For an AP use case where we're receiving a probe request
on a frequency not matching any active link, this silently misattributes the
frame. A FIXME comment here would be appropriate, or dropping the frame may be
more correct. The existing FIXME comment in `ieee80211_rx_for_interface` from
patch 3 suggests the author is aware of incomplete corner cases.

The `WARN_ON_ONCE(!local->emulate_chanctx && !status->freq)` strictness is
intentional and necessary — drivers that don't populate `status->freq` for
management frames will need fixing.

---

### Patch 5/8 — wifi: cfg80211: add attribute for TX/RX denoting there is no
station
**Verdict: Reviewed-by, no issues**

Minimal and clean. `NL80211_ATTR_FRAME_CMD_NO_STA` is correctly added as
`NLA_FLAG`, added to the policy table, extracted in `nl80211_tx_mgmt`, and
included in `nl80211_send_mgmt`. The dual-use (RX: "no STA found"
notification; TX: "don't use STA for address translation") is well-explained
in the kdoc.

---

### Patch 6/8 — wifi: mac80211: report to cfg80211 when no STA is known for a
frame
**Verdict: Reviewed-by, no issues**

One-liner, correct. `.no_sta = !rx->sta` accurately conveys whether mac80211
resolved a station for the frame.

---

### Patch 7/8 — wifi: mac80211: pass station to ieee80211_tx_skb_tid
**Verdict: Reviewed-by, no issues**

The three-value encoding (`NULL` = old lookup, `ERR_PTR(-ENOENT)` = suppress
lookup, valid pointer = use directly) is a clear and established kernel
pattern. Using `ERR_PTR(-ENOENT)` in `ieee80211_tx_skb_tid_band` for
off-channel TX is correct — no station resolution is needed there. The TDLS
change passing the actual `sta` is the right specific behavior.

---

### Patch 8/8 — wifi: mac80211: pass error station if non-STA transmit was
requested
**Verdict: NEEDS FIX — condition is inverted**

The intent per the commit message is: "when cfg80211 requested transmit
without a station, pass an **error station** to ieee80211_tx_skb_tid."
`ERR_PTR(-ENOENT)` = error station = suppress station lookup.

However, the actual code is:

```c
/* net/mac80211/offchannel.c */
ieee80211_tx_skb_tid(sdata, skb,
                     params->no_sta ? sta : ERR_PTR(-ENOENT),
                     7, link_id);
```

At this point, when `no_sta=true`, the earlier guard:
```c
if (!params->no_sta) {
    sta = sta_info_get_bss(sdata, mgmt->da);
    mlo_sta = sta && sta->sta.mlo;
}
```
means `sta` is `NULL`. So:
- `no_sta=true` → passes `NULL` → triggers **station lookup** inside
`ieee80211_xmit` — the opposite of the intent
- `no_sta=false` → passes `ERR_PTR(-ENOENT)` → **suppresses station lookup**
for all normal management TX — also wrong

The ternary is inverted. The fix:
```c
ieee80211_tx_skb_tid(sdata, skb,
                     params->no_sta ? ERR_PTR(-ENOENT) : NULL,
                     7, link_id);
```