[PATCH 0/5] Fix compiler warnings from GCC-10

[PATCH 0/5] Fix compiler warnings from GCC-10

[Stephen Hemminger]

Update to GCC-10 and it starts warning about some new things.

Stephen Hemminger (5):
  devlink: fix uninitialized warning
  bridge: fix string length warning
  tc: fix compiler warnings in ip6 pedit
  misc: fix compiler warning in ifstat and nstat
  f_u32: fix compiler gcc-10 compiler warning

 devlink/devlink.c  | 2 +-
 ip/iplink_bridge.c | 2 +-
 misc/ifstat.c      | 2 +-
 misc/nstat.c       | 3 +--
 tc/f_u32.c         | 2 +-
 tc/p_ip6.c         | 2 +-
 6 files changed, 6 insertions(+), 7 deletions(-)

-- 
2.29.2

[PATCH 1/5] devlink: fix uninitialized warning

[Stephen Hemminger]

GCC-10 complains about uninitialized variable.

devlink.c: In function ‘cmd_dev’:
devlink.c:2803:12: warning: ‘val_u32’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 2803 |    val_u16 = val_u32;
      |    ~~~~~~~~^~~~~~~~~
devlink.c:2747:11: note: ‘val_u32’ was declared here
 2747 |  uint32_t val_u32;
      |           ^~~~~~~

This is a false positive because it can't figure out the control flow
when the parse returns error.

Fixes: 2557dca2b028 ("devlink: Add string to uint{8,16,32} conversion for generic parameters")
Cc: shalomt@mellanox.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 devlink/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 1ff865bc5c22..ca99732efd00 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2744,7 +2744,7 @@ static int cmd_dev_param_set(struct dl *dl)
 	struct param_ctx ctx = {};
 	struct nlmsghdr *nlh;
 	bool conv_exists;
-	uint32_t val_u32;
+	uint32_t val_u32 = 0;
 	uint16_t val_u16;
 	uint8_t val_u8;
 	bool val_bool;
-- 
2.29.2

[PATCH 2/5] bridge: fix string length warning

[Stephen Hemminger]

Gcc-10 complains about possible string length overflow.
This can't happen Ethernet address format is always limited to
18 characters or less. Just resize the temp buffer.

Fixes: 70dfb0b8836d ("iplink: bridge: export bridge_id and designated_root")
Cc: nikolay@cumulusnetworks.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/iplink_bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index 3e81aa059cb3..d12fd0558f7d 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -74,7 +74,7 @@ static void explain(void)
 
 void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len)
 {
-	char eaddr[32];
+	char eaddr[18];
 
 	ether_ntoa_r((const struct ether_addr *)id->addr, eaddr);
 	snprintf(buf, len, "%.2x%.2x.%s", id->prio[0], id->prio[1], eaddr);
-- 
2.29.2

[PATCH 3/5] tc: fix compiler warnings in ip6 pedit

[Stephen Hemminger]

Gcc-10 complains about referencing a zero size array.
This occurs because the array of keys is actually in the following
structure which is part of the overall selector.

The original code was safe, but better to just use the key
array directly.

Fixes: 2d9a8dc439ee ("tc: p_ip6: Support pedit of IPv6 dsfield")
Cc: petrm@mellanox.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/p_ip6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/p_ip6.c b/tc/p_ip6.c
index 71660c610c82..83a6ae8183a7 100644
--- a/tc/p_ip6.c
+++ b/tc/p_ip6.c
@@ -82,7 +82,7 @@ parse_ip6(int *argc_p, char ***argv_p,
 		/* Shift the field by 4 bits on success. */
 		if (!res) {
 			int nkeys = sel->sel.nkeys;
-			struct tc_pedit_key *key = &sel->sel.keys[nkeys - 1];
+			struct tc_pedit_key *key = &sel->keys[nkeys - 1];
 
 			key->mask = htonl(ntohl(key->mask) << 4 | 0xf);
 			key->val = htonl(ntohl(key->val) << 4);
-- 
2.29.2

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

[Stephen Hemminger]

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;
 		}
 		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 */
-- 
2.29.2

[PATCH 5/5] f_u32: fix compiler gcc-10 compiler warning

[Stephen Hemminger]

With gcc-10 it complains about array subscript error.

f_u32.c: In function ‘u32_parse_opt’:
f_u32.c:1113:24: warning: array subscript 0 is outside the bounds of an interior zero-length array ‘struct tc_u32_key[0]’ [-Wzero-length-bounds]
 1113 |    hash = sel2.sel.keys[0].val & sel2.sel.keys[0].mask;
      |           ~~~~~~~~~~~~~^~~
In file included from tc_util.h:11,
                 from f_u32.c:26:
../include/uapi/linux/pkt_cls.h:253:20: note: while referencing ‘keys’
  253 |  struct tc_u32_key keys[0];
      |

This is because the keys are actually allocated in the second element
of the parent structure.

Simplest way to address the warning is to assign directly to the keys
in the containing structure.

This has always been in iproute2 (pre-git) so no Fixes.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/f_u32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/f_u32.c b/tc/f_u32.c
index e0a322d5a11c..2ed5254a40d5 100644
--- a/tc/f_u32.c
+++ b/tc/f_u32.c
@@ -1110,7 +1110,7 @@ static int u32_parse_opt(struct filter_util *qu, char *handle,
 				}
 				NEXT_ARG();
 			}
-			hash = sel2.sel.keys[0].val & sel2.sel.keys[0].mask;
+			hash = sel2.keys[0].val & sel2.keys[0].mask;
 			hash ^= hash >> 16;
 			hash ^= hash >> 8;
 			htid = ((hash % divisor) << 12) | (htid & 0xFFF00000);
-- 
2.29.2

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

[David Laight]

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)

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

[Stephen Hemminger]

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

Re: [PATCH 0/5] Fix compiler warnings from GCC-10

[Jacob Keller]

On 11/29/2020 4:21 PM, Stephen Hemminger wrote:
> Update to GCC-10 and it starts warning about some new things.
> 
> Stephen Hemminger (5):
>   devlink: fix uninitialized warning
>   bridge: fix string length warning
>   tc: fix compiler warnings in ip6 pedit
>   misc: fix compiler warning in ifstat and nstat
>   f_u32: fix compiler gcc-10 compiler warning
> 
>  devlink/devlink.c  | 2 +-
>  ip/iplink_bridge.c | 2 +-
>  misc/ifstat.c      | 2 +-
>  misc/nstat.c       | 3 +--
>  tc/f_u32.c         | 2 +-
>  tc/p_ip6.c         | 2 +-
>  6 files changed, 6 insertions(+), 7 deletions(-)
> 

Nice to see these cleanups. I noticed a few of these recently while
working on devlink.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH 3/5] tc: fix compiler warnings in ip6 pedit

[Petr Machata]

Stephen Hemminger <stephen@networkplumber.org> writes:

> Gcc-10 complains about referencing a zero size array.
> This occurs because the array of keys is actually in the following
> structure which is part of the overall selector.
>
> The original code was safe, but better to just use the key
> array directly.
>
> Fixes: 2d9a8dc439ee ("tc: p_ip6: Support pedit of IPv6 dsfield")
> Cc: petrm@mellanox.com
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Reviewed-by: Petr Machata <petrm@nvidia.com>

Re: [PATCH 0/5] Fix compiler warnings from GCC-10

[patchwork-bot+netdevbpf]

Hello:

This series was applied to iproute2/iproute2.git (refs/heads/main):

On Sun, 29 Nov 2020 16:21:30 -0800 you wrote:
> Update to GCC-10 and it starts warning about some new things.
> 
> Stephen Hemminger (5):
>   devlink: fix uninitialized warning
>   bridge: fix string length warning
>   tc: fix compiler warnings in ip6 pedit
>   misc: fix compiler warning in ifstat and nstat
>   f_u32: fix compiler gcc-10 compiler warning
> 
> [...]

Here is the summary with links:
  - [1/5] devlink: fix uninitialized warning
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=f8176999390f
  - [2/5] bridge: fix string length warning
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=5bdc4e9151a1
  - [3/5] tc: fix compiler warnings in ip6 pedit
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=2319db905295
  - [4/5] misc: fix compiler warning in ifstat and nstat
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=c01498392138
  - [5/5] f_u32: fix compiler gcc-10 compiler warning
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=cae2e9291adf

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html