Re: [PATCH v3] staging: rtl8723bs: stop overreading monitor interface name

[Dan Carpenter]

On Thu, Oct 01, 2026 at 12:59:59PM +0300, Dan Carpenter wrote:
> > > diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> > > index 4416d0ec1..8f5032844 100644
> > > --- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> > > +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
> > > @@ -2158,7 +2158,7 @@ static int rtw_cfg80211_add_monitor_if(struct adapter *padapter, char *name, str
> > >  		goto out;
> > >  
> > >  	*ndev = pwdev_priv->pmon_ndev = mon_ndev;
> > > -	memcpy(pwdev_priv->ifname_mon, name, IFNAMSIZ + 1);
> > > +	strscpy(pwdev_priv->ifname_mon, name);
> > 
> > But we are replacing str*() calls in the kernel with memcpy() calls
> > where it can happen, so why go backwards here?
> > 
> > What caused you to notice this change is needed?  Have you measured a
> > speed up in throughput with this change applied?  How was it tested?
> 
> This seems like a legit patch to me, although it's probably AI generated.
> 
> net/wireless/nl80211.c
>   5199          wdev = rdev_add_virtual_intf(rdev,
>   5200                                  nla_data(info->attrs[NL80211_ATTR_IFNAME]),
>                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This means that it is a string up to IFNAMSIZ (16) bytes long counting
> the NUL terminator, but it could be less.  So doing a memcpy() is a
> read overflow.
> 
>   5201                                  NET_NAME_USER, type, &params);
> 

I tasked ChatGPT with writing a Smatch check for this and it noticed
that the "+ 1" in "IFNAMSIZ + 1" is wrong as well.  "name" can only
be 16 characters long (counting the NUL terminator).

drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2164 rtw_cfg80211_add_monitor_if()
warn: buffer 'name' too small user_len=1-16 for 17 byte copy

regards,
dan carpenter