diff mbox series

[4/5] misc: fix compiler warning in ifstat and nstat

Message ID 20201130002135.6537-5-stephen@networkplumber.org
State New
Headers show
Series Fix compiler warnings from GCC-10 | expand

Commit Message

Stephen Hemminger Nov. 30, 2020, 12:21 a.m. UTC
The code here was doing strncpy() in a way that causes gcc 10
warning about possible string overflow. Just use strlcpy() which
will null terminate and bound the string as expected.

This has existed since start of git era so no Fixes tag.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 misc/ifstat.c | 2 +-
 misc/nstat.c  | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

Comments

David Laight Nov. 30, 2020, 9:18 a.m. UTC | #1
From: Stephen Hemminger
> Sent: 30 November 2020 00:22
> 
> The code here was doing strncpy() in a way that causes gcc 10
> warning about possible string overflow. Just use strlcpy() which
> will null terminate and bound the string as expected.
> 
> This has existed since start of git era so no Fixes tag.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  misc/ifstat.c | 2 +-
>  misc/nstat.c  | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/misc/ifstat.c b/misc/ifstat.c
> index c05183d79a13..d4a33429dc50 100644
> --- a/misc/ifstat.c
> +++ b/misc/ifstat.c
> @@ -251,7 +251,7 @@ static void load_raw_table(FILE *fp)
>  			buf[strlen(buf)-1] = 0;
>  			if (info_source[0] && strcmp(info_source, buf+1))
>  				source_mismatch = 1;
> -			strncpy(info_source, buf+1, sizeof(info_source)-1);
> +			strlcpy(info_source, buf+1, sizeof(info_source));
>  			continue;

ISTM that once it has done a strlen() it ought to use the length
for the later copy.

I don't seem to have the source file (I'm guessing it isn't in the
normal repo), but is that initial strlen() guaranteed not to return
zero?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Stephen Hemminger Nov. 30, 2020, 5:31 p.m. UTC | #2
On Mon, 30 Nov 2020 09:18:59 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> From: Stephen Hemminger
> > Sent: 30 November 2020 00:22
> > 
> > The code here was doing strncpy() in a way that causes gcc 10
> > warning about possible string overflow. Just use strlcpy() which
> > will null terminate and bound the string as expected.
> > 
> > This has existed since start of git era so no Fixes tag.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  misc/ifstat.c | 2 +-
> >  misc/nstat.c  | 3 +--
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/misc/ifstat.c b/misc/ifstat.c
> > index c05183d79a13..d4a33429dc50 100644
> > --- a/misc/ifstat.c
> > +++ b/misc/ifstat.c
> > @@ -251,7 +251,7 @@ static void load_raw_table(FILE *fp)
> >  			buf[strlen(buf)-1] = 0;
> >  			if (info_source[0] && strcmp(info_source, buf+1))
> >  				source_mismatch = 1;
> > -			strncpy(info_source, buf+1, sizeof(info_source)-1);
> > +			strlcpy(info_source, buf+1, sizeof(info_source));
> >  			continue;  
> 
> ISTM that once it has done a strlen() it ought to use the length
> for the later copy.
> 
> I don't seem to have the source file (I'm guessing it isn't in the
> normal repo), but is that initial strlen() guaranteed not to return
> zero?
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 

All this is in the regular iproute2 repo
diff mbox series

Patch

diff --git a/misc/ifstat.c b/misc/ifstat.c
index c05183d79a13..d4a33429dc50 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -251,7 +251,7 @@  static void load_raw_table(FILE *fp)
 			buf[strlen(buf)-1] = 0;
 			if (info_source[0] && strcmp(info_source, buf+1))
 				source_mismatch = 1;
-			strncpy(info_source, buf+1, sizeof(info_source)-1);
+			strlcpy(info_source, buf+1, sizeof(info_source));
 			continue;
 		}
 		if ((n = malloc(sizeof(*n))) == NULL)
diff --git a/misc/nstat.c b/misc/nstat.c
index 6fdd316cce84..ecdd4ce8266d 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -136,8 +136,7 @@  static void load_good_table(FILE *fp)
 			buf[strlen(buf)-1] = 0;
 			if (info_source[0] && strcmp(info_source, buf+1))
 				source_mismatch = 1;
-			info_source[0] = 0;
-			strncat(info_source, buf+1, sizeof(info_source)-1);
+			strlcpy(info_source, buf + 1, sizeof(info_source));
 			continue;
 		}
 		/* idbuf is as big as buf, so this is safe */