* [PATCH 1/5] devlink: fix uninitialized warning
2020-11-30 0:21 [PATCH 0/5] Fix compiler warnings from GCC-10 Stephen Hemminger
@ 2020-11-30 0:21 ` Stephen Hemminger
2020-11-30 0:21 ` [PATCH 2/5] bridge: fix string length warning Stephen Hemminger
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2020-11-30 0:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, shalomt
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/5] bridge: fix string length warning
2020-11-30 0:21 [PATCH 0/5] Fix compiler warnings from GCC-10 Stephen Hemminger
2020-11-30 0:21 ` [PATCH 1/5] devlink: fix uninitialized warning Stephen Hemminger
@ 2020-11-30 0:21 ` Stephen Hemminger
2020-11-30 0:21 ` [PATCH 3/5] tc: fix compiler warnings in ip6 pedit Stephen Hemminger
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2020-11-30 0:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, nikolay
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/5] tc: fix compiler warnings in ip6 pedit
2020-11-30 0:21 [PATCH 0/5] Fix compiler warnings from GCC-10 Stephen Hemminger
2020-11-30 0:21 ` [PATCH 1/5] devlink: fix uninitialized warning Stephen Hemminger
2020-11-30 0:21 ` [PATCH 2/5] bridge: fix string length warning Stephen Hemminger
@ 2020-11-30 0:21 ` Stephen Hemminger
2020-11-30 23:10 ` Petr Machata
2020-11-30 0:21 ` [PATCH 4/5] misc: fix compiler warning in ifstat and nstat Stephen Hemminger
` (3 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2020-11-30 0:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, petrm
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/5] tc: fix compiler warnings in ip6 pedit
2020-11-30 0:21 ` [PATCH 3/5] tc: fix compiler warnings in ip6 pedit Stephen Hemminger
@ 2020-11-30 23:10 ` Petr Machata
0 siblings, 0 replies; 11+ messages in thread
From: Petr Machata @ 2020-11-30 23:10 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, petrm
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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] misc: fix compiler warning in ifstat and nstat
2020-11-30 0:21 [PATCH 0/5] Fix compiler warnings from GCC-10 Stephen Hemminger
` (2 preceding siblings ...)
2020-11-30 0:21 ` [PATCH 3/5] tc: fix compiler warnings in ip6 pedit Stephen Hemminger
@ 2020-11-30 0:21 ` Stephen Hemminger
2020-11-30 9:18 ` David Laight
2020-11-30 0:21 ` [PATCH 5/5] f_u32: fix compiler gcc-10 compiler warning Stephen Hemminger
` (2 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2020-11-30 0:21 UTC (permalink / raw)
To: netdev; +Cc: 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCH 4/5] misc: fix compiler warning in ifstat and nstat
2020-11-30 0:21 ` [PATCH 4/5] misc: fix compiler warning in ifstat and nstat Stephen Hemminger
@ 2020-11-30 9:18 ` David Laight
2020-11-30 17:31 ` Stephen Hemminger
0 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2020-11-30 9:18 UTC (permalink / raw)
To: 'Stephen Hemminger', netdev@vger.kernel.org
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)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] misc: fix compiler warning in ifstat and nstat
2020-11-30 9:18 ` David Laight
@ 2020-11-30 17:31 ` Stephen Hemminger
0 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2020-11-30 17:31 UTC (permalink / raw)
To: David Laight; +Cc: netdev@vger.kernel.org
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] f_u32: fix compiler gcc-10 compiler warning
2020-11-30 0:21 [PATCH 0/5] Fix compiler warnings from GCC-10 Stephen Hemminger
` (3 preceding siblings ...)
2020-11-30 0:21 ` [PATCH 4/5] misc: fix compiler warning in ifstat and nstat Stephen Hemminger
@ 2020-11-30 0:21 ` Stephen Hemminger
2020-11-30 19:21 ` [PATCH 0/5] Fix compiler warnings from GCC-10 Jacob Keller
2020-12-03 16:40 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2020-11-30 0:21 UTC (permalink / raw)
To: netdev; +Cc: 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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 0/5] Fix compiler warnings from GCC-10
2020-11-30 0:21 [PATCH 0/5] Fix compiler warnings from GCC-10 Stephen Hemminger
` (4 preceding siblings ...)
2020-11-30 0:21 ` [PATCH 5/5] f_u32: fix compiler gcc-10 compiler warning Stephen Hemminger
@ 2020-11-30 19:21 ` Jacob Keller
2020-12-03 16:40 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2020-11-30 19:21 UTC (permalink / raw)
To: Stephen Hemminger, netdev
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/5] Fix compiler warnings from GCC-10
2020-11-30 0:21 [PATCH 0/5] Fix compiler warnings from GCC-10 Stephen Hemminger
` (5 preceding siblings ...)
2020-11-30 19:21 ` [PATCH 0/5] Fix compiler warnings from GCC-10 Jacob Keller
@ 2020-12-03 16:40 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2020-12-03 16:40 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
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
^ permalink raw reply [flat|nested] 11+ messages in thread