A non-MLD sta could want to send offchannel management frame (e.g. to
do a offchannel scan). Because ieee80211_rx_for_interface() fills the
link_id information with the link the sta is currently using; hostapd
would send back management frame responses through wrong link causing
the sta to miss them.

To fix that, use link_id that matches the received frame frequency if
any or do not fill link_id indication for management frames, relying
on hostapd instead to infer the proper link from the received frame
frequency.

Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
 net/mac80211/rx.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index caa3e6b3f46e..26be0f378b3f 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -5114,6 +5114,37 @@ static void __ieee80211_rx_handle_8023(struct ieee80211_hw *hw,
 	dev_kfree_skb(skb);
 }
 
+static int ieee80211_rx_get_link_from_freq(struct ieee80211_rx_data *rx,
+					   struct sk_buff *skb,
+					   struct link_sta_info *link_sta)
+{
+	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
+	struct ieee80211_sta *sta = &link_sta->sta->sta;
+	struct ieee80211_link_data *link;
+	struct ieee80211_bss_conf *bss_conf;
+	struct ieee80211_chanctx_conf *conf;
+
+	if (!status->freq)
+		return link_sta->link_id;
+
+	for_each_link_data(rx->sdata, link) {
+		bss_conf = link->conf;
+		if (!bss_conf)
+			continue;
+		conf = rcu_dereference(bss_conf->chanctx_conf);
+		if (!conf || !conf->def.chan)
+			continue;
+
+		if (conf->def.chan->center_freq != status->freq)
+			continue;
+
+		if (ieee80211_rx_is_valid_sta_link_id(sta, link->link_id))
+			return link->link_id;
+	}
+
+	return -1;
+}
+
 static bool ieee80211_rx_for_interface(struct ieee80211_rx_data *rx,
 				       struct sk_buff *skb, bool consume)
 {
@@ -5131,7 +5162,15 @@ static bool ieee80211_rx_for_interface(struct ieee80211_rx_data *rx,
 	link_sta = link_sta_info_get_bss(rx->sdata, hdr->addr2);
 	if (link_sta) {
 		sta = link_sta->sta;
-		link_id = link_sta->link_id;
+
+		/* Use freq to get link id information on management frames to
+		 * allow for offchannel scan, roaming, etc.
+		 */
+		if (ieee80211_is_mgmt(hdr->frame_control))
+			link_id = ieee80211_rx_get_link_from_freq(rx, skb,
+								  link_sta);
+		else
+			link_id = link_sta->link_id;
 	} else {
 		struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 
-- 
2.40.0

On Fri, 2025-07-11 at 12:03 +0200, Remi Pommarel wrote:
> 
> +static int ieee80211_rx_get_link_from_freq(struct ieee80211_rx_data *rx,
> +					   struct sk_buff *skb,
> +					   struct link_sta_info *link_sta)
> +{
> +	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
> +	struct ieee80211_sta *sta = &link_sta->sta->sta;
> +	struct ieee80211_link_data *link;
> +	struct ieee80211_bss_conf *bss_conf;
> +	struct ieee80211_chanctx_conf *conf;
> +
> +	if (!status->freq)
> +		return link_sta->link_id;
> +
> +	for_each_link_data(rx->sdata, link) {


..._rcu()

> +		bss_conf = link->conf;
> +		if (!bss_conf)
> +			continue;
> +		conf = rcu_dereference(bss_conf->chanctx_conf);
> +		if (!conf || !conf->def.chan)
> +			continue;
> +
> +		if (conf->def.chan->center_freq != status->freq)
> +			continue;
> +
> +		if (ieee80211_rx_is_valid_sta_link_id(sta, link->link_id))
> +			return link->link_id;
> +	}

But this is now almost the same as the code added via
https://patch.msgid.link/20250721062929.1662700-1-michael-cy.lee@mediatek.com
so I think it should be refactored/combined.

> @@ -5131,7 +5162,15 @@ static bool ieee80211_rx_for_interface(struct ieee80211_rx_data *rx,
>  	link_sta = link_sta_info_get_bss(rx->sdata, hdr->addr2);
>  	if (link_sta) {
>  		sta = link_sta->sta;
> -		link_id = link_sta->link_id;
> +
> +		/* Use freq to get link id information on management frames to
> +		 * allow for offchannel scan, roaming, etc.
> +		 */
> +		if (ieee80211_is_mgmt(hdr->frame_control))
> +			link_id = ieee80211_rx_get_link_from_freq(rx, skb,
> +								  link_sta);

It seems to me taht _iff_ the link ID ends up not being link_sta-
>link_id here, we should set link_sta=NULL. Otherwise we think we're
actually receiving from that STA but we aren't really, and if we then
use sta->link[link_id] we'd crash, and I'd be very surprised if this
were impossible.

I'm not sure what consequences that has, but OTOH it really should not
be sending anything other than probe requests, authentication frames and
some few public action frames on another link. We need not handle other
frames as if we didn't realise the link was different, it's fine to even
just drop them because we don't recognise the STA there.

So I think that's what we should do. As written this seems really
problematic to have a link_sta != NULL and thus sta != NULL but then
sta->link[link_id] == NULL and not == link_sta.


Arguably, given the code we already have, we should perhaps implement
this in another way and after doing the link_sta lookup via
link_sta_info_get_bss() simply reject the link_sta (set it to NULL) if
the RX link doesn't match the link it's expected to be on. Then we'd
fall back to the existing code I linked to above anyway, which seems
almost better.

johannes