RE: [PATCH next] wifi: rtw89: mcc: prevent shift wrapping in rtw89_core_mlsr_switch()

[Zong-Zhe Yang <kevin_yang@realtek.com>]

Dan Carpenter <dan.carpenter@linaro.org> wrote:
> 
> On Tue, May 27, 2025 at 07:38:17AM +0000, Zong-Zhe Yang wrote:
> > Dan Carpenter <dan.carpenter@linaro.org> wrote:
> > >
> > > The "link_id" value comes from the user via debugfs.  If it's larger
> > > than BITS_PER_LONG then that would result in shift wrapping and
> > > potentially an out of bounds access later.  Fortunately, only root can write to debugfs files
> so the security impact is minimal.
> > >
> >
> > Thank you for catching this problem.
> >
> > >
> > > [...]
> > >
> > > @@ -5239,6 +5239,9 @@ int rtw89_core_mlsr_switch(struct rtw89_dev
> > > *rtwdev, struct rtw89_vif *rtwvif,
> > >         if (unlikely(!ieee80211_vif_is_mld(vif)))
> > >                 return -EOPNOTSUPP;
> > >
> > > +       if (unlikely(link_id >= BITS_PER_LONG))
> > > +               return -EINVAL;
> > > +
> >
> > Since I think this problem only comes from dbgfs path, would you like to just add a check in
> debug.c ?
> >
> > For example,
> > (based on 0 <= valid link id < IEEE80211_MLD_MAX_NUM_LINKS <
> > BITS_PER_LONG)
> >
> > rtw89_debug_priv_mlo_mode_set(...)
> > {
> >         ...
> >         switch (mlo_mode) {
> >         case RTW89_MLO_MODE_MLSR:
> >                if (argv >= IEEE80211_MLD_MAX_NUM_LINKS)
> >                        return -EINVAL;
> >                 ...
> >
> 
> I'd prefer to add the check in one place instead of all the callers.

Understandable.

> We could check IEEE80211_MLD_MAX_NUM_LINKS instead of bits per long if that's more
> readable?

Sound good to me.