| Message ID | 202212231052044562664@zte.com.cn (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [net-next] wifi: airo: use strscpy() to instead of strncpy() | expand |
On Fri, 2022-12-23 at 10:52 +0800, yang.yang29@zte.com.cn wrote: > From: Xu Panda <xu.panda@zte.com.cn> > > The implementation of strscpy() is more robust and safer. > That's now the recommended way to copy NUL-terminated strings. > > Signed-off-by: Xu Panda <xu.panda@zte.com.cn> > Signed-off-by: Yang Yang <yang.yang29@zte.com> > --- > drivers/net/wireless/cisco/airo.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c > index 7c4cc5f5e1eb..600a64f671ce 100644 > --- a/drivers/net/wireless/cisco/airo.c > +++ b/drivers/net/wireless/cisco/airo.c > @@ -6067,8 +6067,7 @@ static int airo_get_nick(struct net_device *dev, > struct airo_info *local = dev->ml_priv; > > readConfigRid(local, 1); > - strncpy(extra, local->config.nodeName, 16); > - extra[16] = '\0'; > + strscpy(extra, local->config.nodeName, 17); > dwrq->length = strlen(extra); > Again, why bother. But is this even correct/identical behaviour? Wouldn't it potentially read 17 input bytes before forcing NUL- termination? johannes
> Again, why bother. But is this even correct/identical behaviour?>> > Wouldn't it potentially read 17 input bytes before forcing NUL- > termination? Thank you for your reminder. This is a terrible error. Strscpy() may cause the array to be out of bounds. I should be more cautious next time. Xu and Yang
diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c index 7c4cc5f5e1eb..600a64f671ce 100644 --- a/drivers/net/wireless/cisco/airo.c +++ b/drivers/net/wireless/cisco/airo.c @@ -6067,8 +6067,7 @@ static int airo_get_nick(struct net_device *dev, struct airo_info *local = dev->ml_priv; readConfigRid(local, 1); - strncpy(extra, local->config.nodeName, 16); - extra[16] = '\0'; + strscpy(extra, local->config.nodeName, 17); dwrq->length = strlen(extra); return 0;