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

[Johannes Berg <johannes@sipsolutions.net>]

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.

johannes