Re: [PATCH v5 2/3] wifi: rtw88: skip already reset station in rtw_sta_remove()

[Luka Gejak <luka.gejak@linux.dev>]

Hi Kalpesh,

Thanks for the new version. Two comments below.

On November 12, 2026 2:14:00 PM GMT+01:00, Kalpesh Solanki
<skalpesh942@gmail.com> wrote:

> diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
> index f7708ad17b6f..a74f199a93ab 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.c
> +++ b/drivers/net/wireless/realtek/rtw88/main.c
> @@ -384,6 +384,9 @@ void rtw_sta_remove(struct rtw_dev *rtwdev, struct ieee80211_sta *sta,
>       struct ieee80211_vif *vif = si->vif;
>       int i;
>
> +     if (!vif)
> +             return;
> +
>       cancel_work_sync(&si->rc_work);

I think my comment on v4 about this got missed:

https://lore.kernel.org/all/83E3139E-BE19-4EAD-9BCB-6DABBF8D1942@linux.dev/

rtw_ops_sta_rc_update() can still queue rc_work after the recovery has
reset the station. If the station is then removed before the restart
adds it back, this return skips the only cancel_work_sync() and
rtw_sta_rc_work() could run on a freed station. Moving the check below
cancel_work_sync() should be enough.

[ ... ]

>       kfree(si->mask);
> +     si->mask = NULL;

As far as I can see si->use_cfg_mask stays set here. mac80211 keeps the
same station across the restart, and when it adds it back, rtw_sta_add()
calls rtw_update_sta_info(), which gets to this:

        rtw_rate_mask_cfg()
                if (!si->use_cfg_mask)
                        return ra_mask;
                ...
                cfg_mask = mask->control[band].legacy;

So if I read it right, for a station that got a bitrate mask, a recovery
now ends in a NULL pointer dereference, where before it read the freed
mask. Maybe clear si->use_cfg_mask together with si->mask? The mask is
then dropped after a recovery, since mac80211 doesn't pass it down again
on restart, but I think that is fine.

Best regards,
Luka Gejak