* [iproute PATCH 31/51] ss: Drop useless assignment
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
After '*b = *a', 'b->next' already has the same value as 'a->next'.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ss.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/misc/ss.c b/misc/ss.c
index 5ea388fbf1c1a..d767b1103ea81 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1440,7 +1440,6 @@ static int remember_he(struct aafilter *a, struct hostent *he)
if ((b = malloc(sizeof(*b))) == NULL)
return cnt;
*b = *a;
- b->next = a->next;
a->next = b;
}
memcpy(b->addr.data, *ptr, len);
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 35/51] ss: Fix potential memleak in unix_stats_print()
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Fixes: 2d0e538f3e1cd ("ss: Drop list traversal from unix_stats_print()")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ss.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/misc/ss.c b/misc/ss.c
index 667b8faad6528..7d84b83c8ad71 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3170,8 +3170,10 @@ static int unix_show(struct filter *f)
if (name[0]) {
u->name = strdup(name);
- if (!u->name)
+ if (!u->name) {
+ free(u);
break;
+ }
}
if (u->rport) {
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 25/51] ifstat: Fix memleak in dump_kern_db() for json output
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Looks like this was forgotten when converting to common json output
formatter.
Fixes: fcc16c2287bf8 ("provide common json output formatter")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ifstat.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/misc/ifstat.c b/misc/ifstat.c
index 6c8cfa0bd94f5..ac3eff6b870a9 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -535,8 +535,12 @@ static void dump_kern_db(FILE *fp)
else
print_one_if(fp, n, n->val);
}
- if (json_output)
- fprintf(fp, "\n} }\n");
+ if (jw) {
+ jsonw_end_object(jw);
+
+ jsonw_end_object(jw);
+ jsonw_destroy(&jw);
+ }
}
static void dump_incr_db(FILE *fp)
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 38/51] tc/em_ipset: Don't leak sockfd on error path
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tc/em_ipset.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tc/em_ipset.c b/tc/em_ipset.c
index fab975f5ea563..b59756515d239 100644
--- a/tc/em_ipset.c
+++ b/tc/em_ipset.c
@@ -84,6 +84,7 @@ static int get_version(unsigned int *version)
res = getsockopt(sockfd, SOL_IP, SO_IP_SET, &req_version, &size);
if (res != 0) {
perror("xt_set getsockopt");
+ close(sockfd);
return -1;
}
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 26/51] lnstat_util: Simplify alloc_and_open() a bit
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Relying upon callers and using unsafe strcpy() is probably not the best
idea. Aside from that, using snprintf() allows to format the string for
lf->path in one go.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/lnstat_util.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/misc/lnstat_util.c b/misc/lnstat_util.c
index cc54598fe1bef..ec19238c24b94 100644
--- a/misc/lnstat_util.c
+++ b/misc/lnstat_util.c
@@ -180,11 +180,8 @@ static struct lnstat_file *alloc_and_open(const char *path, const char *file)
}
/* initialize */
- /* de->d_name is guaranteed to be <= NAME_MAX */
- strcpy(lf->basename, file);
- strcpy(lf->path, path);
- strcat(lf->path, "/");
- strcat(lf->path, lf->basename);
+ snprintf(lf->basename, sizeof(lf->basename), "%s", file);
+ snprintf(lf->path, sizeof(lf->path), "%s/%s", path, file);
/* initialize to default */
lf->interval.tv_sec = 1;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 13/51] iproute_lwtunnel: Argument to strerror must be positive
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/iproute_lwtunnel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 398ab5e077ed8..1a3dc4d4c0ed9 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -643,7 +643,7 @@ static int lwt_parse_bpf(struct rtattr *rta, size_t len,
err = bpf_parse_common(bpf_type, &cfg, &bpf_cb_ops, &x);
if (err < 0) {
fprintf(stderr, "Failed to parse eBPF program: %s\n",
- strerror(err));
+ strerror(-err));
return -1;
}
rta_nest_end(rta, nest);
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 01/51] devlink: Check return code of strslashrsplit()
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
This function shouldn't fail because all callers of
__dl_argv_handle_port() make sure the passed string contains enough
slashes already, but better make sure if this changes in future the
function won't access uninitialized data.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
devlink/devlink.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index f9bc16c350c40..de41a9f4aae10 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -526,18 +526,26 @@ static int __dl_argv_handle_port(char *str,
char **p_bus_name, char **p_dev_name,
uint32_t *p_port_index)
{
- char *handlestr = handlestr;
- char *portstr = portstr;
+ char *handlestr;
+ char *portstr;
int err;
- strslashrsplit(str, &handlestr, &portstr);
+ err = strslashrsplit(str, &handlestr, &portstr);
+ if (err) {
+ pr_err("Port identification \"%s\" is invalid\n", str);
+ return err;
+ }
err = strtouint32_t(portstr, p_port_index);
if (err) {
pr_err("Port index \"%s\" is not a number or not within range\n",
portstr);
return err;
}
- strslashrsplit(handlestr, p_bus_name, p_dev_name);
+ err = strslashrsplit(handlestr, p_bus_name, p_dev_name);
+ if (err) {
+ pr_err("Port identification \"%s\" is invalid\n", str);
+ return err;
+ }
return 0;
}
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 05/51] iplink_can: Prevent overstepping array bounds
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
can_state_names array contains at most CAN_STATE_MAX fields, so allowing
an index to it to be equal to that number is wrong. While here, also
make sure the array is indeed that big so nothing bad happens if
CAN_STATE_MAX ever increases.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/iplink_can.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ip/iplink_can.c b/ip/iplink_can.c
index 5df56b2bbbb3b..2954010fefa22 100644
--- a/ip/iplink_can.c
+++ b/ip/iplink_can.c
@@ -251,7 +251,7 @@ static int can_parse_opt(struct link_util *lu, int argc, char **argv,
return 0;
}
-static const char *can_state_names[] = {
+static const char *can_state_names[CAN_STATE_MAX] = {
[CAN_STATE_ERROR_ACTIVE] = "ERROR-ACTIVE",
[CAN_STATE_ERROR_WARNING] = "ERROR-WARNING",
[CAN_STATE_ERROR_PASSIVE] = "ERROR-PASSIVE",
@@ -275,7 +275,7 @@ static void can_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
if (tb[IFLA_CAN_STATE]) {
uint32_t state = rta_getattr_u32(tb[IFLA_CAN_STATE]);
- fprintf(f, "state %s ", state <= CAN_STATE_MAX ?
+ fprintf(f, "state %s ", state < CAN_STATE_MAX ?
can_state_names[state] : "UNKNOWN");
}
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 11/51] iproute: Check mark value input
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/iproute.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ip/iproute.c b/ip/iproute.c
index 89caac124f489..5fe8a3a75d5b7 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1495,7 +1495,8 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
id = *argv;
} else if (strcmp(*argv, "mark") == 0) {
NEXT_ARG();
- get_unsigned(&mark, *argv, 0);
+ if (get_unsigned(&mark, *argv, 0))
+ invarg("invalid mark value\n", *argv);
filter.markmask = -1;
} else if (strcmp(*argv, "via") == 0) {
int family;
@@ -1712,7 +1713,8 @@ static int iproute_get(int argc, char **argv)
idev = *argv;
} else if (matches(*argv, "mark") == 0) {
NEXT_ARG();
- get_unsigned(&mark, *argv, 0);
+ if (get_unsigned(&mark, *argv, 0))
+ invarg("invalid mark value\n", *argv);
} else if (matches(*argv, "oif") == 0 ||
strcmp(*argv, "dev") == 0) {
NEXT_ARG();
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 23/51] ifstat: Fix memleak in error case
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ifstat.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/misc/ifstat.c b/misc/ifstat.c
index a853ee6d7e3b3..8fa354265a9a1 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -143,8 +143,10 @@ static int get_nlmsg_extended(const struct sockaddr_nl *who,
struct rtattr *attr;
attr = parse_rtattr_one_nested(sub_type, tb[filter_type]);
- if (attr == NULL)
+ if (attr == NULL) {
+ free(n);
return 0;
+ }
memcpy(&n->val, RTA_DATA(attr), sizeof(n->val));
}
memset(&n->rate, 0, sizeof(n->rate));
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 48/51] ifcfg: Quote left-hand side of [ ] expression
From: Phil Sutter @ 2017-08-12 12:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
This prevents word-splitting and therefore leads to more accurate error
message in case 'grep -c' prints something other than a number.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/ifcfg | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/ifcfg b/ip/ifcfg
index 083d9df36742f..30a2dc49816cd 100644
--- a/ip/ifcfg
+++ b/ip/ifcfg
@@ -131,7 +131,7 @@ noarp=$?
ip route add unreachable 224.0.0.0/24 >& /dev/null
ip route add unreachable 255.255.255.255 >& /dev/null
-if [ `ip link ls $dev | grep -c MULTICAST` -ge 1 ]; then
+if [ "`ip link ls $dev | grep -c MULTICAST`" -ge 1 ]; then
ip route add 224.0.0.0/4 dev $dev scope global >& /dev/null
fi
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 10/51] iproute: Fix for missing 'Oifs:' display
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Covscan complained about dead code but after reading it, I assume the
author's intention was to prefix the interface list with 'Oifs: '.
Initializing first to 1 and setting it to 0 after above prefix was
printed should fix it.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/iproute.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/ip/iproute.c b/ip/iproute.c
index cb695ad4141a7..89caac124f489 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -624,7 +624,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
}
if (tb[RTA_MULTIPATH]) {
struct rtnexthop *nh = RTA_DATA(tb[RTA_MULTIPATH]);
- int first = 0;
+ int first = 1;
len = RTA_PAYLOAD(tb[RTA_MULTIPATH]);
@@ -634,10 +634,12 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
if (nh->rtnh_len > len)
break;
if (r->rtm_flags&RTM_F_CLONED && r->rtm_type == RTN_MULTICAST) {
- if (first)
+ if (first) {
fprintf(fp, "Oifs: ");
- else
+ first = 0;
+ } else {
fprintf(fp, " ");
+ }
} else
fprintf(fp, "%s\tnexthop ", _SL_);
if (nh->rtnh_len > sizeof(*nh)) {
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 36/51] netem/maketable: Check return value of fstat()
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Otherwise info.st_size may contain garbage.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
netem/maketable.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/netem/maketable.c b/netem/maketable.c
index 6aff927be7040..ad660e7d457f0 100644
--- a/netem/maketable.c
+++ b/netem/maketable.c
@@ -24,8 +24,8 @@ readdoubles(FILE *fp, int *number)
int limit;
int n=0, i;
- fstat(fileno(fp), &info);
- if (info.st_size > 0) {
+ if (!fstat(fileno(fp), &info) &&
+ info.st_size > 0) {
limit = 2*info.st_size/sizeof(double); /* @@ approximate */
} else {
limit = 10000;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 47/51] examples: Some shell fixes to cbq.init
From: Phil Sutter @ 2017-08-12 12:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
This addresses the following issues:
- $@ is an array, so don't use it in quoted strings - use $* instead.
- Add missing quotes to components of [ ] expressions. These are not
strictly necessary since the output of 'wc -l' should be a single word
only, but in case of errors, bash prints "integer expression expected"
instead of "too many arguments".
- Use -print0/-0 when piping from find to xargs to allow for filenames
which contain whitespace.
- Quote arguments to 'eval' to prevent word-splitting.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
examples/cbq.init-v0.7.3 | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/examples/cbq.init-v0.7.3 b/examples/cbq.init-v0.7.3
index 1bc0d446f8983..66448d88f0053 100644
--- a/examples/cbq.init-v0.7.3
+++ b/examples/cbq.init-v0.7.3
@@ -532,7 +532,7 @@ cbq_off () {
### Prefixed message
cbq_message () {
- echo -e "**CBQ: $@"
+ echo -e "**CBQ: $*"
} # cbq_message
### Failure message
@@ -560,15 +560,15 @@ cbq_time2abs () {
### Display CBQ setup
cbq_show () {
for dev in `cbq_device_list`; do
- [ `tc qdisc show dev $dev| wc -l` -eq 0 ] && continue
+ [ "`tc qdisc show dev $dev| wc -l`" -eq 0 ] && continue
echo -e "### $dev: queueing disciplines\n"
tc $1 qdisc show dev $dev; echo
- [ `tc class show dev $dev| wc -l` -eq 0 ] && continue
+ [ "`tc class show dev $dev| wc -l`" -eq 0 ] && continue
echo -e "### $dev: traffic classes\n"
tc $1 class show dev $dev; echo
- [ `tc filter show dev $dev| wc -l` -eq 0 ] && continue
+ [ "`tc filter show dev $dev| wc -l`" -eq 0 ] && continue
echo -e "### $dev: filtering rules\n"
tc $1 filter show dev $dev; echo
done
@@ -585,7 +585,7 @@ cbq_init () {
### Gather all DEVICE fields from $1/cbq-*
DEVFIELDS=`find $1 -maxdepth 1 \( -type f -or -type l \) -name 'cbq-*' \
- -not -name '*~' | xargs sed -n 's/#.*//; \
+ -not -name '*~' -print0 | xargs -0 sed -n 's/#.*//; \
s/[[:space:]]//g; /^DEVICE=[^,]*,[^,]*\(,[^,]*\)\?/ \
{ s/.*=//; p; }'| sort -u`
[ -z "$DEVFIELDS" ] &&
@@ -593,7 +593,7 @@ cbq_init () {
### Check for different DEVICE fields for the same device
DEVICES=`echo "$DEVFIELDS"| sed 's/,.*//'| sort -u`
- [ `echo "$DEVICES"| wc -l` -ne `echo "$DEVFIELDS"| wc -l` ] &&
+ [ "`echo "$DEVICES"| wc -l`" -ne "`echo "$DEVFIELDS"| wc -l`" ] &&
cbq_failure "different DEVICE fields for single device!\n$DEVFIELDS"
} # cbq_init
@@ -618,7 +618,7 @@ cbq_load_class () {
PRIO_MARK=$PRIO_MARK_DEFAULT
PRIO_REALM=$PRIO_REALM_DEFAULT
- eval `echo "$CFILE"| grep -E "^($CBQ_WORDS)="`
+ eval "`echo "$CFILE"| grep -E "^($CBQ_WORDS)="`"
### Require RATE/WEIGHT
[ -z "$RATE" -o -z "$WEIGHT" ] &&
@@ -661,7 +661,7 @@ if [ "$1" = "compile" ]; then
### echo-only version of "tc" command
tc () {
- echo "$TC $@"
+ echo "$TC $*"
} # tc
elif [ -n "$CBQ_DEBUG" ]; then
@@ -669,13 +669,13 @@ elif [ -n "$CBQ_DEBUG" ]; then
### Logging version of "ip" command
ip () {
- echo -e "\n# ip $@" >> $CBQ_DEBUG
+ echo -e "\n# ip $*" >> $CBQ_DEBUG
$IP "$@" 2>&1 | tee -a $CBQ_DEBUG
} # ip
### Logging version of "tc" command
tc () {
- echo -e "\n# tc $@" >> $CBQ_DEBUG
+ echo -e "\n# tc $*" >> $CBQ_DEBUG
$TC "$@" 2>&1 | tee -a $CBQ_DEBUG
} # tc
else
@@ -711,8 +711,8 @@ if [ "$1" != "compile" -a "$2" != "nocache" -a -z "$CBQ_DEBUG" ]; then
### validate the cache
[ "$2" = "invalidate" -o ! -f $CBQ_CACHE ] && VALID=0
if [ $VALID -eq 1 ]; then
- [ `find $CBQ_PATH -maxdepth 1 -newer $CBQ_CACHE| \
- wc -l` -gt 0 ] && VALID=0
+ [ "`find $CBQ_PATH -maxdepth 1 -newer $CBQ_CACHE| \
+ wc -l`" -gt 0 ] && VALID=0
fi
### compile the config if the cache is invalid
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 09/51] ipntable: Make sure filter.name is NULL-terminated
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/ipntable.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ip/ipntable.c b/ip/ipntable.c
index 1837909fa42e7..30907146e85a3 100644
--- a/ip/ipntable.c
+++ b/ip/ipntable.c
@@ -631,7 +631,8 @@ static int ipntable_show(int argc, char **argv)
} else if (strcmp(*argv, "name") == 0) {
NEXT_ARG();
- strncpy(filter.name, *argv, sizeof(filter.name));
+ strncpy(filter.name, *argv, sizeof(filter.name) - 1);
+ filter.name[sizeof(filter.name) - 1] = '\0';
} else
invarg("unknown", *argv);
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 20/51] lib/inet_proto: Make sure destination buffers are NULL-terminated
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
lib/inet_proto.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/inet_proto.c b/lib/inet_proto.c
index ceda082b12a2e..87ed4769fc3da 100644
--- a/lib/inet_proto.c
+++ b/lib/inet_proto.c
@@ -35,8 +35,10 @@ const char *inet_proto_n2a(int proto, char *buf, int len)
pe = getprotobynumber(proto);
if (pe) {
icache = proto;
- strncpy(ncache, pe->p_name, 16);
- strncpy(buf, pe->p_name, len);
+ strncpy(ncache, pe->p_name, 15);
+ ncache[15] = '\0';
+ strncpy(buf, pe->p_name, len - 1);
+ buf[len] = '\0';
return buf;
}
snprintf(buf, len, "ipproto-%d", proto);
@@ -62,7 +64,8 @@ int inet_proto_a2n(const char *buf)
pe = getprotobyname(buf);
if (pe) {
icache = pe->p_proto;
- strncpy(ncache, pe->p_name, 16);
+ strncpy(ncache, pe->p_name, 15);
+ ncache[15] = '\0';
return pe->p_proto;
}
return -1;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 40/51] tc/m_xt: Fix for potential string buffer overflows
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
- Use strncpy() when writing to target->t->u.user.name and make sure the
final byte remains untouched (xtables_calloc() set it to zero).
- 'tname' length sanitization was completely wrong: If it's length
exceeded the 16 bytes available in 'k', passing a length value of 16
to strncpy() would overwrite the previously NULL'ed 'k[15]'. Also, the
sanitization has to happen if 'tname' is exactly 16 bytes long as
well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tc/m_xt.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tc/m_xt.c b/tc/m_xt.c
index ad52d239caf61..9218b14594403 100644
--- a/tc/m_xt.c
+++ b/tc/m_xt.c
@@ -95,7 +95,8 @@ build_st(struct xtables_target *target, struct xt_entry_target *t)
if (t == NULL) {
target->t = xtables_calloc(1, size);
target->t->u.target_size = size;
- strcpy(target->t->u.user.name, target->name);
+ strncpy(target->t->u.user.name, target->name,
+ sizeof(target->t->u.user.name) - 1);
target->t->u.user.revision = target->revision;
if (target->init != NULL)
@@ -277,8 +278,8 @@ static int parse_ipt(struct action_util *a, int *argc_p,
}
fprintf(stdout, " index %d\n", index);
- if (strlen(tname) > 16) {
- size = 16;
+ if (strlen(tname) >= 16) {
+ size = 15;
k[15] = 0;
} else {
size = 1 + strlen(tname);
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 03/51] ipaddress: Make buffer for filter.flushb static
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
The buffer is accessed outside of the function defining it, so make it
static.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/ipaddress.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 4d37c5e045071..3c9decb51b412 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1488,7 +1488,7 @@ static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
static int ipaddr_flush(void)
{
int round = 0;
- char flushb[4096-512];
+ static char flushb[4096-512];
filter.flushb = flushb;
filter.flushp = 0;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 00/51] Fix potential issues detected by Coverity tool
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Covscan really wasn't amused (indicated by the number of patches in this
series). Try to make it happy.
Phil Sutter (51):
devlink: Check return code of strslashrsplit()
devlink: No need for this self-assignment
ipaddress: Make buffer for filter.flushb static
ipaddress: Avoid accessing uninitialized variable lcl
iplink_can: Prevent overstepping array bounds
iplink_vrf: Complain if main table is not found
ipmaddr: Avoid accessing uninitialized data
ipntable: No need to check and assign to parms_rta
ipntable: Make sure filter.name is NULL-terminated
iproute: Fix for missing 'Oifs:' display
iproute: Check mark value input
iproute_lwtunnel: csum_mode value checking was ineffective
iproute_lwtunnel: Argument to strerror must be positive
ipvrf: Don't try to close an invalid fd
ipvrf: Fix error path of vrf_switch()
xfrm_state: Make sure alg_name is NULL-terminated
lib/bpf: Don't leak fp in bpf_find_mntpt()
lib/fs: Fix format string in find_fs_mount()
lib/fs: Fix and simplify make_path()
lib/inet_proto: Make sure destination buffers are NULL-terminated
lib/libnetlink: Don't pass NULL parameter to memcpy()
lib/rt_names: Drop dead code in rtnl_rttable_n2a()
ifstat: Fix memleak in error case
ifstat, nstat: Check fdopen() return value
ifstat: Fix memleak in dump_kern_db() for json output
lnstat_util: Simplify alloc_and_open() a bit
nstat: Fix for potential NULL pointer dereference
nstat: Avoid passing negative fd to fdopen()
ss: Use C99 initializer in netlink_show_one()
ss: Skip useless check in parse_hostcond()
ss: Drop useless assignment
ss: Make sure index variable is >= 0
ss: Don't leak fd in tcp_show_netlink_file()
ss: Make sure scanned index value to unix_state_map is sane
ss: Fix potential memleak in unix_stats_print()
netem/maketable: Check return value of fstat()
netem/maketable: Check return value of fscanf()
tc/em_ipset: Don't leak sockfd on error path
tc/m_gact: Drop dead code
tc/m_xt: Fix for potential string buffer overflows
tc/q_multiq: Don't pass garbage in TCA_OPTIONS
tc/q_netem: Don't dereference possibly NULL pointer
tc/tc_filter: Make sure filter name is not empty
tipc/bearer: Fix resource leak in error path
tipc/bearer: Prevent NULL pointer dereference
tipc/node: Fix socket fd check in cmd_node_get_addr()
examples: Some shell fixes to cbq.init
ifcfg: Quote left-hand side of [ ] expression
lib/ll_map: Make sure im->name is NULL-terminated
Check user supplied interface name lengths
lib/bpf: Check return value of write()
devlink/devlink.c | 18 ++++++++++-----
examples/cbq.init-v0.7.3 | 24 ++++++++++----------
include/utils.h | 1 +
ip/ifcfg | 2 +-
ip/ip6tunnel.c | 6 +++--
ip/ipaddress.c | 4 ++--
ip/ipl2tp.c | 1 +
ip/iplink.c | 27 +++++++----------------
ip/iplink_can.c | 4 ++--
ip/iplink_vrf.c | 5 ++++-
ip/ipmaddr.c | 3 ++-
ip/ipntable.c | 5 ++---
ip/iproute.c | 14 +++++++-----
ip/iproute_lwtunnel.c | 9 ++++----
ip/iprule.c | 4 ++++
ip/iptunnel.c | 12 ++++++----
ip/iptuntap.c | 4 +++-
ip/ipvrf.c | 16 ++++++++------
ip/xfrm_state.c | 3 ++-
lib/bpf.c | 8 +++++--
lib/fs.c | 22 +++++--------------
lib/inet_proto.c | 9 +++++---
lib/libnetlink.c | 6 +++--
lib/ll_map.c | 4 ++--
lib/rt_names.c | 4 ----
lib/utils.c | 8 +++++++
misc/arpd.c | 1 +
misc/ifstat.c | 28 +++++++++++++++++-------
misc/lnstat_util.c | 7 ++----
misc/nstat.c | 33 +++++++++++++++++++---------
misc/ss.c | 57 +++++++++++++++++++++++++++++-------------------
netem/maketable.c | 8 +++----
tc/em_ipset.c | 1 +
tc/m_gact.c | 14 +++---------
tc/m_xt.c | 7 +++---
tc/q_multiq.c | 2 +-
tc/q_netem.c | 4 +++-
tc/tc_filter.c | 3 +++
tipc/bearer.c | 7 ++++--
tipc/node.c | 3 ++-
40 files changed, 230 insertions(+), 168 deletions(-)
--
2.13.1
^ permalink raw reply
* [iproute PATCH 28/51] nstat: Avoid passing negative fd to fdopen()
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Introduce a wrapper which does the sanity checking and returns NULL
in case fd is invalid.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/nstat.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/misc/nstat.c b/misc/nstat.c
index 23e1569d7872b..c1e7ddec271e2 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -252,9 +252,16 @@ static void load_ugly_table(FILE *fp)
}
}
+static FILE *fdopen_null(int fd, const char *mode)
+{
+ if (fd < 0)
+ return NULL;
+ return fdopen(fd, mode);
+}
+
static void load_sctp_snmp(void)
{
- FILE *fp = fdopen(net_sctp_snmp_open(), "r");
+ FILE *fp = fdopen_null(net_sctp_snmp_open(), "r");
if (fp) {
load_good_table(fp);
@@ -264,7 +271,7 @@ static void load_sctp_snmp(void)
static void load_snmp(void)
{
- FILE *fp = fdopen(net_snmp_open(), "r");
+ FILE *fp = fdopen_null(net_snmp_open(), "r");
if (fp) {
load_ugly_table(fp);
@@ -274,7 +281,7 @@ static void load_snmp(void)
static void load_snmp6(void)
{
- FILE *fp = fdopen(net_snmp6_open(), "r");
+ FILE *fp = fdopen_null(net_snmp6_open(), "r");
if (fp) {
load_good_table(fp);
@@ -284,7 +291,7 @@ static void load_snmp6(void)
static void load_netstat(void)
{
- FILE *fp = fdopen(net_netstat_open(), "r");
+ FILE *fp = fdopen_null(net_netstat_open(), "r");
if (fp) {
load_ugly_table(fp);
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 34/51] ss: Make sure scanned index value to unix_state_map is sane
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ss.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/misc/ss.c b/misc/ss.c
index cda5e3b6a2d6f..667b8faad6528 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3150,7 +3150,8 @@ static int unix_show(struct filter *f)
if (flags & (1 << 16)) {
u->state = SS_LISTEN;
- } else {
+ } else if (u->state > 0 &&
+ u->state <= ARRAY_SIZE(unix_state_map)) {
u->state = unix_state_map[u->state-1];
if (u->type == SOCK_DGRAM && u->state == SS_CLOSE && u->rport)
u->state = SS_ESTABLISHED;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 41/51] tc/q_multiq: Don't pass garbage in TCA_OPTIONS
From: Phil Sutter @ 2017-08-12 12:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
multiq_parse_opt() doesn't change 'opt' at all. So at least make sure
it doesn't fill TCA_OPTIONS attribute with garbage from stack.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tc/q_multiq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/q_multiq.c b/tc/q_multiq.c
index 7823931494563..9c09c9a7748f6 100644
--- a/tc/q_multiq.c
+++ b/tc/q_multiq.c
@@ -43,7 +43,7 @@ static void explain(void)
static int multiq_parse_opt(struct qdisc_util *qu, int argc, char **argv,
struct nlmsghdr *n)
{
- struct tc_multiq_qopt opt;
+ struct tc_multiq_qopt opt = {};
if (argc) {
if (strcmp(*argv, "help") == 0) {
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 30/51] ss: Skip useless check in parse_hostcond()
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
The passed 'addr' parameter is dereferenced by caller before and in
parse_hostcond() multiple times before this check, so assume it is
always true.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/misc/ss.c b/misc/ss.c
index b4f89c85c2d52..5ea388fbf1c1a 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1671,7 +1671,7 @@ void *parse_hostcond(char *addr, bool is_port)
}
}
}
- if (!is_port && addr && *addr && *addr != '*') {
+ if (!is_port && *addr && *addr != '*') {
if (get_prefix_1(&a.addr, addr, fam)) {
if (get_dns_host(&a, addr, fam)) {
fprintf(stderr, "Error: an inet prefix is expected rather than \"%s\".\n", addr);
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 39/51] tc/m_gact: Drop dead code
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
The use of 'ok' variable in parse_gact() is ineffective: The second
conditional increments it either if *argv is 'gact' or if
parse_action_control() doesn't fail (in which case exit() is called).
So this is effectively an unconditional increment and since no decrement
happens anywhere, all remaining checks for 'ok != 0' can be dropped.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tc/m_gact.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/tc/m_gact.c b/tc/m_gact.c
index 1a2583372c34e..df143c9e0953e 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -76,7 +76,6 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p,
{
int argc = *argc_p;
char **argv = *argv_p;
- int ok = 0;
struct tc_gact p = { 0 };
#ifdef CONFIG_GACT_PROB
int rd = 0;
@@ -89,17 +88,14 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p,
if (matches(*argv, "gact") == 0) {
- ok++;
argc--;
argv++;
- } else {
- if (parse_action_control(&argc, &argv, &p.action, false) == -1)
- usage();
- ok++;
+ } else if (parse_action_control(&argc, &argv, &p.action, false) == -1) {
+ usage(); /* does not return */
}
#ifdef CONFIG_GACT_PROB
- if (ok && argc > 0) {
+ if (argc > 0) {
if (matches(*argv, "random") == 0) {
rd = 1;
NEXT_ARG();
@@ -142,15 +138,11 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p,
}
argc--;
argv++;
- ok++;
} else if (matches(*argv, "help") == 0) {
usage();
}
}
- if (!ok)
- return -1;
-
tail = NLMSG_TAIL(n);
addattr_l(n, MAX_MSG, tca_id, NULL, 0);
addattr_l(n, MAX_MSG, TCA_GACT_PARMS, &p, sizeof(p));
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 33/51] ss: Don't leak fd in tcp_show_netlink_file()
From: Phil Sutter @ 2017-08-12 12:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170812120510.28750-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ss.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index 4d2f75b571ea6..cda5e3b6a2d6f 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2687,41 +2687,44 @@ static int tcp_show_netlink_file(struct filter *f)
{
FILE *fp;
char buf[16384];
+ int err = -1;
if ((fp = fopen(getenv("TCPDIAG_FILE"), "r")) == NULL) {
perror("fopen($TCPDIAG_FILE)");
- return -1;
+ return err;
}
while (1) {
- int status, err;
+ int status, err2;
struct nlmsghdr *h = (struct nlmsghdr *)buf;
struct sockstat s = {};
status = fread(buf, 1, sizeof(*h), fp);
if (status < 0) {
perror("Reading header from $TCPDIAG_FILE");
- return -1;
+ break;
}
if (status != sizeof(*h)) {
perror("Unexpected EOF reading $TCPDIAG_FILE");
- return -1;
+ break;
}
status = fread(h+1, 1, NLMSG_ALIGN(h->nlmsg_len-sizeof(*h)), fp);
if (status < 0) {
perror("Reading $TCPDIAG_FILE");
- return -1;
+ break;
}
if (status + sizeof(*h) < h->nlmsg_len) {
perror("Unexpected EOF reading $TCPDIAG_FILE");
- return -1;
+ break;
}
/* The only legal exit point */
- if (h->nlmsg_type == NLMSG_DONE)
- return 0;
+ if (h->nlmsg_type == NLMSG_DONE) {
+ err = 0;
+ break;
+ }
if (h->nlmsg_type == NLMSG_ERROR) {
struct nlmsgerr *err = (struct nlmsgerr *)NLMSG_DATA(h);
@@ -2732,7 +2735,7 @@ static int tcp_show_netlink_file(struct filter *f)
errno = -err->error;
perror("TCPDIAG answered");
}
- return -1;
+ break;
}
parse_diag_msg(h, &s);
@@ -2741,10 +2744,15 @@ static int tcp_show_netlink_file(struct filter *f)
if (f && f->f && run_ssfilter(f->f, &s) == 0)
continue;
- err = inet_show_sock(h, &s);
- if (err < 0)
- return err;
+ err2 = inet_show_sock(h, &s);
+ if (err2 < 0) {
+ err = err2;
+ break;
+ }
}
+
+ fclose(fp);
+ return err;
}
static int tcp_show(struct filter *f, int socktype)
--
2.13.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox