Re: [iproute PATCH v2 0/3] Check user supplied interface name lengths
From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2017-09-29 17:31:15
Subsystem:
library code, the rest · Maintainers:
Andrew Morton, Linus Torvalds
On Wed, 27 Sep 2017 18:05:28 +0200 Phil Sutter [off-list ref] wrote:
On Wed, Sep 27, 2017 at 08:42:49AM +0100, Stephen Hemminger wrote:quoted
On Tue, 26 Sep 2017 18:35:45 +0200 Phil Sutter [off-list ref] wrote:quoted
This series adds explicit checks for user-supplied interface names to make sure their length fits Linux's requirements. The first two patches simplify interface name parsing in some places - these are side-effects of working on the actual implementation provided in patch three. Changes since v1: - Patches 1 and 2 introduced. - Changes to patch 3 are listed in there. Phil Sutter (3): ip{6,}tunnel: Avoid copying user-supplied interface name around tc: flower: No need to cache indev arg Check user supplied interface name lengths include/utils.h | 1 + ip/ip6tunnel.c | 9 +++++---- ip/ipl2tp.c | 3 ++- ip/iplink.c | 27 ++++++++------------------- ip/ipmaddr.c | 1 + ip/iprule.c | 4 ++++ ip/iptunnel.c | 27 +++++++++++++-------------- ip/iptuntap.c | 4 +++- lib/utils.c | 10 ++++++++++ misc/arpd.c | 1 + tc/f_flower.c | 6 ++---- 11 files changed, 50 insertions(+), 43 deletions(-)I like the idea, and checking arguments is good.Cool!
I was thinking something like:
diff --git a/include/utils.h b/include/utils.h
index c9ed230b9604..e2702b56f2e0 100644
--- a/include/utils.h
+++ b/include/utils.h@@ -105,6 +105,8 @@ int get_be64(__be64 *val, const char *arg, int base); int get_be32(__be32 *val, const char *arg, int base); int get_be16(__be16 *val, const char *arg, int base); int get_addr64(__u64 *ap, const char *cp); +int check_ifname(const char *arg); +int get_ifname(char *buf, const char *arg); int hex2mem(const char *buf, uint8_t *mem, int count); char *hexstring_n2a(const __u8 *str, int len, char *buf, int blen);
diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index b4a7def14422..a6f0e99bdc21 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c@@ -180,7 +180,8 @@ static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p) memcpy(&p->laddr, &laddr.data, sizeof(p->laddr)); } else if (strcmp(*argv, "dev") == 0) { NEXT_ARG(); - strncpy(medium, *argv, IFNAMSIZ - 1); + if (get_ifname(medium, *argv)) + invarg("\"medium\" not a valid ifname", *argv); } else if (strcmp(*argv, "encaplimit") == 0) { NEXT_ARG(); if (strcmp(*argv, "none") == 0) {
diff --git a/ip/iplink.c b/ip/iplink.c
index ff5b56c038d2..89aa51ed3b40 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c@@ -1265,6 +1265,8 @@ static int do_set(int argc, char **argv) flags &= ~IFF_UP; } else if (strcmp(*argv, "name") == 0) { NEXT_ARG(); + if (check_ifname(*argv)) + invarg("Invalid \"name\"\n", *argv); newname = *argv; } else if (matches(*argv, "address") == 0) { NEXT_ARG();
@@ -1383,9 +1385,6 @@ static int do_set(int argc, char **argv) } if (newname && strcmp(dev, newname)) { - if (strlen(newname) == 0) - invarg("\"\" is not a valid device identifier\n", - "name"); if (do_changename(dev, newname) < 0) return -1; dev = newname;
diff --git a/lib/utils.c b/lib/utils.c
index bbd3cbc46a0e..a93b45b51a3b 100644
--- a/lib/utils.c
+++ b/lib/utils.c@@ -20,6 +20,7 @@ #include <sys/socket.h> #include <netinet/in.h> #include <string.h> +#include <ctype.h> #include <netdb.h> #include <arpa/inet.h> #include <asm/types.h>
@@ -465,6 +466,34 @@ int get_addr64(__u64 *ap, const char *cp) return 1; } +int check_ifname(const char *name) +{ + /* These check mimic kernel checks in dev_valid_name */ + if (*name == '\0') + return -1; + if (strlen(name) >= IFNAMSIZ) + return -1; + + while (*name) { + if (*name == '/' || isspace(*name)) + return -1; + ++name; + } + return 0; +} + +/* buf is assumed to be IFNAMSIZ */ +int get_ifname(char *buf, const char *name) +{ + int ret; + + ret = check_ifname(name); + if (ret == 0) + strncpy(buf, name, IFNAMSIZ - 1); + + return ret; +} + int get_addr_1(inet_prefix *addr, const char *name, int family) { memset(addr, 0, sizeof(*addr));
--
2.11.0