Re: [PATCH] wifi: mac80211: fix monitor mode frame capture for real chanctx drivers

[Johannes Berg]

On Mon, 2026-03-09 at 10:45 +0000, 傅继晗 wrote:
> 
> I see the key difference between our approaches: your v2 iterates
> the chanctx_list and only proceeds when there is exactly one entry
> (going to fail_rcu if more than one exists), while mine blindly takes
> the first entry via list_first_entry_or_null(). Your approach is
> clearly safer -- in a multi-chanctx scenario, there is no way to know
> which channel the user intends to inject on, so refusing is the
> correct behaviour.

Oh, right, I hadn't even realised that at first.

> I have tested my patch on an MT7921AU (mt76, USB) adapter across
> v6.13, v6.19, and v7.0-rc2 with managed + monitor coexistence, and
> have not observed any crashes. However, my testing was limited to a
> single-chanctx scenario (one managed interface + one monitor
> interface), so it does not rule out crashes in multi-chanctx
> configurations.

Maybe Óscar can comment on which device/version he tested and got the
crash with? I just would like to avoid having crashes because of this,
but generally think that - perhaps optionally - we could have code like
this, since people _do_ want injection to work.

> Could you share some details about the crashes that were reported
> with your v2? For example, which devices/drivers were affected and
> what the crash signature looked like? That would help me understand
> whether the issue was specific to multi-chanctx usage or something
> more fundamental with accessing the chanctx_list in this code path.

No, it was specific to some driver implementation, but I don't have any
more information now.

> If you agree, I would like to send a v2 that combines both approaches:
> use list_first_entry_or_null() for simplicity, but add a
> list_is_singular() guard so we only proceed when there is exactly one
> chanctx -- matching the safety constraint from your v2:
> 
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -2399,10 +2399,24 @@
> -	if (chanctx_conf)
> +	if (chanctx_conf) {
>  		chandef = &chanctx_conf->def;
> -	else if (local->emulate_chanctx)
> +	} else if (local->emulate_chanctx) {
>  		chandef = &local->hw.conf.chandef;
> -	else
> -		goto fail_rcu;
> +	} else {
> +		struct ieee80211_chanctx *ctx;
> +
> +		ctx = list_first_entry_or_null(&local->chanctx_list,
> +					       struct ieee80211_chanctx,
> +					       list);
> +		if (ctx && list_is_singular(&local->chanctx_list))
> +			chandef = &ctx->conf.def;
> +		else
> +			goto fail_rcu;
> +	}
> 
> This avoids the ambiguity of picking an arbitrary chanctx in
> multi-chanctx scenarios while still fixing the common single-chanctx
> case (e.g. one managed + one monitor interface).

Seems reasonable, I think we could even drop the "if (emulate) part
(since in that case the list should always be singular). Just like I
said above - would like to understand the issue that had appeared with
it.

johannes