Re: [PATCH] mac80211_hwsim: fix divide error in mac80211_hwsim_link_info_changed

[Deepak Karn]

On Mon, Dec 8, 2025 at 2:00 PM Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
>
> On Sat, 2025-12-06 at 23:03 +0530, Deepakkumar Karn wrote:
> >
> >
> > Thank you for your response Johannes. cfg80211 already have validation
> > in cfg80211_validate_beacon_int().
>
> Oh OK.
>
> > The problem seems to occur in
> > case of interface shutdown which calls ieee80211_do_stop() that makes
> > beacon_int = 0 or set_tsf which causes divides by zero.
>
> Why would that call set_tsf()?
>
> And yeah that should make beacon_int=0, but not have beaconing enabled.
>
> > What if we:
> > 1. Handle off-channel operation:
>
> >                 if (test_and_clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED,
> >                                        &sdata->state)) {
> > -                       sdata->vif.bss_conf.enable_beacon = true;
> > -                       ieee80211_link_info_change_notify(
> > -                               sdata, &sdata->deflink,
> > -                               BSS_CHANGED_BEACON_ENABLED);
> > +                       if (sdata->vif.bss_conf.beacon_int) {
> > +                               sdata->vif.bss_conf.enable_beacon = true;
> > +                               ieee80211_link_info_change_notify(
> > +                                       sdata, &sdata->deflink,
> > +                                       BSS_CHANGED_BEACON_ENABLED);
> > +                       }
>
> don't follow, that only does it when beacon was actually stopped ...
> maybe do_stop() needs to clear that bit if there's a possibility of this
> happening in the wrong order, but I don't see how there could be a race
> since off-channel must also be stopped at that point anyway, unless that
> is in the wrong order.
>
> > 2. Handle case where debugfs is written after shutdown or any race condition during disable beaconing:
> >
> > diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c
> > index 551f5eb4e747..8363cdd17a97 100644
> > --- a/drivers/net/wireless/virtual/mac80211_hwsim.c
>
> first of all, I really do think you should *not* (need to) modify hwsim
> at all
>
> >         conf = link_conf_dereference_protected(vif, data->link_data[0].link_id);
> > -       if (conf && !conf->enable_beacon)
> > +       if ((conf && !conf->enable_beacon) || !bcn_int)
> >                 return;
>
> But this also makes no sense, it shouldn't be possible to have beacon
> enabled and beacon interval == 0.
>

Thank you Johannes. As you highlighted I searched for a code path if
there is any place that has the
invariant condition of beacon being enabled and beacon int is 0 but
none seems to be having that.

Regards,
Deepakkumar Karn