linux-kernel - Re: [PATCH v3] wifi: mac80211: Prevent disconnect reports when no AP is associated

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a5078d3c7f3d1c2281a3f5a50386fdb7072935bb.camel@sipsolutions.net>
Date: Fri, 20 Jun 2025 13:13:09 +0200
From: Johannes Berg <johannes@...solutions.net>
To: Zhongqiu Han <quic_zhonhan@...cinc.com>, 
	miriam.rachel.korenblit@...el.com
Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org, 
	syzbot+5a7b40bcb34dea5ca959@...kaller.appspotmail.com
Subject: Re: [PATCH v3] wifi: mac80211: Prevent disconnect reports when no
 AP is associated

On Fri, 2025-06-20 at 11:20 +0800, Zhongqiu Han wrote:
> 
> - Rebased on top of current next.
> - Update the v2 code implementation:
>   - Remove zero-initialization of frame_buf

You could keep that I guess, but we shouldn't claim it's any relation
with fixing the bug. Just as a cleanliness thing maybe?

>   - Remove WARN_ON and early return in ieee80211_report_disconnect()
>   - Change the return type of ieee80211_set_disassoc(). If
>     ieee80211_report_disconnect() uses the frame_buf initialized by
>     ieee80211_set_disassoc(), its invocation is now conditional based
>     on the return value of ieee80211_set_disassoc(). 

I don't understand this change ... surely syzbot couldn't have run into
an uninitialized buffer after the WARN_ON since it has panic_on_warn. So
why all these changes:

> -static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
> +/*
> + * Note that if ieee80211_report_disconnect() relies on the *frame_buf
> + * initialized by this function, then it must only be called if this function
> + * returns true; otherwise, it may use an uninitialized buffer.
> + */
> +static bool ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>  				   u16 stype, u16 reason, bool tx,
>  				   u8 *frame_buf)
>  {
> @@ -3935,13 +3940,13 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>  	lockdep_assert_wiphy(local->hw.wiphy);
>  
>  	if (WARN_ON(!ap_sta))
> -		return;
> +		return false;
>  
>  	if (WARN_ON_ONCE(tx && !frame_buf))
> -		return;
> +		return false;
>  
>  	if (WARN_ON(!ifmgd->associated))
> -		return;
> +		return false;
>  
>  	ieee80211_stop_poll(sdata);
>  
> @@ -4168,6 +4173,8 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>  
>  	memset(ifmgd->userspace_selectors, 0,
>  	       sizeof(ifmgd->userspace_selectors));
> +
> +	return true;
>  }

here to have a return value? It's only false when you had a WARN_ON()
which means there's a bug elsewhere?

>  static void ieee80211_reset_ap_probe(struct ieee80211_sub_if_data *sdata)
> @@ -4448,6 +4455,7 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata)
>  	struct ieee80211_local *local = sdata->local;
>  	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
>  	u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
> +	bool report_disconnect;
>  
>  	lockdep_assert_wiphy(local->hw.wiphy);
>  
> @@ -4477,20 +4485,22 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata)
>  		}
>  	}
>  
> -	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DEAUTH,
> -			       ifmgd->driver_disconnect ?
> -					WLAN_REASON_DEAUTH_LEAVING :
> -					WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY,
> -			       true, frame_buf);
> +	report_disconnect = ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DEAUTH,
> +						   ifmgd->driver_disconnect ?
> +						   WLAN_REASON_DEAUTH_LEAVING :
> +						   WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY,
> +						   true, frame_buf);
>  	/* the other links will be destroyed */
>  	sdata->vif.bss_conf.csa_active = false;
>  	sdata->deflink.u.mgd.csa.waiting_bcn = false;
>  	sdata->deflink.u.mgd.csa.blocked_tx = false;
>  	ieee80211_vif_unblock_queues_csa(sdata);
>  
> -	ieee80211_report_disconnect(sdata, frame_buf, sizeof(frame_buf), true,
> -				    WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY,
> -				    ifmgd->reconnect);
> +	if (report_disconnect)
> +		ieee80211_report_disconnect(sdata, frame_buf, sizeof(frame_buf),
> +					    true, WLAN_REASON_DISASSOC_DUE_TO_INACTIVITY,
> +					    ifmgd->reconnect);
> +
>  	ifmgd->reconnect = false;

So all of that also doesn't really do anything.

But then the rest of the patch also doesn't seem to do anything, so what
am I missing?

Does the bug even still exist? Looking at the code now, I feel like
ccbaf782390d ("wifi: mac80211: rework the Tx of the deauth in
ieee80211_set_disassoc()") probably fixed this issue?

johannes