linux-kernel - Re: [PATCH wireless-next v3] wifi: mac80211: consider links for validating SCAN_FLAG

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] [day] [month] [year] [list]

Message-ID: <114409da-5d1e-40ed-aa8e-3bcb769ac0a1@oss.qualcomm.com>
Date: Wed, 9 Jul 2025 08:17:20 +0530
From: Aditya Kumar Singh <aditya.kumar.singh@....qualcomm.com>
To: Johannes Berg <johannes@...solutions.net>
Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH wireless-next v3] wifi: mac80211: consider links for
 validating SCAN_FLAG_AP in scan request during MLO

On 7/8/2025 4:16 PM, Johannes Berg wrote:
> On Wed, 2025-06-25 at 18:31 +0530, Aditya Kumar Singh wrote:
>>
>> -		if (ieee80211_num_beaconing_links(sdata) &&
>> -		    (!(wiphy->features & NL80211_FEATURE_AP_SCAN) ||
>> -		     !(req->flags & NL80211_SCAN_FLAG_AP)))
>> -			return -EOPNOTSUPP;
>> +		for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS;
>> +		     link_id++) {
>> +			link = sdata_dereference(sdata->link[link_id], sdata);
>> +			if (!link)
>> +				continue;
> 
> for_each_link_data()

Sure. When I was writing this, this macro was not present hence did not 
use it. Now that it is there, I will use it. Thanks for pointing it out.

> 
>> +
>> +			/* if the link is not beaconing, ignore it */
>> +			if (!sdata_dereference(link->u.ap.beacon, sdata))
>> +				continue;
>> +
>> +			/* If we are here then at least one of the link is
>> +			 * beaconing and since radio level information is
>> +			 * not present or single underlying radio is present,
>> +			 * no point in checking further and hence return if
>> +			 * flag requirements are not met.
>> +			 */
>> +			if (wiphy->n_radio < 2) {
>> +				if (!(wiphy->features & NL80211_FEATURE_AP_SCAN) ||
>> +				    !(req->flags & NL80211_SCAN_FLAG_AP))
>> +					return -EOPNOTSUPP;
>> +
>> +				continue;
>> +			}
> 
> Is that _really_ worth special-casing in the scan control path? It's not
> like this is a performance question here.
> 
> Maybe ieee80211_is_radio_idx_in_scan_req() shouldn't WARN_ON() then or
> something, so we can reuse it. Or maybe (better?) just reorder the
> checks there, if the chan_radio_idx==-1 and radio_idx==-1 would return
> first, and WARN only if we found a scan channel that isn't covered by a
> radio?

sure reordering seems much better. Thanks for the suggestion, will change.

> 
> And <2 seems really strange anyway, ==1 should basically never happen,
> it's equivalent to ==0, as in no list of radios?

That is correct. If underlying wiphy does not advertise, it would be 0. 
And if it advertises, ideally it should be 2 or more (otherwise why 
advertising?) But, there is no check for 1. So a radio can choose to 
advertise but keep n_radios as 1. And having n_radio 1 is also same as 
not advertising it. Hence to ensure these kicks in only when we know 
n_radios is greater than 1, < 2 is checked. Same way other two places 
also it is used.


-- 
Aditya