[PATCH] tc: flower: fix port value truncation

[PATCH] tc: flower: fix port value truncation

[Lukasz Czapnik]

sscanf truncates read port values silently without any error. As sscanf
man says:
(...) sscanf() conform to C89 and C99 and POSIX.1-2001. These standards
do not specify the ERANGE error.

Replace sscanf with safer get_be16 that returns error when value is out
of range.

Example:
tc filter add dev eth0 protocol ip parent ffff: prio 1 flower ip_proto
tcp dst_port 70000 hw_tc 1

Would result in filter for port 4464 without any warning.

Fixes: 8930840e678b ("tc: flower: Classify packets based port ranges")
Signed-off-by: Lukasz Czapnik <lukasz.czapnik@intel.com>
---
 tc/f_flower.c | 48 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 9659e894..e2420d92 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -493,23 +493,40 @@ static int flower_port_range_attr_type(__u8 ip_proto, enum flower_endpoint type,
 	return 0;
 }
 
+/* parse range args in format 10-20 */
+static int parse_range(char *str, __be16 *min, __be16 *max)
+{
+	char *sep;
+
+	sep = strchr(str, '-');
+	if (sep) {
+		*sep = '\0';
+
+		if (get_be16(min, str, 10))
+			return -1;
+
+		if (get_be16(max, sep + 1, 10))
+			return -1;
+	} else {
+		if (get_be16(min, str, 10))
+			return -1;
+	}
+	return 0;
+}
+
 static int flower_parse_port(char *str, __u8 ip_proto,
 			     enum flower_endpoint endpoint,
 			     struct nlmsghdr *n)
 {
-	__u16 min, max;
+	__be16 min = 0;
+	__be16 max = 0;
 	int ret;
 
-	ret = sscanf(str, "%hu-%hu", &min, &max);
-
-	if (ret == 1) {
-		int type;
+	ret = parse_range(str, &min, &max);
+	if (ret)
+		return -1;
 
-		type = flower_port_attr_type(ip_proto, endpoint);
-		if (type < 0)
-			return -1;
-		addattr16(n, MAX_MSG, type, htons(min));
-	} else if (ret == 2) {
+	if (min && max) {
 		__be16 min_port_type, max_port_type;
 
 		if (max <= min) {
@@ -520,8 +537,15 @@ static int flower_parse_port(char *str, __u8 ip_proto,
 						&min_port_type, &max_port_type))
 			return -1;
 
-		addattr16(n, MAX_MSG, min_port_type, htons(min));
-		addattr16(n, MAX_MSG, max_port_type, htons(max));
+		addattr16(n, MAX_MSG, min_port_type, min);
+		addattr16(n, MAX_MSG, max_port_type, max);
+	} else if (min && !max) {
+		int type;
+
+		type = flower_port_attr_type(ip_proto, endpoint);
+		if (type < 0)
+			return -1;
+		addattr16(n, MAX_MSG, type, min);
 	} else {
 		return -1;
 	}
-- 
2.17.2

Re: [PATCH] tc: flower: fix port value truncation

[Stephen Hemminger]

On Mon, 27 May 2019 23:03:49 +0200
Lukasz Czapnik <lukasz.czapnik@gmail.com> wrote:

> sscanf truncates read port values silently without any error. As sscanf
> man says:
> (...) sscanf() conform to C89 and C99 and POSIX.1-2001. These standards
> do not specify the ERANGE error.
> 
> Replace sscanf with safer get_be16 that returns error when value is out
> of range.
> 
> Example:
> tc filter add dev eth0 protocol ip parent ffff: prio 1 flower ip_proto
> tcp dst_port 70000 hw_tc 1
> 
> Would result in filter for port 4464 without any warning.
> 
> Fixes: 8930840e678b ("tc: flower: Classify packets based port ranges")
> Signed-off-by: Lukasz Czapnik <lukasz.czapnik@intel.com>

Looks good, applied.