* [PATCH iproute2 00/11] fix analyzer warnings
@ 2023-05-09 21:21 Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 01/11] lib/fs: fix file leak in task_get_name Stephen Hemminger
` (11 more replies)
0 siblings, 12 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Address some (but not all) of the issues reported by gcc 13
new analyzer. These are mostly just issues like not checking
for malloc() failure.
Stephen Hemminger (11):
lib/fs: fix file leak in task_get_name
ipmaddr: fix dereference of NULL on malloc() failure
iproute_lwtunnel: fix possible use of NULL when malloc() fails
tc_filter: fix unitialized warning
tc_util fix unitialized warning
tc_exec: don't dereference NULL on calloc failure
m_action: fix warning of overwrite of const string
netem: fix NULL deref on allocation failure
nstat: fix potential NULL deref
rdma/utils: fix some analyzer warnings
tc/prio: handle possible truncated kernel response
ip/ipmaddr.c | 9 ++++++++-
ip/iproute_lwtunnel.c | 18 +++++++++++++-----
lib/fs.c | 4 +++-
misc/nstat.c | 6 ++++++
rdma/utils.c | 10 ++++++++++
tc/m_action.c | 4 ++--
tc/q_netem.c | 3 +++
tc/q_prio.c | 2 ++
tc/tc_exec.c | 4 ++++
tc/tc_filter.c | 7 ++++---
tc/tc_util.c | 2 +-
11 files changed, 56 insertions(+), 13 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH iproute2 01/11] lib/fs: fix file leak in task_get_name
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 02/11] ipmaddr: fix dereference of NULL on malloc() failure Stephen Hemminger
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Fixes the problem identified -fanalyzer.
Why did rdma choose to reimplement the same function as
exiting glibc pthread_getname().
fs.c: In function ‘get_task_name’:
fs.c:355:12: warning: leak of FILE ‘f’ [CWE-775] [-Wanalyzer-file-leak]
355 | if (!fgets(name, len, f))
| ^
‘get_task_name’: events 1-9
|
| 345 | if (!pid)
| | ^
| | |
| | (1) following ‘false’ branch (when ‘pid != 0’)...
|......
| 348 | if (snprintf(path, sizeof(path), "/proc/%d/comm", pid) >= sizeof(path))
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(2) ...to here
| | (3) following ‘false’ branch...
|......
| 351 | f = fopen(path, "r");
| | ~~~~~~~~~~~~~~~~
| | |
| | (4) ...to here
| | (5) opened here
| 352 | if (!f)
| | ~
| | |
| | (6) assuming ‘f’ is non-NULL
| | (7) following ‘false’ branch (when ‘f’ is non-NULL)...
|......
| 355 | if (!fgets(name, len, f))
| | ~ ~~~~~~~~~~~~~~~~~~~
| | | |
| | | (8) ...to here
| | (9) following ‘true’ branch...
|
‘get_task_name’: event 10
|
|cc1:
| (10): ...to here
|
‘get_task_name’: event 11
|
| 355 | if (!fgets(name, len, f))
| | ^
| | |
| | (11) ‘f’ leaks here; was opened at (5)
|
fs.c:355:12: warning: leak of ‘f’ [CWE-401] [-Wanalyzer-malloc-leak]
‘get_task_name’: events 1-9
|
| 345 | if (!pid)
| | ^
| | |
| | (1) following ‘false’ branch (when ‘pid != 0’)...
|......
| 348 | if (snprintf(path, sizeof(path), "/proc/%d/comm", pid) >= sizeof(path))
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(2) ...to here
| | (3) following ‘false’ branch...
|......
| 351 | f = fopen(path, "r");
| | ~~~~~~~~~~~~~~~~
| | |
| | (4) ...to here
| | (5) allocated here
| 352 | if (!f)
| | ~
| | |
| | (6) assuming ‘f’ is non-NULL
| | (7) following ‘false’ branch (when ‘f’ is non-NULL)...
|......
| 355 | if (!fgets(name, len, f))
| | ~ ~~~~~~~~~~~~~~~~~~~
| | | |
| | | (8) ...to here
| | (9) following ‘true’ branch...
|
‘get_task_name’: event 10
|
|cc1:
| (10): ...to here
|
‘get_task_name’: event 11
|
| 355 | if (!fgets(name, len, f))
| | ^
| | |
| | (11) ‘f’ leaks here; was allocated at (5)
Fixes: 81bfd01a4c9e ("lib: move get_task_name() from rdma")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/fs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/fs.c b/lib/fs.c
index 22d4af7583dd..7f4b159ccb65 100644
--- a/lib/fs.c
+++ b/lib/fs.c
@@ -352,8 +352,10 @@ int get_task_name(pid_t pid, char *name, size_t len)
if (!f)
return -1;
- if (!fgets(name, len, f))
+ if (!fgets(name, len, f)) {
+ fclose(f);
return -1;
+ }
/* comm ends in \n, get rid of it */
name[strcspn(name, "\n")] = '\0';
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 02/11] ipmaddr: fix dereference of NULL on malloc() failure
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 01/11] lib/fs: fix file leak in task_get_name Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 03/11] iproute_lwtunnel: fix possible use of NULL when malloc() fails Stephen Hemminger
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Found by -fanalyzer. This is a bug since beginning of initial
versions of ip multicast support (pre git).
ipmaddr.c: In function ‘read_dev_mcast’:
ipmaddr.c:105:25: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
105 | memcpy(ma, &m, sizeof(m));
| ^~~~~~~~~~~~~~~~~~~~~~~~~
‘do_multiaddr’: events 1-4
|
| 354 | int do_multiaddr(int argc, char **argv)
| | ^~~~~~~~~~~~
| | |
| | (1) entry to ‘do_multiaddr’
| 355 | {
| 356 | if (argc < 1)
| | ~
| | |
| | (2) following ‘true’ branch (when ‘argc <= 0’)...
| 357 | return multiaddr_list(0, NULL);
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
| | (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
|
+--> ‘multiaddr_list’: events 5-10
|
| 255 | static int multiaddr_list(int argc, char **argv)
| | ^~~~~~~~~~~~~~
| | |
| | (5) entry to ‘multiaddr_list’
|......
| 262 | while (argc > 0) {
| | ~~~~~~~~
| | |
| | (6) following ‘false’ branch (when ‘argc <= 0’)...
|......
| 275 | if (!filter.family || filter.family == AF_PACKET)
| | ~ ~~~~~~~~~~~~~
| | | |
| | | (7) ...to here
| | (8) following ‘true’ branch...
| 276 | read_dev_mcast(&list);
| | ~~~~~~~~~~~~~~~~~~~~~
| | |
| | (9) ...to here
| | (10) calling ‘read_dev_mcast’ from ‘multiaddr_list’
|
+--> ‘read_dev_mcast’: events 11-12
|
| 82 | static void read_dev_mcast(struct ma_info **result_p)
| | ^~~~~~~~~~~~~~
| | |
| | (11) entry to ‘read_dev_mcast’
|......
| 87 | if (!fp)
| | ~
| | |
| | (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
|
‘read_dev_mcast’: event 13
|
|cc1:
| (13): ...to here
|
‘read_dev_mcast’: events 14-17
|
| 90 | while (fgets(buf, sizeof(buf), fp)) {
| | ^~~~~
| | |
| | (14) following ‘true’ branch...
| 91 | char hexa[256];
| 92 | struct ma_info m = { .addr.family = AF_PACKET };
| | ~
| | |
| | (15) ...to here
|......
| 103 | struct ma_info *ma = malloc(sizeof(m));
| | ~~~~~~~~~~~~~~~~~
| | |
| | (16) this call could return NULL
| 104 |
| 105 | memcpy(ma, &m, sizeof(m));
| | ~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (17) ‘ma’ could be NULL: unchecked value from (16)
|
ipmaddr.c: In function ‘read_igmp’:
ipmaddr.c:152:17: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
152 | memcpy(ma, &m, sizeof(m));
| ^~~~~~~~~~~~~~~~~~~~~~~~~
‘do_multiaddr’: events 1-4
|
| 354 | int do_multiaddr(int argc, char **argv)
| | ^~~~~~~~~~~~
| | |
| | (1) entry to ‘do_multiaddr’
| 355 | {
| 356 | if (argc < 1)
| | ~
| | |
| | (2) following ‘true’ branch (when ‘argc <= 0’)...
| 357 | return multiaddr_list(0, NULL);
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
| | (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
|
+--> ‘multiaddr_list’: events 5-10
|
| 255 | static int multiaddr_list(int argc, char **argv)
| | ^~~~~~~~~~~~~~
| | |
| | (5) entry to ‘multiaddr_list’
|......
| 262 | while (argc > 0) {
| | ~~~~~~~~
| | |
| | (6) following ‘false’ branch (when ‘argc <= 0’)...
|......
| 275 | if (!filter.family || filter.family == AF_PACKET)
| | ~~~~~~~~~~~~~
| | |
| | (7) ...to here
| 276 | read_dev_mcast(&list);
| 277 | if (!filter.family || filter.family == AF_INET)
| | ~
| | |
| | (8) following ‘true’ branch...
| 278 | read_igmp(&list);
| | ~~~~~~~~~~~~~~~~
| | |
| | (9) ...to here
| | (10) calling ‘read_igmp’ from ‘multiaddr_list’
|
+--> ‘read_igmp’: events 11-14
|
| 116 | static void read_igmp(struct ma_info **result_p)
| | ^~~~~~~~~
| | |
| | (11) entry to ‘read_igmp’
|......
| 126 | if (!fp)
| | ~
| | |
| | (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
| 127 | return;
| 128 | if (!fgets(buf, sizeof(buf), fp)) {
| | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | | |
| | | (13) ...to here
| | (14) following ‘false’ branch...
|
‘read_igmp’: event 15
|
|cc1:
| (15): ...to here
|
‘read_igmp’: events 16-19
|
| 133 | while (fgets(buf, sizeof(buf), fp)) {
| | ^~~~~
| | |
| | (16) following ‘true’ branch...
|......
| 136 | if (buf[0] != '\t') {
| | ~~~~~~
| | |
| | (17) ...to here
|......
| 151 | ma = malloc(sizeof(m));
| | ~~~~~~~~~~~~~~~~~
| | |
| | (18) this call could return NULL
| 152 | memcpy(ma, &m, sizeof(m));
| | ~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (19) ‘ma’ could be NULL: unchecked value from (18)
|
ipmaddr.c: In function ‘read_igmp6’:
ipmaddr.c:181:25: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
181 | memcpy(ma, &m, sizeof(m));
| ^~~~~~~~~~~~~~~~~~~~~~~~~
‘do_multiaddr’: events 1-4
|
| 354 | int do_multiaddr(int argc, char **argv)
| | ^~~~~~~~~~~~
| | |
| | (1) entry to ‘do_multiaddr’
| 355 | {
| 356 | if (argc < 1)
| | ~
| | |
| | (2) following ‘true’ branch (when ‘argc <= 0’)...
| 357 | return multiaddr_list(0, NULL);
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
| | (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
|
+--> ‘multiaddr_list’: events 5-10
|
| 255 | static int multiaddr_list(int argc, char **argv)
| | ^~~~~~~~~~~~~~
| | |
| | (5) entry to ‘multiaddr_list’
|......
| 262 | while (argc > 0) {
| | ~~~~~~~~
| | |
| | (6) following ‘false’ branch (when ‘argc <= 0’)...
|......
| 275 | if (!filter.family || filter.family == AF_PACKET)
| | ~~~~~~~~~~~~~
| | |
| | (7) ...to here
|......
| 279 | if (!filter.family || filter.family == AF_INET6)
| | ~
| | |
| | (8) following ‘true’ branch...
| 280 | read_igmp6(&list);
| | ~~~~~~~~~~~~~~~~~
| | |
| | (9) ...to here
| | (10) calling ‘read_igmp6’ from ‘multiaddr_list’
|
+--> ‘read_igmp6’: events 11-12
|
| 159 | static void read_igmp6(struct ma_info **result_p)
| | ^~~~~~~~~~
| | |
| | (11) entry to ‘read_igmp6’
|......
| 164 | if (!fp)
| | ~
| | |
| | (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
|
‘read_igmp6’: event 13
|
|cc1:
| (13): ...to here
|
‘read_igmp6’: events 14-17
|
| 167 | while (fgets(buf, sizeof(buf), fp)) {
| | ^~~~~
| | |
| | (14) following ‘true’ branch...
| 168 | char hexa[256];
| 169 | struct ma_info m = { .addr.family = AF_INET6 };
| | ~
| | |
| | (15) ...to here
|......
| 179 | struct ma_info *ma = malloc(sizeof(m));
| | ~~~~~~~~~~~~~~~~~
| | |
| | (16) this call could return NULL
| 180 |
| 181 | memcpy(ma, &m, sizeof(m));
| | ~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (17) ‘ma’ could be NULL: unchecked value from (16)
|
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
ip/ipmaddr.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c
index f8d6b992d254..a8ef20ec624a 100644
--- a/ip/ipmaddr.c
+++ b/ip/ipmaddr.c
@@ -102,6 +102,8 @@ static void read_dev_mcast(struct ma_info **result_p)
if (len >= 0) {
struct ma_info *ma = malloc(sizeof(m));
+ if (ma == NULL)
+ break;
memcpy(ma, &m, sizeof(m));
ma->addr.bytelen = len;
ma->addr.bitlen = len<<3;
@@ -149,6 +151,9 @@ static void read_igmp(struct ma_info **result_p)
sscanf(buf, "%08x%d", (__u32 *)&m.addr.data, &m.users);
ma = malloc(sizeof(m));
+ if (ma == NULL)
+ break;
+
memcpy(ma, &m, sizeof(m));
maddr_ins(result_p, ma);
}
@@ -178,8 +183,10 @@ static void read_igmp6(struct ma_info **result_p)
if (len >= 0) {
struct ma_info *ma = malloc(sizeof(m));
- memcpy(ma, &m, sizeof(m));
+ if (ma == NULL)
+ break;
+ memcpy(ma, &m, sizeof(m));
ma->addr.bytelen = len;
ma->addr.bitlen = len<<3;
maddr_ins(result_p, ma);
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 03/11] iproute_lwtunnel: fix possible use of NULL when malloc() fails
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 01/11] lib/fs: fix file leak in task_get_name Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 02/11] ipmaddr: fix dereference of NULL on malloc() failure Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 04/11] tc_filter: fix unitialized warning Stephen Hemminger
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
iproute_lwtunnel.c: In function ‘parse_srh’:
iproute_lwtunnel.c:903:9: warning: use of possibly-NULL ‘srh’ where non-null expected [CWE-690] [-Wanalyzer-possible-null-argument]
903 | memset(srh, 0, srhlen);
| ^~~~~~~~~~~~~~~~~~~~~~
‘parse_srh’: events 1-2
|
| 902 | srh = malloc(srhlen);
| | ^~~~~~~~~~~~~~
| | |
| | (1) this call could return NULL
| 903 | memset(srh, 0, srhlen);
| | ~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) argument 1 (‘srh’) from (1) could be NULL where non-null expected
|
In file included from iproute_lwtunnel.c:13:
/usr/include/string.h:61:14: note: argument 1 of ‘memset’ must be non-null
61 | extern void *memset (void *__s, int __c, size_t __n) __THROW __nonnull ((1));
| ^~~~~~
iproute_lwtunnel.c: In function ‘parse_encap_seg6’:
iproute_lwtunnel.c:980:9: warning: use of possibly-NULL ‘tuninfo’ where non-null expected [CWE-690] [-Wanalyzer-possible-null-argument]
980 | memset(tuninfo, 0, sizeof(*tuninfo) + srhlen);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
‘parse_encap_seg6’: events 1-2
|
| 934 | static int parse_encap_seg6(struct rtattr *rta, size_t len, int *argcp,
| | ^~~~~~~~~~~~~~~~
| | |
| | (1) entry to ‘parse_encap_seg6’
|......
| 976 | srh = parse_srh(segbuf, hmac, encap);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) calling ‘parse_srh’ from ‘parse_encap_seg6’
|
+--> ‘parse_srh’: events 3-5
|
| 882 | static struct ipv6_sr_hdr *parse_srh(char *segbuf, int hmac, bool encap)
| | ^~~~~~~~~
| | |
| | (3) entry to ‘parse_srh’
|......
| 922 | if (hmac) {
| | ~
| | |
| | (4) following ‘false’ branch (when ‘hmac == 0’)...
|......
| 931 | return srh;
| | ~~~
| | |
| | (5) ...to here
|
<------+
|
‘parse_encap_seg6’: events 6-8
|
| 976 | srh = parse_srh(segbuf, hmac, encap);
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (6) returning to ‘parse_encap_seg6’ from ‘parse_srh’
|......
| 979 | tuninfo = malloc(sizeof(*tuninfo) + srhlen);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (7) this call could return NULL
| 980 | memset(tuninfo, 0, sizeof(*tuninfo) + srhlen);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (8) argument 1 (‘tuninfo’) from (7) could be NULL where non-null expected
|
/usr/include/string.h:61:14: note: argument 1 of ‘memset’ must be non-null
61 | extern void *memset (void *__s, int __c, size_t __n) __THROW __nonnull ((1));
| ^~~~~~
iproute_lwtunnel.c: In function ‘parse_rpl_srh’:
iproute_lwtunnel.c:1018:21: warning: dereference of possibly-NULL ‘srh’ [CWE-690] [-Wanalyzer-possible-null-dereference]
1018 | srh->hdrlen = (srhlen >> 3) - 1;
| ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
‘parse_rpl_srh’: events 1-2
|
| 1016 | srh = calloc(1, srhlen);
| | ^~~~~~~~~~~~~~~~~
| | |
| | (1) this call could return NULL
| 1017 |
| 1018 | srh->hdrlen = (srhlen >> 3) - 1;
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) ‘srh’ could be NULL: unchecked value from (1)
|
Fixes: 00e76d4da37f ("iproute: add helper functions for SRH processing")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
ip/iproute_lwtunnel.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 308178efe054..96de3b207ef4 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -900,6 +900,9 @@ static struct ipv6_sr_hdr *parse_srh(char *segbuf, int hmac, bool encap)
srhlen += 40;
srh = malloc(srhlen);
+ if (srh == NULL)
+ return NULL;
+
memset(srh, 0, srhlen);
srh->hdrlen = (srhlen >> 3) - 1;
@@ -935,14 +938,14 @@ static int parse_encap_seg6(struct rtattr *rta, size_t len, int *argcp,
char ***argvp)
{
int mode_ok = 0, segs_ok = 0, hmac_ok = 0;
- struct seg6_iptunnel_encap *tuninfo;
+ struct seg6_iptunnel_encap *tuninfo = NULL;
struct ipv6_sr_hdr *srh;
char **argv = *argvp;
char segbuf[1024] = "";
int argc = *argcp;
int encap = -1;
__u32 hmac = 0;
- int ret = 0;
+ int ret = -1;
int srhlen;
while (argc > 0) {
@@ -974,9 +977,13 @@ static int parse_encap_seg6(struct rtattr *rta, size_t len, int *argcp,
}
srh = parse_srh(segbuf, hmac, encap);
+ if (srh == NULL)
+ goto out;
srhlen = (srh->hdrlen + 1) << 3;
tuninfo = malloc(sizeof(*tuninfo) + srhlen);
+ if (tuninfo == NULL)
+ goto out;
memset(tuninfo, 0, sizeof(*tuninfo) + srhlen);
tuninfo->mode = encap;
@@ -984,13 +991,12 @@ static int parse_encap_seg6(struct rtattr *rta, size_t len, int *argcp,
memcpy(tuninfo->srh, srh, srhlen);
if (rta_addattr_l(rta, len, SEG6_IPTUNNEL_SRH, tuninfo,
- sizeof(*tuninfo) + srhlen)) {
- ret = -1;
+ sizeof(*tuninfo) + srhlen))
goto out;
- }
*argcp = argc + 1;
*argvp = argv - 1;
+ ret = 0;
out:
free(tuninfo);
@@ -1014,6 +1020,8 @@ static struct ipv6_rpl_sr_hdr *parse_rpl_srh(char *segbuf)
srhlen = 8 + 16 * nsegs;
srh = calloc(1, srhlen);
+ if (srh == NULL)
+ return NULL;
srh->hdrlen = (srhlen >> 3) - 1;
srh->type = 3;
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 04/11] tc_filter: fix unitialized warning
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (2 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 03/11] iproute_lwtunnel: fix possible use of NULL when malloc() fails Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 05/11] tc_util " Stephen Hemminger
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
When run with -fanalyzer.
tc_filter.c: In function ‘tc_filter_list’:
tc_filter.c:718:17: warning: use of uninitialized value ‘chain_index’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
718 | addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
‘do_chain’: events 1-4
|
| 772 | int do_chain(int argc, char **argv)
| | ^~~~~~~~
| | |
| | (1) entry to ‘do_chain’
| 773 | {
| 774 | if (argc < 1)
| | ~
| | |
| | (2) following ‘true’ branch (when ‘argc <= 0’)...
| 775 | return tc_filter_list(RTM_GETCHAIN, 0, NULL);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
| | (4) calling ‘tc_filter_list’ from ‘do_chain’
|
+--> ‘tc_filter_list’: events 5-8
|
| 582 | static int tc_filter_list(int cmd, int argc, char **argv)
| | ^~~~~~~~~~~~~~
| | |
| | (5) entry to ‘tc_filter_list’
|......
| 597 | __u32 chain_index;
| | ~~~~~~~~~~~
| | |
| | (6) region created on stack here
| | (7) capacity: 4 bytes
|......
| 601 | while (argc > 0) {
| | ~~~~~~~~
| | |
| | (8) following ‘false’ branch (when ‘argc <= 0’)...
|
‘tc_filter_list’: event 9
|
|../include/uapi/linux/pkt_sched.h:72:35:
| 72 | #define TC_H_MAKE(maj,min) (((maj)&TC_H_MAJ_MASK)|((min)&TC_H_MIN_MASK))
| | ~~~~~~^~~~~~~~~~~~~~~
| | |
| | (9) ...to here
tc_filter.c:698:26: note: in expansion of macro ‘TC_H_MAKE’
| 698 | req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
| | ^~~~~~~~~
|
‘tc_filter_list’: events 10-16
|
| 702 | if (d[0]) {
| | ^
| | |
| | (10) following ‘false’ branch...
|......
| 707 | } else if (block_index) {
| | ~~~~~~~~~~~~
| | ||
| | |(11) ...to here
| | (12) following ‘false’ branch...
|......
| 717 | if (filter_chain_index_set)
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(13) ...to here
| | (14) following ‘true’ branch...
| 718 | addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (15) ...to here
| | (16) use of uninitialized value ‘chain_index’ here
|
tc_filter.c:718:17: warning: use of uninitialized value ‘chain_index’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
718 | addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
‘do_filter’: events 1-4
|
| 744 | int do_filter(int argc, char **argv)
| | ^~~~~~~~~
| | |
| | (1) entry to ‘do_filter’
| 745 | {
| 746 | if (argc < 1)
| | ~
| | |
| | (2) following ‘true’ branch (when ‘argc <= 0’)...
| 747 | return tc_filter_list(RTM_GETTFILTER, 0, NULL);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
| | (4) calling ‘tc_filter_list’ from ‘do_filter’
|
+--> ‘tc_filter_list’: events 5-8
|
| 582 | static int tc_filter_list(int cmd, int argc, char **argv)
| | ^~~~~~~~~~~~~~
| | |
| | (5) entry to ‘tc_filter_list’
|......
| 597 | __u32 chain_index;
| | ~~~~~~~~~~~
| | |
| | (6) region created on stack here
| | (7) capacity: 4 bytes
|......
| 601 | while (argc > 0) {
| | ~~~~~~~~
| | |
| | (8) following ‘false’ branch (when ‘argc <= 0’)...
|
‘tc_filter_list’: event 9
|
|../include/uapi/linux/pkt_sched.h:72:35:
| 72 | #define TC_H_MAKE(maj,min) (((maj)&TC_H_MAJ_MASK)|((min)&TC_H_MIN_MASK))
| | ~~~~~~^~~~~~~~~~~~~~~
| | |
| | (9) ...to here
tc_filter.c:698:26: note: in expansion of macro ‘TC_H_MAKE’
| 698 | req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
| | ^~~~~~~~~
|
‘tc_filter_list’: events 10-16
|
| 702 | if (d[0]) {
| | ^
| | |
| | (10) following ‘false’ branch...
|......
| 707 | } else if (block_index) {
| | ~~~~~~~~~~~~
| | ||
| | |(11) ...to here
| | (12) following ‘false’ branch...
|......
| 717 | if (filter_chain_index_set)
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(13) ...to here
| | (14) following ‘true’ branch...
| 718 | addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (15) ...to here
| | (16) use of uninitialized value ‘chain_index’ here
|
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
tc/tc_filter.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index 700a09f62882..a1203c73738a 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -70,7 +70,7 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv)
__u32 protocol = 0;
int protocol_set = 0;
__u32 block_index = 0;
- __u32 chain_index;
+ __u32 chain_index = 0;
int chain_index_set = 0;
char *fhandle = NULL;
char d[IFNAMSIZ] = {};
@@ -594,7 +594,6 @@ static int tc_filter_list(int cmd, int argc, char **argv)
char d[IFNAMSIZ] = {};
__u32 prio = 0;
__u32 protocol = 0;
- __u32 chain_index;
__u32 block_index = 0;
char *fhandle = NULL;
@@ -676,6 +675,8 @@ static int tc_filter_list(int cmd, int argc, char **argv)
protocol = res;
filter_protocol = protocol;
} else if (matches(*argv, "chain") == 0) {
+ __u32 chain_index;
+
NEXT_ARG();
if (filter_chain_index_set)
duparg("chain", *argv);
@@ -715,7 +716,7 @@ static int tc_filter_list(int cmd, int argc, char **argv)
}
if (filter_chain_index_set)
- addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
+ addattr32(&req.n, sizeof(req), TCA_CHAIN, filter_chain_index);
if (brief) {
struct nla_bitfield32 flags = {
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 05/11] tc_util fix unitialized warning
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (3 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 04/11] tc_filter: fix unitialized warning Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 06/11] tc_exec: don't dereference NULL on calloc failure Stephen Hemminger
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
tc_util.c: In function ‘parse_action_control_slash_spaces’:
tc_util.c:488:28: warning: use of uninitialized value ‘result2’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
488 | *result2_p = result2;
| ~~~~~~~~~~~^~~~~~~~~
‘parse_action_control_slash_spaces’: events 1-5
|
| 455 | static int parse_action_control_slash_spaces(int *argc_p, char ***argv_p,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (1) entry to ‘parse_action_control_slash_spaces’
|......
| 461 | int result1 = -1, result2;
| | ~~~~~~~
| | |
| | (2) region created on stack here
| | (3) capacity: 4 bytes
|......
| 467 | switch (ok) {
| | ~~~~~~
| | |
| | (4) following ‘case 0:’ branch...
|......
| 475 | ret = parse_action_control(&argc, &argv,
| | ~
| | |
| | (5) inlined call to ‘parse_action_control’ from ‘parse_action_control_slash_spaces’
|
+--> ‘parse_action_control’: events 6-7
|
| 432 | return __parse_action_control(argc_p, argv_p, result_p,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (6) ...to here
| | (7) calling ‘__parse_action_control’ from ‘parse_action_control_slash_spaces’
| 433 | allow_num, false);
| | ~~~~~~~~~~~~~~~~~
|
‘__parse_action_control’: events 8-11
|
| 371 | static int __parse_action_control(int *argc_p, char ***argv_p, int *result_p,
| | ^~~~~~~~~~~~~~~~~~~~~~
| | |
| | (8) entry to ‘__parse_action_control’
|......
| 378 | if (!argc)
| | ~
| | |
| | (9) following ‘false’ branch (when ‘argc != 0’)...
| 379 | return -1;
| 380 | if (action_a2n(*argv, &result, allow_num) == -1) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (10) ...to here
| | (11) calling ‘action_a2n’ from ‘__parse_action_control’
|
+--> ‘action_a2n’: events 12-16
|
| 335 | int action_a2n(char *arg, int *result, bool allow_num)
| | ^~~~~~~~~~
| | |
| | (12) entry to ‘action_a2n’
|......
| 356 | for (iter = a2n; iter->a; iter++) {
| | ~~~~
| | |
| | (13) following ‘true’ branch...
| 357 | if (matches(arg, iter->a) != 0)
| | ~~~~~~~~~~~~~~~~~~~~~
| | |
| | (14) ...to here
|......
| 366 | if (result)
| | ~
| | |
| | (15) following ‘true’ branch (when ‘result’ is non-NULL)...
| 367 | *result = n;
| | ~~~~~~~~~~~
| | |
| | (16) ...to here
|
<------+
|
‘__parse_action_control’: event 17
|
| 380 | if (action_a2n(*argv, &result, allow_num) == -1) {
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (17) returning to ‘__parse_action_control’ from ‘action_a2n’
|
<------+
|
‘parse_action_control_slash_spaces’: event 18
|
| 475 | ret = parse_action_control(&argc, &argv,
| | ^
| | |
| | (18) inlined call to ‘parse_action_control’ from ‘parse_action_control_slash_spaces’
|
+--> ‘parse_action_control’: event 19
|
| 432 | return __parse_action_control(argc_p, argv_p, result_p,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (19) returning to ‘parse_action_control_slash_spaces’ from ‘__parse_action_control’
| 433 | allow_num, false);
| | ~~~~~~~~~~~~~~~~~
|
<------+
|
‘parse_action_control_slash_spaces’: events 20-24
|
| 477 | if (ret)
| | ^
| | |
| | (20) following ‘false’ branch...
| 478 | return ret;
| 479 | ok++;
| | ~~~~
| | |
| | (21) ...to here
|......
| 487 | if (ok == 2)
| | ~
| | |
| | (22) following ‘true’ branch (when ‘ok == 2’)...
| 488 | *result2_p = result2;
| | ~~~~~~~~~~~~~~~~~~~~
| | |
| | (23) ...to here
| | (24) use of uninitialized value ‘result2’ here
|
tc_util.c:488:28: warning: use of uninitialized value ‘result2’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
488 | *result2_p = result2;
| ~~~~~~~~~~~^~~~~~~~~
‘parse_action_control_slash’: events 1-5
|
| 505 | int parse_action_control_slash(int *argc_p, char ***argv_p,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (1) entry to ‘parse_action_control_slash’
|......
| 510 | char *p = strchr(*argv, '/');
| | ~~~~~~~~~~~~~~~~~~
| | |
| | (2) when ‘strchr’ returns NULL
| 511 |
| 512 | if (!p)
| | ~
| | |
| | (3) following ‘true’ branch (when ‘p’ is NULL)...
| 513 | return parse_action_control_slash_spaces(argc_p, argv_p,
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (4) ...to here
| | (5) calling ‘parse_action_control_slash_spaces’ from ‘parse_action_control_slash’
| 514 | result1_p, result2_p,
| | ~~~~~~~~~~~~~~~~~~~~~
| 515 | allow_num);
| | ~~~~~~~~~~
|
+--> ‘parse_action_control_slash_spaces’: events 6-10
|
| 455 | static int parse_action_control_slash_spaces(int *argc_p, char ***argv_p,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (6) entry to ‘parse_action_control_slash_spaces’
|......
| 461 | int result1 = -1, result2;
| | ~~~~~~~
| | |
| | (7) region created on stack here
| | (8) capacity: 4 bytes
|......
| 467 | switch (ok) {
| | ~~~~~~
| | |
| | (9) following ‘case 0:’ branch...
|......
| 475 | ret = parse_action_control(&argc, &argv,
| | ~
| | |
| | (10) inlined call to ‘parse_action_control’ from ‘parse_action_control_slash_spaces’
|
+--> ‘parse_action_control’: events 11-12
|
| 432 | return __parse_action_control(argc_p, argv_p, result_p,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (11) ...to here
| | (12) calling ‘__parse_action_control’ from ‘parse_action_control_slash_spaces’
| 433 | allow_num, false);
| | ~~~~~~~~~~~~~~~~~
|
‘__parse_action_control’: events 13-16
|
| 371 | static int __parse_action_control(int *argc_p, char ***argv_p, int *result_p,
| | ^~~~~~~~~~~~~~~~~~~~~~
| | |
| | (13) entry to ‘__parse_action_control’
|......
| 378 | if (!argc)
| | ~
| | |
| | (14) following ‘false’ branch (when ‘argc != 0’)...
| 379 | return -1;
| 380 | if (action_a2n(*argv, &result, allow_num) == -1) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (15) ...to here
| | (16) calling ‘action_a2n’ from ‘__parse_action_control’
|
+--> ‘action_a2n’: events 17-21
|
| 335 | int action_a2n(char *arg, int *result, bool allow_num)
| | ^~~~~~~~~~
| | |
| | (17) entry to ‘action_a2n’
|......
| 356 | for (iter = a2n; iter->a; iter++) {
| | ~~~~
| | |
| | (18) following ‘true’ branch...
| 357 | if (matches(arg, iter->a) != 0)
| | ~~~~~~~~~~~~~~~~~~~~~
| | |
| | (19) ...to here
|......
| 366 | if (result)
| | ~
| | |
| | (20) following ‘true’ branch (when ‘result’ is non-NULL)...
| 367 | *result = n;
| | ~~~~~~~~~~~
| | |
| | (21) ...to here
|
<------+
|
‘__parse_action_control’: event 22
|
| 380 | if (action_a2n(*argv, &result, allow_num) == -1) {
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (22) returning to ‘__parse_action_control’ from ‘action_a2n’
|
<------+
|
‘parse_action_control_slash_spaces’: event 23
|
| 475 | ret = parse_action_control(&argc, &argv,
| | ^
| | |
| | (23) inlined call to ‘parse_action_control’ from ‘parse_action_control_slash_spaces’
|
+--> ‘parse_action_control’: event 24
|
| 432 | return __parse_action_control(argc_p, argv_p, result_p,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (24) returning to ‘parse_action_control_slash_spaces’ from ‘__parse_action_control’
| 433 | allow_num, false);
| | ~~~~~~~~~~~~~~~~~
|
<------+
|
‘parse_action_control_slash_spaces’: events 25-29
|
| 477 | if (ret)
| | ^
| | |
| | (25) following ‘false’ branch...
| 478 | return ret;
| 479 | ok++;
| | ~~~~
| | |
| | (26) ...to here
|......
| 487 | if (ok == 2)
| | ~
| | |
| | (27) following ‘true’ branch (when ‘ok == 2’)...
| 488 | *result2_p = result2;
| | ~~~~~~~~~~~~~~~~~~~~
| | |
| | (28) ...to here
| | (29) use of uninitialized value ‘result2’ here
|
Fixes: e67aba559581 ("tc: actions: add helpers to parse and print control actions")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
tc/tc_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 0714134eb548..ed9efa70cabd 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -458,7 +458,7 @@ static int parse_action_control_slash_spaces(int *argc_p, char ***argv_p,
{
int argc = *argc_p;
char **argv = *argv_p;
- int result1 = -1, result2;
+ int result1 = -1, result2 = -1;
int *result_p = &result1;
int ok = 0;
int ret;
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 06/11] tc_exec: don't dereference NULL on calloc failure
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (4 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 05/11] tc_util " Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 07/11] m_action: fix warning of overwrite of const string Stephen Hemminger
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Reported as:
tc_exec.c: In function ‘do_exec’:
tc_exec.c:103:18: warning: dereference of NULL ‘eu’ [CWE-476] [-Wanalyzer-null-dereference]
103 | return eu->parse_eopt(eu, argc, argv);
| ~~^~~~~~~~~~~~
‘do_exec’: events 1-6
|
| 81 | int do_exec(int argc, char **argv)
| | ^~~~~~~
| | |
| | (1) entry to ‘do_exec’
|......
| 86 | if (argc < 1) {
| | ~
| | |
| | (2) following ‘false’ branch (when ‘argc > 0’)...
|......
| 91 | if (matches(*argv, "help") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(3) ...to here
| | (4) following ‘true’ branch...
|......
| 96 | strncpy(kind, *argv, sizeof(kind) - 1);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (5) ...to here
| 97 |
| 98 | eu = get_exec_kind(kind);
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (6) calling ‘get_exec_kind’ from ‘do_exec’
|
+--> ‘get_exec_kind’: events 7-10
|
| 40 | static struct exec_util *get_exec_kind(const char *name)
| | ^~~~~~~~~~~~~
| | |
| | (7) entry to ‘get_exec_kind’
|......
| 63 | if (eu == NULL)
| | ~
| | |
| | (8) following ‘true’ branch (when ‘eu’ is NULL)...
| 64 | goto noexist;
| | ~~~~
| | |
| | (9) ...to here
|......
| 72 | if (eu) {
| | ~
| | |
| | (10) following ‘false’ branch (when ‘eu’ is NULL)...
|
‘get_exec_kind’: event 11
|
|cc1:
| (11): ...to here
|
<------+
|
‘do_exec’: events 12-13
|
| 98 | eu = get_exec_kind(kind);
| | ^~~~~~~~~~~~~~~~~~~
| | |
| | (12) return of NULL to ‘do_exec’ from ‘get_exec_kind’
|......
| 103 | return eu->parse_eopt(eu, argc, argv);
| | ~~~~~~~~~~~~~~
| | |
| | (13) dereference of NULL ‘eu’
|
Fixes: 4bd624467bc6 ("tc: built-in eBPF exec proxy")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
tc/tc_exec.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tc/tc_exec.c b/tc/tc_exec.c
index 5d8834029a0b..182fbb4c35c9 100644
--- a/tc/tc_exec.c
+++ b/tc/tc_exec.c
@@ -96,6 +96,10 @@ int do_exec(int argc, char **argv)
strncpy(kind, *argv, sizeof(kind) - 1);
eu = get_exec_kind(kind);
+ if (eu == NULL) {
+ fprintf(stderr, "Allocation failed finding exec\n");
+ return -1;
+ }
argc--;
argv++;
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 07/11] m_action: fix warning of overwrite of const string
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (5 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 06/11] tc_exec: don't dereference NULL on calloc failure Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 08/11] netem: fix NULL deref on allocation failure Stephen Hemminger
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
The function get_action_kind() searches first for the given
action, then rescans on failure for "gact". In the process,
it would overwrite the argument. Avoid the warning
by using a const argument and not copying.
The problem dates back to pre-git history.
m_action.c: In function ‘get_action_kind’:
m_action.c:126:17: warning: write to string literal [-Wanalyzer-write-to-string-literal]
126 | strcpy(str, "gact");
| ^~~~~~~~~~~~~~~~~~~
‘do_action’: events 1-6
|
| 853 | int do_action(int argc, char **argv)
| | ^~~~~~~~~
| | |
| | (1) entry to ‘do_action’
|......
| 858 | while (argc > 0) {
| | ~~~~~~~~
| | |
| | (2) following ‘true’ branch...
| 859 |
| 860 | if (matches(*argv, "add") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(3) ...to here
| | (4) following ‘false’ branch...
| 861 | ret = tc_action_modify(RTM_NEWACTION,
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (5) ...to here
| | (6) calling ‘tc_action_modify’ from ‘do_action’
| 862 | NLM_F_EXCL | NLM_F_CREATE,
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~
| 863 | &argc, &argv);
| | ~~~~~~~~~~~~~
|
+--> ‘tc_action_modify’: events 7-8
|
| 715 | static int tc_action_modify(int cmd, unsigned int flags,
| | ^~~~~~~~~~~~~~~~
| | |
| | (7) entry to ‘tc_action_modify’
|......
| 735 | if (parse_action(&argc, &argv, TCA_ACT_TAB, &req.n)) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (8) calling ‘parse_action’ from ‘tc_action_modify’
|
+--> ‘parse_action’: events 9-18
|
| 203 | int parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n)
| | ^~~~~~~~~~~~
| | |
| | (9) entry to ‘parse_action’
|......
| 217 | if (argc <= 0)
| | ~
| | |
| | (10) following ‘false’ branch...
|......
| 220 | tail2 = addattr_nest(n, MAX_MSG, tca_id);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (11) ...to here
| 221 |
| 222 | while (argc > 0) {
| | ~~~~~~~~
| | |
| | (12) following ‘true’ branch...
| 223 |
| 224 | memset(k, 0, sizeof(k));
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (13) ...to here
| 225 |
| 226 | if (strcmp(*argv, "action") == 0) {
| | ~
| | |
| | (14) following ‘true’ branch (when the strings are equal)...
| 227 | argc--;
| | ~~~~~~
| | |
| | (15) ...to here
|......
| 231 | if (!gact_ld)
| | ~
| | |
| | (16) following ‘true’ branch...
| 232 | get_action_kind("gact");
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (17) ...to here
| | (18) calling ‘get_action_kind’ from ‘parse_action’
|
+--> ‘get_action_kind’: events 19-24
|
| 86 | static struct action_util *get_action_kind(char *str)
| | ^~~~~~~~~~~~~~~
| | |
| | (19) entry to ‘get_action_kind’
|......
| 114 | if (a == NULL)
| | ~
| | |
| | (20) following ‘true’ branch (when ‘a’ is NULL)...
| 115 | goto noexist;
| | ~~~~
| | |
| | (21) ...to here
|......
| 124 | if (!looked4gact) {
| | ~
| | |
| | (22) following ‘true’ branch (when ‘looked4gact == 0’)...
| 125 | looked4gact = 1;
| 126 | strcpy(str, "gact");
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (23) ...to here
| | (24) write to string literal here
|
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
tc/m_action.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tc/m_action.c b/tc/m_action.c
index a446cabdb98c..16474c56118c 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -83,7 +83,7 @@ static int parse_noaopt(struct action_util *au, int *argc_p,
return -1;
}
-static struct action_util *get_action_kind(char *str)
+static struct action_util *get_action_kind(const char *str)
{
static void *aBODY;
void *dlh;
@@ -123,7 +123,7 @@ noexist:
#ifdef CONFIG_GACT
if (!looked4gact) {
looked4gact = 1;
- strcpy(str, "gact");
+ str = "gact";
goto restart_s;
}
#endif
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 08/11] netem: fix NULL deref on allocation failure
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (6 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 07/11] m_action: fix warning of overwrite of const string Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 09/11] nstat: fix potential NULL deref Stephen Hemminger
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
q_netem.c: In function ‘get_distribution’:
q_netem.c:159:35: warning: dereference of possibly-NULL ‘data’ [CWE-690] [-Wanalyzer-possible-null-dereference]
159 | data[n++] = x;
| ~~~~~~~~~~^~~
‘netem_parse_opt’: events 1-24
|
| 192 | static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
| | ^~~~~~~~~~~~~~~
| | |
| | (1) entry to ‘netem_parse_opt’
|......
| 212 | for ( ; argc > 0; --argc, ++argv) {
| | ~~~~~~~~
| | |
| | (2) following ‘true’ branch (when ‘argc > 0’)...
| 213 | if (matches(*argv, "limit") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(3) ...to here
| | (4) following ‘true’ branch...
|......
| 219 | } else if (matches(*argv, "latency") == 0 ||
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | || |
| | |(5) ...to here (8) following ‘true’ branch...
| | (6) following ‘true’ branch...
| 220 | matches(*argv, "delay") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (7) ...to here
|......
| 243 | } else if (matches(*argv, "loss") == 0 ||
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | || |
| | |(9) ...to here (12) following ‘true’ branch...
| | (10) following ‘true’ branch...
| 244 | matches(*argv, "drop") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (11) ...to here
|......
| 366 | } else if (matches(*argv, "ecn") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(13) ...to here
| | (14) following ‘true’ branch...
| 367 | present[TCA_NETEM_ECN] = 1;
| 368 | } else if (matches(*argv, "reorder") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(15) ...to here
| | (16) following ‘true’ branch...
|......
| 383 | } else if (matches(*argv, "corrupt") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(17) ...to here
| | (18) following ‘true’ branch...
|......
| 398 | } else if (matches(*argv, "gap") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(19) ...to here
| | (20) following ‘true’ branch...
|......
| 404 | } else if (matches(*argv, "duplicate") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(21) ...to here
| | (22) following ‘true’ branch...
|......
| 417 | } else if (matches(*argv, "distribution") == 0) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | ||
| | |(23) ...to here
| | (24) following ‘false’ branch...
|
‘netem_parse_opt’: event 25
|
|../include/utils.h:50:29:
| 50 | #define NEXT_ARG() do { argv++; if (--argc <= 0) incomplete_command(); } while(0)
| | ~~~~^~
| | |
| | (25) ...to here
q_netem.c:418:25: note: in expansion of macro ‘NEXT_ARG’
| 418 | NEXT_ARG();
| | ^~~~~~~~
|
‘netem_parse_opt’: event 26
|
|../include/utils.h:50:36:
| 50 | #define NEXT_ARG() do { argv++; if (--argc <= 0) incomplete_command(); } while(0)
| | ^
| | |
| | (26) following ‘false’ branch (when ‘argc != 0’)...
q_netem.c:418:25: note: in expansion of macro ‘NEXT_ARG’
| 418 | NEXT_ARG();
| | ^~~~~~~~
|
‘netem_parse_opt’: events 27-29
|
| 419 | dist_data = calloc(sizeof(dist_data[0]), MAX_DIST);
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (27) ...to here
| | (28) this call could return NULL
| 420 | dist_size = get_distribution(*argv, dist_data, MAX_DIST);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (29) calling ‘get_distribution’ from ‘netem_parse_opt’
|
+--> ‘get_distribution’: events 30-31
|
| 124 | static int get_distribution(const char *type, __s16 *data, int maxdata)
| | ^~~~~~~~~~~~~~~~
| | |
| | (30) entry to ‘get_distribution’
|......
| 135 | if (f == NULL) {
| | ~
| | |
| | (31) following ‘false’ branch (when ‘f’ is non-NULL)...
|
‘get_distribution’: event 32
|
|cc1:
| (32): ...to here
|
‘get_distribution’: events 33-35
|
| 142 | while (getline(&line, &len, f) != -1) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
| | |
| | (33) following ‘true’ branch...
|......
| 145 | if (*line == '\n' || *line == '#')
| | ~~~~~~
| | ||
| | |(34) ...to here
| | (35) following ‘false’ branch...
|
‘get_distribution’: event 36
|
|cc1:
| (36): ...to here
|
‘get_distribution’: events 37-41
|
| 150 | if (endp == p)
| | ^
| | |
| | (37) following ‘false’ branch...
|......
| 153 | if (n >= maxdata) {
| | ~
| | |
| | (38) ...to here
| | (39) following ‘false’ branch (when ‘n < maxdata’)...
|......
| 159 | data[n++] = x;
| | ~~~~~~~~~~~~~
| | | |
| | | (41) ‘data + (long unsigned int)n * 2’ could be NULL: unchecked value from (28)
| | (40) ...to here
|
Fixes: c1b81cb5fe92 ("netem potential dist table overflow")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
tc/q_netem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tc/q_netem.c b/tc/q_netem.c
index 26402e9ad93f..d1d79b0b4d35 100644
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -417,6 +417,9 @@ random_loss_model:
} else if (matches(*argv, "distribution") == 0) {
NEXT_ARG();
dist_data = calloc(sizeof(dist_data[0]), MAX_DIST);
+ if (dist_data == NULL)
+ return -1;
+
dist_size = get_distribution(*argv, dist_data, MAX_DIST);
if (dist_size <= 0) {
free(dist_data);
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 09/11] nstat: fix potential NULL deref
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (7 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 08/11] netem: fix NULL deref on allocation failure Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 10/11] rdma/utils: fix some analyzer warnings Stephen Hemminger
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Reported as:
CC nstat
nstat.c: In function ‘load_ugly_table’:
nstat.c:205:24: warning: dereference of NULL ‘p’ [CWE-476] [-Wanalyzer-null-dereference]
205 | while (*p) {
| ^~
‘main’: events 1-14
|
| 575 | int main(int argc, char *argv[])
| | ^~~~
| | |
| | (1) entry to ‘main’
|......
| 635 | if (scan_interval > 0) {
| | ~
| | |
| | (2) following ‘true’ branch...
| 636 | if (time_constant == 0)
| | ~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
|......
| 640 | if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
| | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | | |
| | | (4) when ‘socket’ succeeds
| | (5) following ‘false’ branch (when ‘fd >= 0’)...
|......
| 644 | if (bind(fd, (struct sockaddr *)&sun, 2+1+strlen(sun.sun_path+1)) < 0) {
| | ~ ~~~~~~~~~~~~~~~~~~~~~~
| | | |
| | (7) following ‘false’ branch... (6) ...to here
|......
| 648 | if (listen(fd, 5) < 0) {
| | ~~~~~~~~~~~~~~
| | ||
| | |(8) ...to here
| | |(9) when ‘listen’ succeeds
| | (10) following ‘false’ branch...
|......
| 652 | if (daemon(0, 0)) {
| | ~~~~~~~~~~~~~
| | ||
| | |(11) ...to here
| | (12) following ‘false’ branch...
|......
| 656 | signal(SIGPIPE, SIG_IGN);
| | ~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (13) ...to here
| 657 | signal(SIGCHLD, sigchild);
| 658 | server_loop(fd);
| | ~~~~~~~~~~~~~~~
| | |
| | (14) calling ‘server_loop’ from ‘main’
|
+--> ‘server_loop’: events 15-16
|
| 472 | static void server_loop(int fd)
| | ^~~~~~~~~~~
| | |
| | (15) entry to ‘server_loop’
|......
| 483 | load_netstat();
| | ~~~~~~~~~~~~~~
| | |
| | (16) calling ‘load_netstat’ from ‘server_loop’
|
+--> ‘load_netstat’: events 17-20
|
| 302 | static void load_netstat(void)
| | ^~~~~~~~~~~~
| | |
| | (17) entry to ‘load_netstat’
|......
| 306 | if (fp) {
| | ~
| | |
| | (18) following ‘true’ branch (when ‘fp’ is non-NULL)...
| 307 | load_ugly_table(fp);
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (19) ...to here
| | (20) calling ‘load_ugly_table’ from ‘load_netstat’
|
+--> ‘load_ugly_table’: events 21-26
|
| 178 | static void load_ugly_table(FILE *fp)
| | ^~~~~~~~~~~~~~~
| | |
| | (21) entry to ‘load_ugly_table’
| 179 | {
| 180 | char *buf = NULL;
| | ~~~
| | |
| | (22) ‘buf’ is NULL
|......
| 186 | while ((nread = getline(&buf, &buflen, fp)) != -1) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (23) following ‘true’ branch...
|......
| 192 | p = strchr(buf, ':');
| | ~~~~~~~~~~~~~~~~
| | |
| | (24) ...to here
| | (25) when ‘strchr’ returns non-NULL
| 193 | if (!p) {
| | ~
| | |
| | (26) following ‘false’ branch (when ‘p’ is non-NULL)...
|
‘load_ugly_table’: event 27
|
|cc1:
| (27): ...to here
|
‘load_ugly_table’: events 28-40
|
| 205 | while (*p) {
| | ^~
| | |
| | (28) following ‘true’ branch...
| | (40) dereference of NULL ‘p’
|......
| 208 | if ((next = strchr(p, ' ')) != NULL)
| | ~ ~~~~~~~~~~~~~~
| | | |
| | | (29) ...to here
| | | (30) when ‘strchr’ returns NULL
| | (31) following ‘false’ branch (when ‘next’ is NULL)...
| 209 | *next++ = 0;
| 210 | else if ((next = strchr(p, '\n')) != NULL)
| | ~ ~~~~~~~~~~~~~~~
| | | |
| | | (32) ...to here
| | | (33) when ‘strchr’ returns NULL
| | (34) following ‘false’ branch (when ‘next’ is NULL)...
| 211 | *next++ = 0;
| 212 | if (off < sizeof(idbuf)) {
| | ~~~~~~~~~~~~~~~~~~~~
| | | |
| | | (35) ...to here
| | (36) following ‘false’ branch...
|......
| 216 | n = malloc(sizeof(*n));
| | ~~~~~~~~~~~~~~~~~~
| | |
| | (37) ...to here
| 217 | if (!n) {
| | ~
| | |
| | (38) following ‘false’ branch (when ‘n’ is non-NULL)...
|......
| 221 | n->id = strdup(idbuf);
| | ~~~~~~~~~~~~~
| | |
| | (39) ...to here
|
nstat.c:254:35: warning: dereference of NULL ‘n’ [CWE-476] [-Wanalyzer-null-dereference]
254 | n = n->next;
| ~~^~~~~~~~~
‘main’: events 1-14
|
| 575 | int main(int argc, char *argv[])
| | ^~~~
| | |
| | (1) entry to ‘main’
|......
| 635 | if (scan_interval > 0) {
| | ~
| | |
| | (2) following ‘true’ branch...
| 636 | if (time_constant == 0)
| | ~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
|......
| 640 | if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
| | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | | |
| | | (4) when ‘socket’ succeeds
| | (5) following ‘false’ branch (when ‘fd >= 0’)...
|......
| 644 | if (bind(fd, (struct sockaddr *)&sun, 2+1+strlen(sun.sun_path+1)) < 0) {
| | ~ ~~~~~~~~~~~~~~~~~~~~~~
| | | |
| | (7) following ‘false’ branch... (6) ...to here
|......
| 648 | if (listen(fd, 5) < 0) {
| | ~~~~~~~~~~~~~~
| | ||
| | |(8) ...to here
| | |(9) when ‘listen’ succeeds
| | (10) following ‘false’ branch...
|......
| 652 | if (daemon(0, 0)) {
| | ~~~~~~~~~~~~~
| | ||
| | |(11) ...to here
| | (12) following ‘false’ branch...
|......
| 656 | signal(SIGPIPE, SIG_IGN);
| | ~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (13) ...to here
| 657 | signal(SIGCHLD, sigchild);
| 658 | server_loop(fd);
| | ~~~~~~~~~~~~~~~
| | |
| | (14) calling ‘server_loop’ from ‘main’
|
+--> ‘server_loop’: events 15-16
|
| 472 | static void server_loop(int fd)
| | ^~~~~~~~~~~
| | |
| | (15) entry to ‘server_loop’
|......
| 483 | load_netstat();
| | ~~~~~~~~~~~~~~
| | |
| | (16) calling ‘load_netstat’ from ‘server_loop’
|
+--> ‘load_netstat’: events 17-20
|
| 302 | static void load_netstat(void)
| | ^~~~~~~~~~~~
| | |
| | (17) entry to ‘load_netstat’
|......
| 306 | if (fp) {
| | ~
| | |
| | (18) following ‘true’ branch (when ‘fp’ is non-NULL)...
| 307 | load_ugly_table(fp);
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (19) ...to here
| | (20) calling ‘load_ugly_table’ from ‘load_netstat’
|
+--> ‘load_ugly_table’: events 21-25
|
| 178 | static void load_ugly_table(FILE *fp)
| | ^~~~~~~~~~~~~~~
| | |
| | (21) entry to ‘load_ugly_table’
|......
| 186 | while ((nread = getline(&buf, &buflen, fp)) != -1) {
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (22) following ‘true’ branch...
|......
| 192 | p = strchr(buf, ':');
| | ~~~~~~~~~~~~~~~~
| | |
| | (23) ...to here
| | (24) when ‘strchr’ returns non-NULL
| 193 | if (!p) {
| | ~
| | |
| | (25) following ‘false’ branch (when ‘p’ is non-NULL)...
|
‘load_ugly_table’: event 26
|
|cc1:
| (26): ...to here
|
‘load_ugly_table’: events 27-28
|
| 205 | while (*p) {
| | ^
| | |
| | (27) following ‘false’ branch...
|......
| 228 | nread = getline(&buf, &buflen, fp);
| | ~
| | |
| | (28) inlined call to ‘getline’ from ‘load_ugly_table’
|
+--> ‘getline’: event 29
|
|/usr/include/bits/stdio.h:120:10:
| 120 | return __getdelim (__lineptr, __n, '\n', __stream);
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (29) ...to here
|
<------+
|
‘load_ugly_table’: events 30-36
|
|nstat.c:229:20:
| 229 | if (nread == -1) {
| | ^
| | |
| | (30) following ‘false’ branch...
|......
| 234 | count2 = count_spaces(buf);
| | ~~~~~~~~~~~~~~~~~
| | |
| | (31) ...to here
|......
| 239 | if (!p) {
| | ~
| | |
| | (32) following ‘false’ branch (when ‘p’ is non-NULL)...
|......
| 244 | *p = 0;
| | ~~~~~~
| | |
| | (33) ...to here
| 245 | if (sscanf(p+1, "%llu", &n->val) != 1) {
| | ~
| | |
| | (34) following ‘false’ branch...
|......
| 251 | if (skip)
| | ~
| | |
| | (35) ...to here
|......
| 254 | n = n->next;
| | ~~~~~~~~~~~
| | |
| | (36) dereference of NULL ‘n’
|
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
misc/nstat.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/misc/nstat.c b/misc/nstat.c
index 0ab92ecbeb47..2c10feaa3adf 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -219,9 +219,15 @@ static void load_ugly_table(FILE *fp)
exit(-1);
}
n->id = strdup(idbuf);
+ if (n->id == NULL) {
+ perror("nstat: strdup");
+ exit(-1);
+ }
n->rate = 0;
n->next = db;
db = n;
+ if (next == NULL)
+ break;
p = next;
}
n = db;
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 10/11] rdma/utils: fix some analyzer warnings
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (8 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 09/11] nstat: fix potential NULL deref Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 11/11] tc/prio: handle possible truncated kernel response Stephen Hemminger
2023-05-14 2:20 ` [PATCH iproute2 00/11] fix analyzer warnings patchwork-bot+netdevbpf
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Add error checks for cases where analyzer thinks it is possible
to us a possibly NULL value.
utils.c: In function ‘get_port_from_argv’:
utils.c:76:17: warning: use of NULL where non-null expected [CWE-476] [-Wanalyzer-null-argument]
76 | slash = strchr(rd_argv(rd), '/');
| ^~~~~~~~~~~~~~~~~~~~~~~~
‘get_port_from_argv’: events 1-2
|
| 68 | static int get_port_from_argv(struct rd *rd, uint32_t *port,
| | ^~~~~~~~~~~~~~~~~~
| | |
| | (1) entry to ‘get_port_from_argv’
|......
| 76 | slash = strchr(rd_argv(rd), '/');
| | ~
| | |
| | (2) inlined call to ‘rd_argv’ from ‘get_port_from_argv’
|
+--> ‘rd_argv’: event 3
|
| 18 | if (!rd_argc(rd))
| | ^
| | |
| | (3) following ‘true’ branch...
|
<------+
|
‘get_port_from_argv’: events 4-5
|
| 76 | slash = strchr(rd_argv(rd), '/');
| | ^~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (4) ...to here
| | (5) argument 1 (‘<unknown>’) NULL where non-null expected
|
In file included from rdma.h:10,
from utils.c:7:
/usr/include/string.h:246:14: note: argument 1 of ‘strchr’ must be non-null
246 | extern char *strchr (const char *__s, int __c)
| ^~~~~~
Fixes: 40df8263a0f0 ("rdma: Add dev object")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
rdma/utils.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/rdma/utils.c b/rdma/utils.c
index 21177b565bf1..a33ff420f8cb 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -75,6 +75,13 @@ static int get_port_from_argv(struct rd *rd, uint32_t *port,
slash = strchr(rd_argv(rd), '/');
/* if no port found, return 0 */
+ if (slash == NULL) {
+ if (strict_port)
+ return -EINVAL;
+ else
+ return 0;
+ }
+
if (slash++) {
if (*slash == '-') {
if (strict_port)
@@ -747,6 +754,9 @@ struct dev_map *dev_map_lookup(struct rd *rd, bool allow_port_index)
return NULL;
dev_name = strdup(rd_argv(rd));
+ if (!dev_name)
+ return NULL;
+
if (allow_port_index) {
slash = strrchr(dev_name, '/');
if (slash)
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH iproute2 11/11] tc/prio: handle possible truncated kernel response
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (9 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 10/11] rdma/utils: fix some analyzer warnings Stephen Hemminger
@ 2023-05-09 21:21 ` Stephen Hemminger
2023-05-14 2:20 ` [PATCH iproute2 00/11] fix analyzer warnings patchwork-bot+netdevbpf
11 siblings, 0 replies; 13+ messages in thread
From: Stephen Hemminger @ 2023-05-09 21:21 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Reported by -fanalyzer. If kernel did not send full qdisc
info, then uninitialized or null data could be referenced.
q_prio.c: In function ‘prio_print_opt’:
q_prio.c:105:57: warning: dereference of NULL ‘0’ [CWE-476] [-Wanalyzer-null-dereference]
105 | print_uint(PRINT_ANY, "bands", "bands %u ", qopt->bands);
| ~~~~^~~~~~~
‘prio_print_opt’: event 1
|
| 98 | if (opt == NULL)
| | ^
| | |
| | (1) following ‘false’ branch (when ‘opt’ is non-NULL)...
|
‘prio_print_opt’: event 2
|
|../include/uapi/linux/rtnetlink.h:228:38:
| 228 | #define RTA_PAYLOAD(rta) ((int)((rta)->rta_len) - RTA_LENGTH(0))
| | ~~~~~~^~~~~~~~~~
| | |
| | (2) ...to here
../include/libnetlink.h:236:19: note: in expansion of macro ‘RTA_PAYLOAD’
| 236 | ({ data = RTA_PAYLOAD(rta) >= len ? RTA_DATA(rta) : NULL; \
| | ^~~~~~~~~~~
q_prio.c:101:13: note: in expansion of macro ‘parse_rtattr_nested_compat’
| 101 | if (parse_rtattr_nested_compat(tb, TCA_PRIO_MAX, opt, qopt,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~
|
‘prio_print_opt’: event 3
|
|../include/libnetlink.h:236:59:
| 236 | ({ data = RTA_PAYLOAD(rta) >= len ? RTA_DATA(rta) : NULL; \
q_prio.c:101:13: note: in expansion of macro ‘parse_rtattr_nested_compat’
| 101 | if (parse_rtattr_nested_compat(tb, TCA_PRIO_MAX, opt, qopt,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~
|
‘prio_print_opt’: events 4-5
|
| 105 | print_uint(PRINT_ANY, "bands", "bands %u ", qopt->bands);
| | ~~~~^~~~~~~
| | |
| | (4) ...to here
| | (5) dereference of NULL ‘<unknown>’
|
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
tc/q_prio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tc/q_prio.c b/tc/q_prio.c
index c8c6477e1a98..a3781ffe8b2c 100644
--- a/tc/q_prio.c
+++ b/tc/q_prio.c
@@ -101,6 +101,8 @@ int prio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
if (parse_rtattr_nested_compat(tb, TCA_PRIO_MAX, opt, qopt,
sizeof(*qopt)))
return -1;
+ if (qopt == NULL)
+ return -1; /* missing data from kernel */
print_uint(PRINT_ANY, "bands", "bands %u ", qopt->bands);
open_json_array(PRINT_ANY, "priomap");
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH iproute2 00/11] fix analyzer warnings
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
` (10 preceding siblings ...)
2023-05-09 21:21 ` [PATCH iproute2 11/11] tc/prio: handle possible truncated kernel response Stephen Hemminger
@ 2023-05-14 2:20 ` patchwork-bot+netdevbpf
11 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-05-14 2:20 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Hello:
This series was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:
On Tue, 9 May 2023 14:21:14 -0700 you wrote:
> Address some (but not all) of the issues reported by gcc 13
> new analyzer. These are mostly just issues like not checking
> for malloc() failure.
>
> Stephen Hemminger (11):
> lib/fs: fix file leak in task_get_name
> ipmaddr: fix dereference of NULL on malloc() failure
> iproute_lwtunnel: fix possible use of NULL when malloc() fails
> tc_filter: fix unitialized warning
> tc_util fix unitialized warning
> tc_exec: don't dereference NULL on calloc failure
> m_action: fix warning of overwrite of const string
> netem: fix NULL deref on allocation failure
> nstat: fix potential NULL deref
> rdma/utils: fix some analyzer warnings
> tc/prio: handle possible truncated kernel response
>
> [...]
Here is the summary with links:
- [iproute2,01/11] lib/fs: fix file leak in task_get_name
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=9f0fe8ee0900
- [iproute2,02/11] ipmaddr: fix dereference of NULL on malloc() failure
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=8cda7a24a971
- [iproute2,03/11] iproute_lwtunnel: fix possible use of NULL when malloc() fails
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=fa44c2d6f1da
- [iproute2,04/11] tc_filter: fix unitialized warning
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=c73a41054638
- [iproute2,05/11] tc_util fix unitialized warning
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=c9c1c9d59a6c
- [iproute2,06/11] tc_exec: don't dereference NULL on calloc failure
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=0b9b9d659880
- [iproute2,07/11] m_action: fix warning of overwrite of const string
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=b134c2c34458
- [iproute2,08/11] netem: fix NULL deref on allocation failure
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=6c266d7c22a8
- [iproute2,09/11] nstat: fix potential NULL deref
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=d348d1d6466a
- [iproute2,10/11] rdma/utils: fix some analyzer warnings
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=33722349feb9
- [iproute2,11/11] tc/prio: handle possible truncated kernel response
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=c90d25e96b01
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] 13+ messages in thread
end of thread, other threads:[~2023-05-14 2:20 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-09 21:21 [PATCH iproute2 00/11] fix analyzer warnings Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 01/11] lib/fs: fix file leak in task_get_name Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 02/11] ipmaddr: fix dereference of NULL on malloc() failure Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 03/11] iproute_lwtunnel: fix possible use of NULL when malloc() fails Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 04/11] tc_filter: fix unitialized warning Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 05/11] tc_util " Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 06/11] tc_exec: don't dereference NULL on calloc failure Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 07/11] m_action: fix warning of overwrite of const string Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 08/11] netem: fix NULL deref on allocation failure Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 09/11] nstat: fix potential NULL deref Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 10/11] rdma/utils: fix some analyzer warnings Stephen Hemminger
2023-05-09 21:21 ` [PATCH iproute2 11/11] tc/prio: handle possible truncated kernel response Stephen Hemminger
2023-05-14 2:20 ` [PATCH iproute2 00/11] fix analyzer warnings patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).