lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed,  7 Mar 2018 10:40:39 +0200
From:   Serhey Popovych <serhe.popovych@...il.com>
To:     netdev@...r.kernel.org
Cc:     dsahern@...il.com
Subject: [PATCH iproute2-next v4 4/4] iplink: Perform most of request buffer setups and checks in iplink_parse()

To benefit other users (e.g. link_veth.c) of iplink_parse() from
additional attribute checks and setups made in iplink_modify(). This
catches most of weired cobination of parameters to peer device
configuration.

Drop @name, @dev, @link, @group and @index from iplink_parse() parameters
list: they are not needed outside.

While there change return -1 to exit(-1) for group parsing errors: we
want to stop further command processing unless -force option is given
to get error line easily.

Signed-off-by: Serhey Popovych <serhe.popovych@...il.com>
---
 ip/ip_common.h    |    4 +-
 ip/iplink.c       |  143 ++++++++++++++++++++++++++---------------------------
 ip/iplink_vxcan.c |   23 +++------
 ip/link_veth.c    |   23 +++------
 4 files changed, 82 insertions(+), 111 deletions(-)

diff --git a/ip/ip_common.h b/ip/ip_common.h
index e4e628b..1b89795 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -132,9 +132,7 @@ struct link_util {
 
 struct link_util *get_link_kind(const char *kind);
 
-int iplink_parse(int argc, char **argv, struct iplink_req *req,
-		 char **name, char **type, char **link, char **dev,
-		 int *group, int *index);
+int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type);
 
 /* iplink_bridge.c */
 void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len);
diff --git a/ip/iplink.c b/ip/iplink.c
index 6d3ebde..02042ed 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -569,10 +569,11 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 	return 0;
 }
 
-int iplink_parse(int argc, char **argv, struct iplink_req *req,
-		 char **name, char **type, char **link, char **dev,
-		 int *group, int *index)
+int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 {
+	char *name = NULL;
+	char *dev = NULL;
+	char *link = NULL;
 	int ret, len;
 	char abuf[32];
 	int qlen = -1;
@@ -583,9 +584,10 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 	int numrxqueues = -1;
 	int dev_index = 0;
 	int link_netnsid = -1;
+	int index = 0;
+	int group = -1;
 	int addr_len = 0;
 
-	*group = -1;
 	ret = argc;
 
 	while (argc > 0) {
@@ -597,25 +599,25 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			req->i.ifi_flags &= ~IFF_UP;
 		} else if (strcmp(*argv, "name") == 0) {
 			NEXT_ARG();
-			if (*name)
+			if (name)
 				duparg("name", *argv);
 			if (check_ifname(*argv))
 				invarg("\"name\" not a valid ifname", *argv);
-			*name = *argv;
-			if (!*dev) {
-				*dev = *name;
-				dev_index = ll_name_to_index(*dev);
+			name = *argv;
+			if (!dev) {
+				dev = name;
+				dev_index = ll_name_to_index(dev);
 			}
 		} else if (strcmp(*argv, "index") == 0) {
 			NEXT_ARG();
-			if (*index)
+			if (index)
 				duparg("index", *argv);
-			*index = atoi(*argv);
-			if (*index <= 0)
+			index = atoi(*argv);
+			if (index <= 0)
 				invarg("Invalid \"index\" value", *argv);
 		} else if (matches(*argv, "link") == 0) {
 			NEXT_ARG();
-			*link = *argv;
+			link = *argv;
 		} else if (matches(*argv, "address") == 0) {
 			NEXT_ARG();
 			addr_len = ll_addr_a2n(abuf, sizeof(abuf), *argv);
@@ -661,8 +663,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				      generic, drv, offload))
 				exit(-1);
 
-			if (offload && *name == *dev)
-				*dev = NULL;
+			if (offload && name == dev)
+				dev = NULL;
 		} else if (strcmp(*argv, "netns") == 0) {
 			NEXT_ARG();
 			if (netns != -1)
@@ -755,8 +757,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				return -1;
 			addattr_nest_end(&req->n, vflist);
 
-			if (*name == *dev)
-				*dev = NULL;
+			if (name == dev)
+				dev = NULL;
 		} else if (matches(*argv, "master") == 0) {
 			int ifindex;
 
@@ -806,10 +808,11 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 				  *argv, len);
 		} else if (strcmp(*argv, "group") == 0) {
 			NEXT_ARG();
-			if (*group != -1)
+			if (group != -1)
 				duparg("group", *argv);
-			if (rtnl_group_a2n(group, *argv))
+			if (rtnl_group_a2n(&group, *argv))
 				invarg("Invalid \"group\" value\n", *argv);
+			addattr32(&req->n, sizeof(*req), IFLA_GROUP, group);
 		} else if (strcmp(*argv, "mode") == 0) {
 			int mode;
 
@@ -907,23 +910,25 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 
 			if (strcmp(*argv, "dev") == 0)
 				NEXT_ARG();
-			if (*dev != *name)
+			if (dev != name)
 				duparg2("dev", *argv);
 			if (check_ifname(*argv))
 				invarg("\"dev\" not a valid ifname", *argv);
-			*dev = *argv;
-			dev_index = ll_name_to_index(*dev);
+			dev = *argv;
+			dev_index = ll_name_to_index(dev);
 		}
 		argc--; argv++;
 	}
 
+	ret -= argc;
+
 	/* Allow "ip link add dev" and "ip link add name" */
-	if (!*name)
-		*name = *dev;
-	else if (!*dev)
-		*dev = *name;
-	else if (!strcmp(*name, *dev))
-		*name = *dev;
+	if (!name)
+		name = dev;
+	else if (!dev)
+		dev = name;
+	else if (!strcmp(name, dev))
+		name = dev;
 
 	if (dev_index && addr_len) {
 		int halen = nl_get_ll_addr_len(dev_index);
@@ -936,73 +941,40 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 		}
 	}
 
-	return ret - argc;
-}
-
-static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
-{
-	char *dev = NULL;
-	char *name = NULL;
-	char *link = NULL;
-	char *type = NULL;
-	int index = 0;
-	int group;
-	struct link_util *lu = NULL;
-	struct iplink_req req = {
-		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
-		.n.nlmsg_flags = NLM_F_REQUEST | flags,
-		.n.nlmsg_type = cmd,
-		.i.ifi_family = preferred_family,
-	};
-	int ret;
-
-	ret = iplink_parse(argc, argv,
-			   &req, &name, &type, &link, &dev, &group, &index);
-	if (ret < 0)
-		return ret;
-
-	argc -= ret;
-	argv += ret;
-
-	if (!(flags & NLM_F_CREATE) && index) {
+	if (!(req->n.nlmsg_flags & NLM_F_CREATE) && index) {
 		fprintf(stderr,
 			"index can be used only when creating devices.\n");
 		exit(-1);
 	}
 
 	if (group != -1) {
-		if (dev)
-			addattr_l(&req.n, sizeof(req), IFLA_GROUP,
-					&group, sizeof(group));
-		else {
+		if (!dev) {
 			if (argc) {
 				fprintf(stderr,
 					"Garbage instead of arguments \"%s ...\". Try \"ip link help\".\n",
 					*argv);
-				return -1;
+				exit(-1);
 			}
-			if (flags & NLM_F_CREATE) {
+			if (req->n.nlmsg_flags & NLM_F_CREATE) {
 				fprintf(stderr,
 					"group cannot be used when creating devices.\n");
-				return -1;
+				exit(-1);
 			}
 
-			addattr32(&req.n, sizeof(req), IFLA_GROUP, group);
-			if (rtnl_talk(&rth, &req.n, NULL) < 0)
-				return -2;
-			return 0;
+			*type = NULL;
+			return ret;
 		}
 	}
 
-	if (!(flags & NLM_F_CREATE)) {
+	if (!(req->n.nlmsg_flags & NLM_F_CREATE)) {
 		if (!dev) {
 			fprintf(stderr,
 				"Not enough information: \"dev\" argument is required.\n");
 			exit(-1);
 		}
 
-		req.i.ifi_index = ll_name_to_index(dev);
-		if (!req.i.ifi_index)
+		req->i.ifi_index = ll_name_to_index(dev);
+		if (!req->i.ifi_index)
 			return nodev(dev);
 
 		/* Not renaming to the same name */
@@ -1021,18 +993,37 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 			ifindex = ll_name_to_index(link);
 			if (!ifindex)
 				return nodev(link);
-			addattr_l(&req.n, sizeof(req), IFLA_LINK, &ifindex, 4);
+			addattr32(&req->n, sizeof(*req), IFLA_LINK, ifindex);
 		}
 
-		req.i.ifi_index = index;
+		req->i.ifi_index = index;
 	}
 
 	if (name) {
-		addattr_l(&req.n, sizeof(req),
+		addattr_l(&req->n, sizeof(*req),
 			  IFLA_IFNAME, name, strlen(name) + 1);
 	}
 
+	return ret;
+}
+
+static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
+{
+	char *type = NULL;
+	struct iplink_req req = {
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+		.n.nlmsg_flags = NLM_F_REQUEST | flags,
+		.n.nlmsg_type = cmd,
+		.i.ifi_family = preferred_family,
+	};
+	int ret;
+
+	ret = iplink_parse(argc, argv, &req, &type);
+	if (ret < 0)
+		return ret;
+
 	if (type) {
+		struct link_util *lu;
 		struct rtattr *linkinfo;
 		char *ulinep = strchr(type, '_');
 		int iflatype;
@@ -1046,6 +1037,10 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 			iflatype = IFLA_INFO_SLAVE_DATA;
 		else
 			iflatype = IFLA_INFO_DATA;
+
+		argc -= ret;
+		argv += ret;
+
 		if (lu && argc) {
 			struct rtattr *data;
 
diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index ebe9e56..8b08c9a 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -33,16 +33,11 @@ static void usage(void)
 static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
-	char *dev = NULL;
-	char *name = NULL;
-	char *link = NULL;
 	char *type = NULL;
-	int index = 0;
 	int err;
 	struct rtattr *data;
-	int group;
 	struct ifinfomsg *ifm, *peer_ifm;
-	unsigned int ifi_flags, ifi_change;
+	unsigned int ifi_flags, ifi_change, ifi_index;
 
 	if (strcmp(argv[0], "peer") != 0) {
 		usage();
@@ -52,35 +47,29 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 	ifm = NLMSG_DATA(n);
 	ifi_flags = ifm->ifi_flags;
 	ifi_change = ifm->ifi_change;
+	ifi_index = ifm->ifi_index;
 	ifm->ifi_flags = 0;
 	ifm->ifi_change = 0;
+	ifm->ifi_index = 0;
 
 	data = addattr_nest(n, 1024, VXCAN_INFO_PEER);
 
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
-	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
-			   &name, &type, &link, &dev, &group, &index);
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type);
 	if (err < 0)
 		return err;
 
 	if (type)
 		duparg("type", argv[err]);
 
-	if (name) {
-		addattr_l(n, 1024,
-			  IFLA_IFNAME, name, strlen(name) + 1);
-	}
-
 	peer_ifm = RTA_DATA(data);
-	peer_ifm->ifi_index = index;
+	peer_ifm->ifi_index = ifm->ifi_index;
 	peer_ifm->ifi_flags = ifm->ifi_flags;
 	peer_ifm->ifi_change = ifm->ifi_change;
 	ifm->ifi_flags = ifi_flags;
 	ifm->ifi_change = ifi_change;
-
-	if (group != -1)
-		addattr32(n, 1024, IFLA_GROUP, group);
+	ifm->ifi_index = ifi_index;
 
 	addattr_nest_end(n, data);
 	return argc - 1 - err;
diff --git a/ip/link_veth.c b/ip/link_veth.c
index a8e7cf7..33e8f2b 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -31,16 +31,11 @@ static void usage(void)
 static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
-	char *dev = NULL;
-	char *name = NULL;
-	char *link = NULL;
 	char *type = NULL;
-	int index = 0;
 	int err;
 	struct rtattr *data;
-	int group;
 	struct ifinfomsg *ifm, *peer_ifm;
-	unsigned int ifi_flags, ifi_change;
+	unsigned int ifi_flags, ifi_change, ifi_index;
 
 	if (strcmp(argv[0], "peer") != 0) {
 		usage();
@@ -50,35 +45,29 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 	ifm = NLMSG_DATA(n);
 	ifi_flags = ifm->ifi_flags;
 	ifi_change = ifm->ifi_change;
+	ifi_index = ifm->ifi_index;
 	ifm->ifi_flags = 0;
 	ifm->ifi_change = 0;
+	ifm->ifi_index = 0;
 
 	data = addattr_nest(n, 1024, VETH_INFO_PEER);
 
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
-	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
-			   &name, &type, &link, &dev, &group, &index);
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type);
 	if (err < 0)
 		return err;
 
 	if (type)
 		duparg("type", argv[err]);
 
-	if (name) {
-		addattr_l(n, 1024,
-			  IFLA_IFNAME, name, strlen(name) + 1);
-	}
-
 	peer_ifm = RTA_DATA(data);
-	peer_ifm->ifi_index = index;
+	peer_ifm->ifi_index = ifm->ifi_index;
 	peer_ifm->ifi_flags = ifm->ifi_flags;
 	peer_ifm->ifi_change = ifm->ifi_change;
 	ifm->ifi_flags = ifi_flags;
 	ifm->ifi_change = ifi_change;
-
-	if (group != -1)
-		addattr32(n, 1024, IFLA_GROUP, group);
+	ifm->ifi_index = ifi_index;
 
 	addattr_nest_end(n, data);
 	return argc - 1 - err;
-- 
1.7.10.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ