Re: [syzbot] divide error in mac80211_hwsim_bss_info_changed

On Mon, 2021-05-17 at 03:45 -0700, syzbot wrote:

Hello,

syzbot found the following issue on:

HEAD commit:    eebe426d Merge tag 'fixes-for-5.12-rc7' of git://git.kerne..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15e73ceed00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=9c3d8981d2bdb103
dashboard link: https://syzkaller.appspot.com/bug?extid=26727b5e00947e02242c
compiler:       Debian clang version 11.0.1-2

Unfortunately, I don't have any reproducer for this issue yet.

That's not very surprising ...

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+26727b5e00947e02242c@syzkaller.appspotmail.com

divide error: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 13049 Comm: kworker/u4:16 Not tainted 5.12.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: phy10 ieee80211_roc_work
RIP: 0010:mac80211_hwsim_bss_info_changed+0x514/0xf90 drivers/net/wireless/mac80211_hwsim.c:2024

The issue here is that we have the following code in offchannel.c:

        mutex_lock(&local->iflist_mtx);
        list_for_each_entry(sdata, &local->interfaces, list) {
...
                if (test_and_clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED,
                                       &sdata->state)) {
                        sdata->vif.bss_conf.enable_beacon = true;
                        ieee80211_bss_info_change_notify(
                                sdata, BSS_CHANGED_BEACON_ENABLED);
                }


However, we have the following code in ibss.c
ieee80211_ibss_disconnect():

        sdata->vif.bss_conf.ibss_joined = false;
        sdata->vif.bss_conf.ibss_creator = false;
        sdata->vif.bss_conf.enable_beacon = false;
        sdata->vif.bss_conf.ssid_len = 0;

        /* remove beacon */
        presp = rcu_dereference_protected(ifibss->presp,
                                          lockdep_is_held(&sdata->wdev.mtx));
        RCU_INIT_POINTER(sdata->u.ibss.presp, NULL);
        if (presp)
                kfree_rcu(presp, rcu_head);

        clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
        ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED |
                                                BSS_CHANGED_IBSS);


There's no common locking here, so it's simply racy.

Normally, all of the ieee80211_ibss_disconnect() happens together, but
if ieee80211_offchannel_return() happens to hit here before
SDATA_STATE_OFFCHANNEL_BEACON_STOPPED is cleared, we might inadvertently
enable beaconing while the interface is actually stopping.

I'm not really sure what happens next, but perhaps the interface is
going down and the beacon_int is reset to 0, or such, leading to the
problem at hand.


Off the top of my head, I don't really have a good idea about how to fix
this - we'd want to add some more consistent locking everywhere. I
assume that with the rtnl-weaning having happened, that might simply
consist in aligning mac80211 to the cfg80211 wiphy mutex everywhere.

johannes