lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d59c69a46c36db6fb6616b4c2ba9847cccd29e5d.camel@sipsolutions.net>
Date: Tue, 22 Jul 2025 14:22:54 +0200
From: Johannes Berg <johannes@...solutions.net>
To: Remi Pommarel <repk@...plefau.lt>, linux-wireless@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PACTH v2 wireless-next 1/3] wifi: mac80211: Get link_id
 from freq for received management frame

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ