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

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

[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 of strslashrsplit()

 devlink/devlink.c | 16 ++++++++++++----
 ip/iplink_vrf.c   |  4 +++-
 ip/iproute.c      |  6 ++++--
 3 files changed, 19 insertions(+), 7 deletions(-)

-- 
2.13.1

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

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@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 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1493,7 +1493,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", *argv);
 			filter.markmask = -1;
 		} else if (strcmp(*argv, "via") == 0) {
 			int family;
@@ -1710,7 +1711,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", *argv);
 		} else if (matches(*argv, "oif") == 0 ||
 			   strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
-- 
2.13.1

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

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: David Ahern <dsahern@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/iplink_vrf.c
index 917630e853375..2b85a3a56eb70 100644
--- a/ip/iplink_vrf.c
+++ b/ip/iplink_vrf.c
@@ -131,7 +131,9 @@ __u32 ipvrf_get_table(const char *name)
 					   &answer.n, sizeof(answer)) < 0) {
 		/* special case "default" vrf to be the main table */
 		if (errno == ENODEV && !strcmp(name, "default"))
-			rtnl_rttable_a2n(&tb_id, "main");
+			if (rtnl_rttable_a2n(&tb_id, "main"))
+				fprintf(stderr,
+					"BUG: RTTable \"main\" not found.\n");
 
 		return tb_id;
 	}
-- 
2.13.1

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

[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 <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

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

[Stephen Hemminger]

On Mon, 21 Aug 2017 18:36:49 +0200
Phil Sutter <phil@nwl.cc> wrote:

> 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 of strslashrsplit()
> 
>  devlink/devlink.c | 16 ++++++++++++----
>  ip/iplink_vrf.c   |  4 +++-
>  ip/iproute.c      |  6 ++++--
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 

These 3 look fine. Applied