Re: [syzbot] divide error in mac80211_hwsim_bss_info_changed

[Johannes Berg]

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@xxxxxxxxxxxxxxxxxxxxxxxxx
> 
> 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