[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d63e5f8a-8e0a-4619-98a8-e73c80e307cd@quicinc.com>
Date: Wed, 25 Jun 2025 11:58:32 +0800
From: Zhongqiu Han <quic_zhonhan@...cinc.com>
To: Johannes Berg <johannes@...solutions.net>,
<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 6/24/2025 9:35 PM, Johannes Berg wrote:
>>>
>>>> - 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:
>>
>> yes, syzbot couldn't have run into an uninitialized buffer after the
>> WARN_ON on **patch v2** such as:
>>
>> --- a/net/mac80211/mlme.c
>> +++ b/net/mac80211/mlme.c
>> @@ -4433,6 +4433,10 @@ static void ieee80211_report_disconnect(struct
>> ieee80211_sub_if_data *sdata,
>> .u.mlme.data = tx ? DEAUTH_TX_EVENT : DEAUTH_RX_EVENT,
>> .u.mlme.reason = reason,
>> };
>> + struct sta_info *ap_sta = sta_info_get(sdata, sdata->vif.cfg.ap_addr);
>> +
>> + if (WARN_ON(!ap_sta))
>> + return;
>
> I think you misunderstood ... We have this WARN_ON since 687a7c8a7227
> ("wifi: mac80211: change disassoc sequence a bit"). Therefore, !ap_sta
> cannot be the cause of syzbot complaints, since WARN_ON would panic it
In my experience, WARN_ON is rarely configured to trigger a panic.
> before it ever gets to the uninitialized memory use.
>
Hi johannes
Thanks a lot for your discussion and review~
>>>
We have this WARN_ON since 687a7c8a7227
("wifi: mac80211: change disassoc sequence a bit")
>>>
this WARN_ON was added in func ieee80211_set_disassoc() but not
ieee80211_report_disconnect()
I add WARN_ON on ieee80211_report_disconnect() on my patch v2,
It was precisely because of the WARN_ON at 687a7c8a7227 that caused
ieee80211_set_disassoc to return early(when panic_on_warn is not
enabled). As a result, ieee80211_set_disassoc failed to properly
initialize frame_buf. Then, when ieee80211_report_disconnect was called,
it ended up using an uninitialized value.
>
>> "You're adding a WARN_ON() that's now guaranteed to trigger, no
>
> so now it's no longer your WARN_ON, I guess, but how did it trigger? I
> really think we need to figure out how it triggered and fix _that_.
>
The bug was triggered as follow:
Commit 687a7c8a7227 ("wifi: mac80211: change disassoc sequence a bit")
introduced a code path where ieee80211_set_disassoc may return early if
WARN_ON(!ap_sta) is triggered(panic_on_warn is not enabled). In this
case, frame_buf is not initialized.
Later, when ieee80211_report_disconnect is called, it attempts to use
the uninitialized frame_buf, resulting in a bug.
This is the reason I tagged:
Fixes: 687a7c8a7227 ("wifi: mac80211: change disassoc sequence a bit")
In my patch v2, I want to fix the bug by adding "WARN_ON(!ap_sta) and
return" on ieee80211_report_disconnect() to avoid continue use frame_buf.
In my patch v3, I plan to fix the bug by avoid calling
ieee80211_report_disconnect() when frame_buf is not initialized by
ieee80211_set_disassoc()
> johannes
--
Thx and BRs,
Zhongqiu Han
Powered by blists - more mailing lists