linux-kernel - Re: [PATCH wireless-next 2/3] wifi: mac80211: Allow scan on a radio while operating on DFS on another radio

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: <f694233ea7378257fbc992bd3d6f10ac7f6d3695.camel@sipsolutions.net>
Date: Thu, 22 May 2025 13:23:18 +0200
From: Johannes Berg <johannes@...solutions.net>
To: Raj Kumar Bhagat <quic_rajkbhag@...cinc.com>
Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH wireless-next 2/3] wifi: mac80211: Allow scan on a radio
 while operating on DFS on another radio

On Thu, 2025-05-22 at 16:10 +0530, Raj Kumar Bhagat wrote:
> 
> I'm not sure If I fully understood the this comment.
> 
> This patch assumes that multiple radios are grouped under a single wiphy.
> Each radio has its own list of frequencies it can scan, and there is no overlap
> in frequencies between any two radios within the same wiphy.

Yeah I guess I'm just overly paranoid due to lack of familiarity with
all the multi-radio things.

> If this assumption holds, then if one radio is operating on a DFS channel and a
> new scan request does not include any frequencies from that radio's list, the
> scan should be allowed—since the DFS radio wouldn't be involved in handling that
> scan request.

Agree.

> > > +	for (i = 0; i < scan_req->n_channels; i++) {
> > > +		chan = scan_req->channels[i];
> > > +		chan_radio_idx = cfg80211_get_radio_idx_by_chan(wiphy, chan);
> > > +		/*
> > > +		 * Skip channels with an invalid radio index and continue
> > > +		 * checking. If any channel in the scan request matches the
> > > +		 * given radio index, return true.
> > > +		 */
> > > +		if (chan_radio_idx < 0)
> > > +			continue;
> > 
> > This seems ... wrong? If there's a channel in the scan request that
> > didn't map to _any_ radio then how are we even scanning there? And the
> > comment seems even stranger, why would we _want_ to ignore it (which it
> > conveniently doesn't answer)?
> > 
> 
> It seems, (chan_radio_idx < 0) should never be true because the chan is
> taken from the valid scan request. I should remove this check in next version?

I'm not sure, why did you add it? Maybe it should be a WARN_ON and abort
the whole function? It just doesn't seem right to _ignore_.

johannes