On 6/25/2025 2:45 PM, Johannes Berg wrote:
On Wed, 2025-06-25 at 11:58 +0800, Zhongqiu Han wrote:
>>>
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.
Sure, but now you're talking about something that's not supposed to
happen. The WARN there means "someone made a mistake in this code". I'm
not even super concerned about using uninitialised memory in that case
although I agree we should avoid it, so the trivial fix for the data
leak, without making the logic more complex, would be to just initialise
the data to zero. Not to fix the syzbot issue (which btw is already no
longer happening according to the dashboard), but to fix the potential
data leak.
Hi johannes~
Thanks for your discussion~
I really appreciate your review and don’t want to take up too much of
your time.
yes, potential data leak issue should be fixed.
Since ieee80211_set_disassoc() returns early only with a WARN, with
syzbot you won't ever get to the uninitialized memory use though, which
is what I was asking.
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.
No ... that can't be right, we _know_ that syzbot enables panic_on_warn.
So this particular bug report from syzbot is definitely _not_ triggered
this way.
Let's revisit the initial stack trace where the problem originated.
the issue occurred on commit:
HEAD commit: ff202c5028a1 Merge tag 'soc-fixes-6.14' of git://git.kerne..
=====================================================
BUG: KMSAN: uninit-value in cfg80211_tx_mlme_mgmt+0x155/0x300
net/wireless/mlme.c:226
cfg80211_tx_mlme_mgmt+0x155/0x300 net/wireless/mlme.c:226
ieee80211_report_disconnect net/mac80211/mlme.c:4238 [inline]
ieee80211_sta_connection_lost+0xfa/0x150 net/mac80211/mlme.c:7811
ieee80211_sta_work+0x1dea/0x4ef0
ieee80211_iface_work+0x1900/0x1970 net/mac80211/iface.c:1684
cfg80211_wiphy_work+0x396/0x860 net/wireless/core.c:435
process_one_work kernel/workqueue.c:3236 [inline]
process_scheduled_works+0xc1a/0x1e80 kernel/workqueue.c:3317
worker_thread+0xea7/0x14f0 kernel/workqueue.c:3398
kthread+0x6b9/0xef0 kernel/kthread.c:464
ret_from_fork+0x6d/0x90 arch/x86/kernel/process.c:148
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
Local variable frame_buf created at:
ieee80211_sta_connection_lost+0x43/0x150 net/mac80211/mlme.c:7806
ieee80211_sta_work+0x1dea/0x4ef0
=====================================================
frame_buf is created at ieee80211_sta_connection_lost()
and the func is short:
void ieee80211_sta_connection_lost(struct ieee80211_sub_if_data *sdata,
u8 reason, bool tx)
{
u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DEAUTH, reason,
tx, frame_buf);
ieee80211_report_disconnect(sdata, frame_buf, sizeof(frame_buf), true,
reason, false);
}
In func ieee80211_set_disassoc(), frame_buf will never be NULL, so
ieee80211_send_deauth_disassoc must be called if
ieee80211_set_disassoc() not return early.
/* deauthenticate/disassociate now */
if (tx || frame_buf) {
drv_mgd_prepare_tx(sdata->local, sdata, &info);
ieee80211_send_deauth_disassoc(sdata, sdata->vif.cfg.ap_addr,
sdata->vif.cfg.ap_addr, stype,
reason, tx, frame_buf);
}
I also checked the cmdline from kconfig, there is no panic_on_warn on
kernel config: https://syzkaller.appspot.com/x/.config
x=aca5947365998f67
However there is possible it is partial or not used cmdline as
CONFIG_CMDLINE_FORCE is not set.
CONFIG_CMDLINE="earlyprintk=serial net.ifnames=0
sysctl.kernel.hung_task_all_cpu_backtrace=1 ima_policy=tcb nf-conntrack
ftp.ports=20000 nf-conntrack-tftp.ports=20000 nf-conntrack
sip.ports=20000 nf-conntrack-irc.ports=20000 nf-conntrack
sane.ports=20000 binder.debug_mask=0 rcupdate.rcu_expedited=1
rcupdate.rcu_cpu_stall_cputime=1 no_hash_pointers page_owner=on
sysctl.vm.nr_hugepages=4 sysctl.vm.nr_overcommit_hugepages=4
secretmem.enable=1 sysctl.max_rcu_stall_to_panic=1 msr.allow_writes=off
coredump_filter=0xffff root=/dev/sda console=ttyS0 vsyscall=native
numa=fake=2 kvm-intel.nested=1 spec_store_bypass_disable=prctl nopcid
vivid.n_devs=64
vivid.multiplanar=1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2,1,2
netrom.nr_ndevs=32 rose.rose_ndevs=32 smp.csd_lock_timeout=100000
watchdog_thresh=55 workqueue.watchdog_thresh=140
sysctl.net.core.netdev_unregister_timeout_secs=140 dummy_hcd.num=32
max_loop=32 nbds_max=32 kmsan.panic=1"
Although I suspect the if the syzbot has indeed enabled panic_on_warn or
not, the most important thing is to avoid data leak and also what I’m
concerned about might also trigger a WARN_ON on ieee80211_set_disassoc.
Later, when ieee80211_report_disconnect is called, it attempts to use
the uninitialized frame_buf, resulting in a bug.
I agree this is a bug but it's not one that requires major surgery to
fix - the only bug here is that if we already have another bug that
leads to the WARN, we can leak stack data to userspace. We can simply
initialise the data to avoid that. I'm not worried about reporting a bad
thing to userspace if we already had a WARN indicating some _other_ bug
that we should fix.
Sure, I got you point~ maybe the only thing we need to do is avoiding
data leak to userspace side.
I'll happily take a patch that says "initialise the frame buffer so that
on the off chance of other bugs, we don't leak stack data to userspace",
but I do think that's about all we need at this point. The syzbot report
is already stale and no longer happening anyway. If there is another
report about the WARN_ON after commit ccbaf782390d ("wifi: mac80211:
rework the Tx of the deauth in ieee80211_set_disassoc()"), we need to
see how it's possible that we get into the WARN case, but I haven't seen
such a report.
Sure, I plan to arise one such patch to initialize all the frame_buf on
net/mac80211/mlme.c file.
Thanks johannes for the nice discussion~
johannes
--
Thx and BRs,
Zhongqiu Han
