Re: [iproute PATCH v3 4/5] tc/tc_filter: Make sure filter name is not empty

2017-08-21 Thread Phil Sutter
On Mon, Aug 21, 2017 at 02:53:46PM +, David Laight wrote: > From: Phil Sutter > > Sent: 21 August 2017 11:03 > > To: Stephen Hemminger > > Cc: netdev@vger.kernel.org > > Subject: [iproute PATCH v3 4/5] tc/tc_filter: Make sure filter name is not > > empty

[iproute PATCH v2 0/7] Covscan: Misc fixes

2017-08-21 Thread Phil Sutter
This series collects patches from v1 addressing miscellaneous issues detected by covscan. No changes to the actual patches, just splitting into smaller series. Phil Sutter (7): nstat: Avoid passing negative fd to fdopen() ss: Make sure index variable is >= 0 ss: Make sure scanned in

[iproute PATCH v2 6/7] lib/fs: Fix and simplify make_path()

2017-08-21 Thread Phil Sutter
Calling stat() before mkdir() is racey: The entry might change in between. Also, the call to stat() seems to exist only to check if the directory exists already. So simply call mkdir() unconditionally and catch only errors other than EEXIST. Signed-off-by: Phil Sutter <p...@nwl.cc> --- li

[iproute PATCH v2 2/7] ss: Make sure index variable is >= 0

2017-08-21 Thread Phil Sutter
This shouldn't happen but relying upon external data without checking may lead to unexpected results. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ss.c b/misc/ss.c index 10360e5a04ff8..1ee02d73b2d7f

[iproute PATCH v2 3/7] ss: Make sure scanned index value to unix_state_map is sane

2017-08-21 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/misc/ss.c b/misc/ss.c index 1ee02d73b2d7f..6c091a694231e 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -3151,7 +3151,8 @@ static int unix_show(struct fil

[iproute PATCH v2 7/7] lib/libnetlink: Don't pass NULL parameter to memcpy()

2017-08-21 Thread Phil Sutter
Both addattr_l() and rta_addattr_l() may be called with NULL data pointer and 0 alen parameters. Avoid calling memcpy() in that case. Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/libnetlink.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/libnetlink.c

[iproute PATCH v2 1/7] nstat: Avoid passing negative fd to fdopen()

2017-08-21 Thread Phil Sutter
Introduce a wrapper which does the sanity checking and returns NULL in case fd is invalid. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/nstat.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/misc/nstat.c b/misc/nstat.c index 1212b1f2c8128..7cdde75

[iproute PATCH v2 4/7] netem/maketable: Check return value of fscanf()

2017-08-21 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@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..517f1dc461e8a 100644 --- a/netem/maketable.c +++ b/netem/maketable.c @@ -38,8 +38,8 @@ readdouble

[iproute PATCH v2 5/7] lib/bpf: Check return value of write()

2017-08-21 Thread Phil Sutter
This is merely to silence the compiler warning. If write to stderr failed, assume that printing an error message will fail as well so don't even try. Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/bpf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/bpf.c b/lib

[iproute PATCH v2 2/3] iplink_vrf: Complain if main table is not found

2017-08-21 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> Acked-by: David Ahern <dsah...@gmail.com> --- Changes since v1: - Remove double newline addon. - Added David's ACK from v1 review. --- ip/iplink_vrf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ip/iplink_vrf.c b/ip/

[iproute PATCH v2 3/3] devlink: Check return code of strslashrsplit()

2017-08-21 Thread Phil Sutter
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 <p...@nwl.cc> --- devlink/dev

[iproute PATCH v2 0/3] Covscan: Fix for missing error checking

2017-08-21 Thread Phil Sutter
This series collects patches from v1 dealing with spots where error checking is necessary or recommended. Minor changes to patches 1 and 2, patch 3 remains unchanged. Phil Sutter (3): iproute: Check mark value input iplink_vrf: Complain if main table is not found devlink: Check return code

[iproute PATCH v2 1/3] iproute: Check mark value input

2017-08-21 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- Changes since v1: - Drop newline from end of error message, invarg() already does that. --- ip/iproute.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ip/iproute.c b/ip/iproute.c index cb695ad4141a7..5936e2a978bc7

[iproute PATCH v2] lib/bpf: Don't leak fp in bpf_find_mntpt()

2017-08-21 Thread Phil Sutter
If fopen() succeeded but len != PATH_MAX, the function leaks the open FILE pointer. Fix this by checking len value before calling fopen(). Signed-off-by: Phil Sutter <p...@nwl.cc> Acked-by: Daniel Borkmann <dan...@iogearbox.net> --- No change since v1, just resubmitting - I f

[iproute PATCH v3 3/7] lib/fs: Fix format string in find_fs_mount()

2017-08-21 Thread Phil Sutter
A field width of 4096 allows fscanf() to store that amount of characters into the given buffer, though that doesn't include the terminating NULL byte. Decrease the value by one to leave space for it. Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/fs.c | 2 +- 1 file changed, 1 insertion

[iproute PATCH v3 4/7] lib/inet_proto: Review inet_proto_{a2n,n2a}()

2017-08-21 Thread Phil Sutter
in get_u8() to find out whether passed 'buf' contains a valid decimal number instead of checking the first character's value manually. Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/inet_proto.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git

[iproute PATCH v3 1/7] ipntable: Avoid memory allocation for filter.name

2017-08-21 Thread Phil Sutter
The original issue was that filter.name might end up unterminated if user provided string was too long. But in fact it is not necessary to copy the commandline parameter at all: just make filter.name point to it instead. Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/ipntable.c | 6 +++

[iproute PATCH v3 5/7] lnstat_util: Simplify alloc_and_open() a bit

2017-08-21 Thread Phil Sutter
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 <p...@nwl.cc> --- misc/lnstat_util.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions

[iproute PATCH v3 2/7] xfrm_state: Make sure alg_name is NULL-terminated

2017-08-21 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/xfrm_state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c index e11c93bf1c3b5..7c0389038986e 100644 --- a/ip/xfrm_state.c +++ b/ip/xfrm_state.c @@ -125,7 +125,8 @@ static int xfrm_algo

[iproute PATCH v3 0/7] Covscan: Fixes for string termination

2017-08-21 Thread Phil Sutter
upstream. Changes since v2: - Rebased onto current upstream master branch. - Replaced patches 1, 4 and 7 by more appropriate ones given feedback from v2 review. Phil Sutter (7): ipntable: Avoid memory allocation for filter.name xfrm_state: Make sure alg_name is NULL-terminated lib/fs: Fix format

[iproute PATCH v3 6/7] tc/m_xt: Fix for potential string buffer overflows

2017-08-21 Thread Phil Sutter
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 <p...@nwl.cc> --- tc/m_xt.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tc/m_xt.c b/tc/

[iproute PATCH v3 7/7] lib/ll_map: Choose size of new cache items at run-time

2017-08-21 Thread Phil Sutter
Instead of having a fixed buffer of 16 bytes for the interface name, tailor size of new ll_cache entry using the interface name's actual length. This also makes sure the following call to strcpy() is safe. Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/ll_map.c | 4 ++-- 1 file chan

[iproute PATCH v3 2/5] nstat: Fix for potential NULL pointer dereference

2017-08-21 Thread Phil Sutter
If the string at 'p' contains neither space not newline, 'p' will become NULL. Make sure this isn't the case before dereferencing it. Signed-off-by: Phil Sutter <p...@nwl.cc> --- Changes since v2: - Call abort() if 'p' becomes NULL. --- misc/nstat.c | 2 ++ 1 file changed, 2 insertions(+)

[iproute PATCH v3 1/5] ifstat, nstat: Check fdopen() return value

2017-08-21 Thread Phil Sutter
Prevent passing NULL FILE pointer to fgets() later. Fix both tools in a single patch since the code changes are basically identical. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ifstat.c | 16 +++- misc/nstat.c | 16 +++- 2 files changed, 22 insertions(

[iproute PATCH v3 0/5] Covscan: Fix potential NULL pointer dereferences

2017-08-21 Thread Phil Sutter
This series collects patches from v1 which eliminate possible cases of NULL pointer dereferences. Changes since v2: - Rebased onto current master branch. - Adjusted patches according to feedback. Phil Sutter (5): ifstat, nstat: Check fdopen() return value nstat: Fix for potential NULL

[iproute PATCH v3 4/5] tc/tc_filter: Make sure filter name is not empty

2017-08-21 Thread Phil Sutter
The later check for 'k[0] != 0' requires a non-empty filter name, otherwise NULL pointer dereference in 'q' might happen. Signed-off-by: Phil Sutter <p...@nwl.cc> --- Changes since v2: - Instead of calling strlen(), just make sure **argv is not 0. --- tc/tc_filter.c | 3 +++ 1 file chan

[iproute PATCH v3 5/5] tipc/bearer: Prevent NULL pointer dereference

2017-08-21 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- Changes since v2: - Keep assignment and check in separate statements. --- tipc/bearer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tipc/bearer.c b/tipc/bearer.c index c3d4491f8f6ef..0d84570150624 100644 --- a/tipc/be

[iproute PATCH v3 3/5] tc/q_netem: Don't dereference possibly NULL pointer

2017-08-21 Thread Phil Sutter
Assuming 'opt' might be NULL, move the call to RTA_PAYLOAD to after the check since it dereferences its parameter. Signed-off-by: Phil Sutter <p...@nwl.cc> --- Changes since v2: - Dropped empty line between assignment and check. --- tc/q_netem.c | 3 ++- 1 file changed, 2 insertions

[iproute PATCH v3 1/6] ipaddress: Avoid accessing uninitialized variable lcl

2017-08-21 Thread Phil Sutter
If no address was given, ipaddr_modify() accesses uninitialized data when assigning to req.ifa.ifa_prefixlen. Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/ipaddress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ip/ipaddress.c b/ip/ipaddress.c index 4d37c5e

[iproute PATCH v3 5/6] netem/maketable: Check return value of fstat()

2017-08-21 Thread Phil Sutter
Otherwise info.st_size may contain garbage. Signed-off-by: Phil Sutter <p...@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

[iproute PATCH v3 6/6] tc/q_multiq: Don't pass garbage in TCA_OPTIONS

2017-08-21 Thread Phil Sutter
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 <p...@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

[iproute PATCH v3 2/6] iplink_can: Prevent overstepping array bounds

2017-08-21 Thread Phil Sutter
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 <p...@nwl.cc> -

[iproute PATCH v3 4/6] ss: Use C99 initializer in netlink_show_one()

2017-08-21 Thread Phil Sutter
This has the additional benefit of initializing st.ino to zero which is used later in is_sctp_assoc() function. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ss.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/misc/ss.c b/misc/ss.c index 10360e5

[iproute PATCH v3 3/6] ipmaddr: Avoid accessing uninitialized data

2017-08-21 Thread Phil Sutter
Looks like this can only happen if /proc/net/igmp is malformed, but better be sure. Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/ipmaddr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c index 4f726fdd976f1..85a69e779563d 100644 --

[iproute PATCH v3 0/6] Covscan: Don't access garbage

2017-08-21 Thread Phil Sutter
This series collects patches from v1 which resolve situations where garbage might be read, either due to missing initialization of variables or accessing data which went out of scope. Changes since v2: - Rebased onto current master branch. - Dropped first patch since it is not a real issue. Phil

Re: [iproute PATCH v2 4/5] tc/tc_filter: Make sure filter name is not empty

2017-08-18 Thread Phil Sutter
On Fri, Aug 18, 2017 at 04:34:44PM +, David Laight wrote: > From: Phil Sutter > > Sent: 18 August 2017 12:16 > > On Fri, Aug 18, 2017 at 09:30:35AM +, David Laight wrote: > > > From: Phil Sutter > > > > Sent: 17 August 2017 18:10 > > > > The

Re: [iproute PATCH v2 4/7] lib/inet_proto: Make sure destination buffers are NULL-terminated

2017-08-18 Thread Phil Sutter
On Fri, Aug 18, 2017 at 09:37:33AM -0700, Stephen Hemminger wrote: > On Thu, 17 Aug 2017 19:09:29 +0200 > Phil Sutter <p...@nwl.cc> wrote: > > > Signed-off-by: Phil Sutter <p...@nwl.cc> > > --- > > lib/inet_proto.c | 9 ++--- > > 1 file changed,

Re: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated

2017-08-18 Thread Phil Sutter
On Fri, Aug 18, 2017 at 04:32:47PM +, David Laight wrote: > From: Phil Sutter > > Sent: 18 August 2017 11:52 > > On Fri, Aug 18, 2017 at 09:19:16AM +, David Laight wrote: > > > From: Phil Sutter > > > > Sent: 17 August 2017 18:09 > > &g

Re: [iproute PATCH v2 1/7] ipaddress: Make buffer for filter.flushb static

2017-08-18 Thread Phil Sutter
On Thu, Aug 17, 2017 at 07:09:25PM +0200, Phil Sutter wrote: > The buffer is accessed outside of the function defining it, so make it > static. > > Signed-off-by: Phil Sutter <p...@nwl.cc> Self-NACK: Access to flushb should be sane since all accessors are called from ipadd

Re: [iproute PATCH v2 2/2] ifcfg: Quote left-hand side of [ ] expression

2017-08-18 Thread Phil Sutter
On Fri, Aug 18, 2017 at 09:32:52AM +, David Laight wrote: > From: Phil Sutter > > Sent: 17 August 2017 18:10 > > This prevents word-splitting and therefore leads to more accurate error > > message in case 'grep -c' prints something other than a number. > > > &

Re: [iproute PATCH v2 2/3] iproute_lwtunnel: Argument to strerror must be positive

2017-08-18 Thread Phil Sutter
On Fri, Aug 18, 2017 at 09:21:34AM +, David Laight wrote: > From: Phil Sutter > > Sent: 17 August 2017 18:10 > > Signed-off-by: Phil Sutter <p...@nwl.cc> > > --- > > ip/iproute_lwtunnel.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > &

Re: [iproute PATCH v2 4/5] tc/tc_filter: Make sure filter name is not empty

2017-08-18 Thread Phil Sutter
On Fri, Aug 18, 2017 at 09:30:35AM +, David Laight wrote: > From: Phil Sutter > > Sent: 17 August 2017 18:10 > > The later check for 'k[0] != 0' requires a non-empty filter name, > > otherwise NULL pointer dereference in 'q' might happen. > > > > Signed

Re: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated

2017-08-18 Thread Phil Sutter
Hi David, On Fri, Aug 18, 2017 at 09:19:16AM +, David Laight wrote: > From: Phil Sutter > > Sent: 17 August 2017 18:09 > > To: Stephen Hemminger > > Cc: netdev@vger.kernel.org > > Subject: [iproute PATCH v2 1/7] ipntable: Make sure filter.name is > > NU

Re: [iproute PATCH v2 1/7] devlink: No need for this self-assignment

2017-08-18 Thread Phil Sutter
On Thu, Aug 17, 2017 at 09:48:50PM +0200, Jiri Pirko wrote: > Thu, Aug 17, 2017 at 07:09:25PM CEST, p...@nwl.cc wrote: > >dl_argv_handle_both() will either assign to handle_bit or error out in > >which case the variable is not used by the caller. > > I'm pretty sure that I did this to silence the

[iproute PATCH v2 0/2] Covscan: Shell script fixes

2017-08-17 Thread Phil Sutter
This series collects patches from v1 which deal with programming mistakes in shell scripts. No changes to the actual patches, just splitting into smaller series. Phil Sutter (2): examples: Some shell fixes to cbq.init ifcfg: Quote left-hand side of [ ] expression examples/cbq.init-v0.7.3

[iproute PATCH v2 4/7] ipmaddr: Avoid accessing uninitialized data

2017-08-17 Thread Phil Sutter
Looks like this can only happen if /proc/net/igmp is malformed, but better be sure. Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/ipmaddr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c index 4f726fdd976f1..85a69e779563d 100644 --

[iproute PATCH v2 0/5] Covscan: Fix potential memory leaks

2017-08-17 Thread Phil Sutter
This series collects patches from v1 which deal with potential memory leaks. No changes to the actual patches, just splitting into smaller series. Phil Sutter (5): ipvrf: Fix error path of vrf_switch() ifstat: Fix memleak in error case ifstat: Fix memleak in dump_kern_db() for json output

[iproute PATCH v2 3/7] iplink_can: Prevent overstepping array bounds

2017-08-17 Thread Phil Sutter
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 <p...@nwl.cc> -

[iproute PATCH v2 3/5] ifstat: Fix memleak in dump_kern_db() for json output

2017-08-17 Thread Phil Sutter
Looks like this was forgotten when converting to common json output formatter. Fixes: fcc16c2287bf8 ("provide common json output formatter") Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ifstat.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/misc

[iproute PATCH v2 0/7] Covscan: Dead code elimination

2017-08-17 Thread Phil Sutter
This series collects patches from v1 which deal with dead code, either by removing it or changing context so it is accessed again if that makes sense. No changes to the actual patches, just splitting into smaller series. Phil Sutter (7): devlink: No need for this self-assignment ipntable

[iproute PATCH v2 2/2] ifcfg: Quote left-hand side of [ ] expression

2017-08-17 Thread Phil Sutter
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 <p...@nwl.cc> --- ip/ifcfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ip/ifcfg b/ip/ifcfg

[iproute PATCH v2 2/7] xfrm_state: Make sure alg_name is NULL-terminated

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/xfrm_state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c index e11c93bf1c3b5..7c0389038986e 100644 --- a/ip/xfrm_state.c +++ b/ip/xfrm_state.c @@ -125,7 +125,8 @@ static int xfrm_algo

[iproute PATCH v2 1/3] iproute_lwtunnel: csum_mode value checking was ineffective

2017-08-17 Thread Phil Sutter
ila_csum_name2mode() returning -1 on error but being declared as returning __u8 doesn't make much sense. Change the code to correctly detect this issue. Checking for __u8 overruns shouldn't be necessary though since ila_csum_name2mode() return values are well-defined. Signed-off-by: Phil Sutter

[iproute PATCH v2 0/5] Covscan: Fix potential NULL pointer dereferences

2017-08-17 Thread Phil Sutter
This series collects patches from v1 which eliminate possible cases of NULL pointer dereferences. No changes to the actual patches, just splitting into smaller series. Phil Sutter (5): ifstat, nstat: Check fdopen() return value nstat: Fix for potential NULL pointer dereference tc/q_netem

[iproute PATCH v2 5/7] lnstat_util: Simplify alloc_and_open() a bit

2017-08-17 Thread Phil Sutter
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 <p...@nwl.cc> --- misc/lnstat_util.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions

[iproute PATCH v2 4/7] lib/rt_names: Drop dead code in rtnl_rttable_n2a()

2017-08-17 Thread Phil Sutter
Since 'id' is 32bit unsigned, it can never exceed RT_TABLE_MAX (which is defined to 0x). Therefore drop that never matching conditional. Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/rt_names.c | 4 1 file changed, 4 deletions(-) diff --git a/lib/rt_names.c b/lib/rt_n

[iproute PATCH v2 2/5] nstat: Fix for potential NULL pointer dereference

2017-08-17 Thread Phil Sutter
If the string at 'p' contains neither space not newline, 'p' will become NULL. Make sure this isn't the case before dereferencing it. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/nstat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/nstat.c b/misc/nstat.c

[iproute PATCH v2 4/7] lib/inet_proto: Make sure destination buffers are NULL-terminated

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@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 @@ cons

[iproute PATCH v2 3/3] tipc/node: Fix socket fd check in cmd_node_get_addr()

2017-08-17 Thread Phil Sutter
socket() returns -1 on error, not 0. Signed-off-by: Phil Sutter <p...@nwl.cc> --- tipc/node.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tipc/node.c b/tipc/node.c index 201fe1a4df3bd..fe085aec9b4ac 100644 --- a/tipc/node.c +++ b/tipc/node.c @@ -109,7 +109,8 @@

[iproute PATCH v2 4/5] tc/tc_filter: Make sure filter name is not empty

2017-08-17 Thread Phil Sutter
The later check for 'k[0] != 0' requires a non-empty filter name, otherwise NULL pointer dereference in 'q' might happen. Signed-off-by: Phil Sutter <p...@nwl.cc> --- tc/tc_filter.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tc/tc_filter.c b/tc/tc_filter.c index b13fb91

[iproute PATCH v2 7/7] lib/ll_map: Make sure im->name is NULL-terminated

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/ll_map.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ll_map.c b/lib/ll_map.c index 4e4556c9ac80b..4d06eb69f138a 100644 --- a/lib/ll_map.c +++ b/lib/ll_map.c @@ -120,11 +120,11 @@ int ll_remember_index(const

[iproute PATCH v2 0/3] Covscan: Fixes for obvious programming mistakes

2017-08-17 Thread Phil Sutter
This series collects those patches from v1 which are clear programming flaws. No changes to the actual patches, just splitting into smaller series. Phil Sutter (3): iproute_lwtunnel: csum_mode value checking was ineffective iproute_lwtunnel: Argument to strerror must be positive tipc/node

[iproute PATCH v2 0/7] Covscan: Don't access garbage

2017-08-17 Thread Phil Sutter
This series collects patches from v1 which resolve situations where garbage might be read, either due to missing initialization of variables or accessing data which went out of scope. No changes to the actual patches, just splitting into smaller series. Phil Sutter (7): ipaddress: Make buffer

[iproute PATCH v2 1/5] ipvrf: Fix error path of vrf_switch()

2017-08-17 Thread Phil Sutter
Apart from trying to close(-1), this also leaked memory. Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/ipvrf.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ip/ipvrf.c b/ip/ipvrf.c index 0094cf8557cd7..e6fad32abd956 100644 --- a/ip/ipvrf.c +++ b/ip/i

[iproute PATCH v2 2/7] ipaddress: Avoid accessing uninitialized variable lcl

2017-08-17 Thread Phil Sutter
If no address was given, ipaddr_modify() accesses uninitialized data when assigning to req.ifa.ifa_prefixlen. Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/ipaddress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ip/ipaddress.c b/ip/ipaddress.c index 3c9decb

[iproute PATCH v2 1/2] examples: Some shell fixes to cbq.init

2017-08-17 Thread Phil Sutter
"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 <p...@nwl.cc> --- exa

[iproute PATCH v2 2/7] ipntable: No need to check and assign to parms_rta

2017-08-17 Thread Phil Sutter
This variable is initialized at declaration and nowhere else does any assignment to it happen, so just drop the check. Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/ipntable.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/ip/ipntable.c b/ip/ipntable.c index 7be1f04d33d90..3090714

[iproute PATCH v2 0/7] Covscan: Fixes for string termination

2017-08-17 Thread Phil Sutter
upstream. No changes to the actual patches, just splitting into smaller series. Phil Sutter (7): ipntable: Make sure filter.name is NULL-terminated xfrm_state: Make sure alg_name is NULL-terminated lib/fs: Fix format string in find_fs_mount() lib/inet_proto: Make sure destination buffers are NULL

[iproute PATCH v2 7/7] tc/m_gact: Drop dead code

2017-08-17 Thread Phil Sutter
remaining checks for 'ok != 0' can be dropped. Signed-off-by: Phil Sutter <p...@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 @@

[iproute PATCH v2 1/7] ipaddress: Make buffer for filter.flushb static

2017-08-17 Thread Phil Sutter
The buffer is accessed outside of the function defining it, so make it static. Signed-off-by: Phil Sutter <p...@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 --

[iproute PATCH v2 5/7] ss: Skip useless check in parse_hostcond()

2017-08-17 Thread Phil Sutter
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 <p...@nwl.cc> --- misc/ss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ss.c b/mis

[iproute PATCH v2 3/7] lib/fs: Fix format string in find_fs_mount()

2017-08-17 Thread Phil Sutter
A field width of 4096 allows fscanf() to store that amount of characters into the given buffer, though that doesn't include the terminating NULL byte. Decrease the value by one to leave space for it. Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/fs.c | 2 +- 1 file changed, 1 insertion

[iproute PATCH v2 7/7] tc/q_multiq: Don't pass garbage in TCA_OPTIONS

2017-08-17 Thread Phil Sutter
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 <p...@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

[iproute PATCH v2 1/7] ipntable: Make sure filter.name is NULL-terminated

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- ip/ipntable.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ip/ipntable.c b/ip/ipntable.c index 879626ee4f491..7be1f04d33d90 100644 --- a/ip/ipntable.c +++ b/ip/ipntable.c @@ -633,7 +633,8 @@ static int ipntable_show(in

[iproute PATCH v2 1/2] ss: Don't leak fd in tcp_show_netlink_file()

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ss.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/misc/ss.c b/misc/ss.c index d767b1103ea81..07eecfa7a36db 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -2687,41 +2687,44 @@ stat

[iproute PATCH v2 2/3] iproute_lwtunnel: Argument to strerror must be positive

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@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

[iproute PATCH v2 1/7] devlink: No need for this self-assignment

2017-08-17 Thread Phil Sutter
dl_argv_handle_both() will either assign to handle_bit or error out in which case the variable is not used by the caller. Signed-off-by: Phil Sutter <p...@nwl.cc> --- devlink/devlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlink/devlink.c b/devlink/devlink.c

[iproute PATCH v2 5/5] tipc/bearer: Fix resource leak in error path

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- tipc/bearer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tipc/bearer.c b/tipc/bearer.c index 810344f672af1..c3d4491f8f6ef 100644 --- a/tipc/bearer.c +++ b/tipc/bearer.c @@ -163,6 +163,7 @@ static int nl_add_udp_enable_opts(struct nl

[iproute PATCH v2 3/5] tc/q_netem: Don't dereference possibly NULL pointer

2017-08-17 Thread Phil Sutter
Assuming 'opt' might be NULL, move the call to RTA_PAYLOAD to after the check since it dereferences its parameter. Signed-off-by: Phil Sutter <p...@nwl.cc> --- tc/q_netem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tc/q_netem.c b/tc/q_netem.c index 0975ae1

[iproute PATCH v2 2/5] ifstat: Fix memleak in error case

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@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_ex

[iproute PATCH v2 4/5] ss: Fix potential memleak in unix_stats_print()

2017-08-17 Thread Phil Sutter
Fixes: 2d0e538f3e1cd ("ss: Drop list traversal from unix_stats_print()") Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ss.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/misc/ss.c b/misc/ss.c index 07eecfa7a36db..34c6da5443642 100644 --- a/misc/ss.c

[iproute PATCH v2 3/7] iproute: Fix for missing 'Oifs:' display

2017-08-17 Thread Phil Sutter
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 <p...@nwl.cc> --- ip/iproute

[iproute PATCH v2 5/5] tipc/bearer: Prevent NULL pointer dereference

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- tipc/bearer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tipc/bearer.c b/tipc/bearer.c index c3d4491f8f6ef..0598328ab1f1b 100644 --- a/tipc/bearer.c +++ b/tipc/bearer.c @@ -438,8 +438,8 @@ static int cmd_bearer_

[iproute PATCH v2 6/7] tc/m_xt: Fix for potential string buffer overflows

2017-08-17 Thread Phil Sutter
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 <p...@nwl.cc> --- tc/m_xt.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tc/m_xt.c b/tc/

[iproute PATCH v2 2/2] tc/em_ipset: Don't leak sockfd on error path

2017-08-17 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@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 *v

[iproute PATCH v2 5/7] ss: Use C99 initializer in netlink_show_one()

2017-08-17 Thread Phil Sutter
This has the additional benefit of initializing st.ino to zero which is used later in is_sctp_assoc() function. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ss.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/misc/ss.c b/misc/ss.c index b2a7f06

[iproute PATCH v2 6/7] ss: Drop useless assignment

2017-08-17 Thread Phil Sutter
After '*b = *a', 'b->next' already has the same value as 'a->next'. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ss.c | 1 - 1 file changed, 1 deletion(-) diff --git a/misc/ss.c b/misc/ss.c index 2debccce5260b..b2a7f069e294c 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -14

[iproute PATCH v2 1/5] ifstat, nstat: Check fdopen() return value

2017-08-17 Thread Phil Sutter
Prevent passing NULL FILE pointer to fgets() later. Fix both tools in a single patch since the code changes are basically identical. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ifstat.c | 16 +++- misc/nstat.c | 16 +++- 2 files changed, 22 insertions(

[iproute PATCH v2 0/2] Covscan: Fix potential file descriptor leaks

2017-08-17 Thread Phil Sutter
This series collects patches from v1 which deal with potential file descriptor leaks. No changes to the actual patches, just splitting into smaller series. Phil Sutter (2): ss: Don't leak fd in tcp_show_netlink_file() tc/em_ipset: Don't leak sockfd on error path misc/ss.c | 32

[iproute PATCH v2 6/7] netem/maketable: Check return value of fstat()

2017-08-17 Thread Phil Sutter
Otherwise info.st_size may contain garbage. Signed-off-by: Phil Sutter <p...@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

Re: [iproute PATCH 50/51] Check user supplied interface name lengths

2017-08-15 Thread Phil Sutter
On Tue, Aug 15, 2017 at 09:09:45AM -0700, Stephen Hemminger wrote: > On Sat, 12 Aug 2017 14:05:09 +0200 > Phil Sutter <p...@nwl.cc> wrote: > > > +void assert_valid_dev_name(const char *, const char *); > > Not a fan of long function names. > “I have only made thi

Re: [iproute PATCH 21/51] lib/libnetlink: Don't pass NULL parameter to memcpy()

2017-08-15 Thread Phil Sutter
On Tue, Aug 15, 2017 at 08:15:55AM -0700, Stephen Hemminger wrote: > On Sat, 12 Aug 2017 14:04:40 +0200 > Phil Sutter <p...@nwl.cc> wrote: > > > Both addattr_l() and rta_addattr_l() may be called with NULL data > > pointer and 0 alen parameters. Avoid ca

Re: [iproute PATCH 05/51] iplink_can: Prevent overstepping array bounds

2017-08-15 Thread Phil Sutter
On Tue, Aug 15, 2017 at 08:10:49AM -0700, Stephen Hemminger wrote: > On Sat, 12 Aug 2017 14:04:24 +0200 > Phil Sutter <p...@nwl.cc> wrote: > > > can_state_names array contains at most CAN_STATE_MAX fields, so allowing > > an index to it to be equal to that number

Re: [iproute PATCH 03/51] ipaddress: Make buffer for filter.flushb static

2017-08-15 Thread Phil Sutter
On Tue, Aug 15, 2017 at 08:13:08AM -0700, Stephen Hemminger wrote: > On Sat, 12 Aug 2017 14:04:22 +0200 > Phil Sutter <p...@nwl.cc> wrote: > > > The buffer is accessed outside of the function defining it, so make it > > static. > > > > Signed-off-by: Ph

Re: [iproute PATCH 00/51] Fix potential issues detected by Coverity tool

2017-08-15 Thread Phil Sutter
On Tue, Aug 15, 2017 at 08:07:25AM -0700, Stephen Hemminger wrote: > On Sat, 12 Aug 2017 14:04:19 +0200 > Phil Sutter <p...@nwl.cc> wrote: > > > Covscan really wasn't amused (indicated by the number of patches in this > > series). Try to make it happy. > > &g

Re: [iproute PATCH 51/51] lib/bpf: Check return value of write()

2017-08-14 Thread Phil Sutter
On Mon, Aug 14, 2017 at 11:17:39AM +0200, Daniel Borkmann wrote: > On 08/12/2017 02:05 PM, Phil Sutter wrote: > > This is merely to silence the compiler warning. If write to stderr > > failed, assume that printing an error message will fail as well so don't > > even tr

[iproute PATCH 49/51] lib/ll_map: Make sure im->name is NULL-terminated

2017-08-12 Thread Phil Sutter
Signed-off-by: Phil Sutter <p...@nwl.cc> --- lib/ll_map.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ll_map.c b/lib/ll_map.c index 4e4556c9ac80b..4d06eb69f138a 100644 --- a/lib/ll_map.c +++ b/lib/ll_map.c @@ -120,11 +120,11 @@ int ll_remember_index(const

[iproute PATCH 32/51] ss: Make sure index variable is >= 0

2017-08-12 Thread Phil Sutter
This shouldn't happen but relying upon external data without checking may lead to unexpected results. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ss.c b/misc/ss.c index d767b1103ea81..4d2f75b571ea6

[iproute PATCH 02/51] devlink: No need for this self-assignment

2017-08-12 Thread Phil Sutter
dl_argv_handle_both() will either assign to handle_bit or error out in which case the variable is not used by the caller. Signed-off-by: Phil Sutter <p...@nwl.cc> --- devlink/devlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devlink/devlink.c b/devlink/devlink.c

[iproute PATCH 24/51] ifstat, nstat: Check fdopen() return value

2017-08-12 Thread Phil Sutter
Prevent passing NULL FILE pointer to fgets() later. Fix both tools in a single patch since the code changes are basically identical. Signed-off-by: Phil Sutter <p...@nwl.cc> --- misc/ifstat.c | 16 +++- misc/nstat.c | 16 +++- 2 files changed, 22 insertions(

<    1   2   3   4   5   6   7   8   9   10   >