netdev - [PATCH iproute2-next v3 5/8] veth,vxcan: Save/reinitialize/restore whole @struct ifinfomsg

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:   Thu, 22 Feb 2018 15:02:03 +0200
From:   Serhey Popovych <serhe.popovych@...il.com>
To:     netdev@...r.kernel.org
Cc:     dsahern@...il.com
Subject: [PATCH iproute2-next v3 5/8] veth,vxcan: Save/reinitialize/restore whole @struct ifinfomsg

Now in iplink_parse() we use ->ifi_change and ->ifi_flags fields and
plan to use ->ifi_index with upcoming change.

Saving, restoring and reinitializing individual fields is error prone:
using new field in iplink_parse() without updating callers in veth and
vxcan will overwrite main device ifinfomsg data.

Since @struct ifinfomsg is small enough with known sizeof() compiler may
inline memcpy()/memset() with few load/store instructions.

Signed-off-by: Serhey Popovych <serhe.popovych@...il.com>
---
 ip/iplink_vxcan.c |   22 ++++++++--------------
 ip/link_veth.c    |   22 ++++++++--------------
 2 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index ebe9e56..d7e94a0 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -38,11 +38,10 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 	char *link = NULL;
 	char *type = NULL;
 	int index = 0;
+	int group;
 	int err;
 	struct rtattr *data;
-	int group;
-	struct ifinfomsg *ifm, *peer_ifm;
-	unsigned int ifi_flags, ifi_change;
+	struct ifinfomsg ifm_save, *ifm, *peer_ifm;
 
 	if (strcmp(argv[0], "peer") != 0) {
 		usage();
@@ -50,10 +49,8 @@ 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;
-	ifm->ifi_flags = 0;
-	ifm->ifi_change = 0;
+	memcpy(&ifm_save, ifm, sizeof(*ifm));
+	memset(ifm, 0, sizeof(*ifm));
 
 	data = addattr_nest(n, 1024, VXCAN_INFO_PEER);
 
@@ -72,16 +69,13 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 			  IFLA_IFNAME, name, strlen(name) + 1);
 	}
 
-	peer_ifm = RTA_DATA(data);
-	peer_ifm->ifi_index = 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);
 
+	peer_ifm = RTA_DATA(data);
+	memcpy(peer_ifm, ifm, sizeof(*ifm));
+	memcpy(ifm, &ifm_save, sizeof(*ifm));
+
 	addattr_nest_end(n, data);
 	return argc - 1 - err;
 }
diff --git a/ip/link_veth.c b/ip/link_veth.c
index a8e7cf7..b6d37b9 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -36,11 +36,10 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 	char *link = NULL;
 	char *type = NULL;
 	int index = 0;
+	int group;
 	int err;
 	struct rtattr *data;
-	int group;
-	struct ifinfomsg *ifm, *peer_ifm;
-	unsigned int ifi_flags, ifi_change;
+	struct ifinfomsg ifm_save, *ifm, *peer_ifm;
 
 	if (strcmp(argv[0], "peer") != 0) {
 		usage();
@@ -48,10 +47,8 @@ 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;
-	ifm->ifi_flags = 0;
-	ifm->ifi_change = 0;
+	memcpy(&ifm_save, ifm, sizeof(*ifm));
+	memset(ifm, 0, sizeof(*ifm));
 
 	data = addattr_nest(n, 1024, VETH_INFO_PEER);
 
@@ -70,16 +67,13 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 			  IFLA_IFNAME, name, strlen(name) + 1);
 	}
 
-	peer_ifm = RTA_DATA(data);
-	peer_ifm->ifi_index = 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);
 
+	peer_ifm = RTA_DATA(data);
+	memcpy(peer_ifm, ifm, sizeof(*ifm));
+	memcpy(ifm, &ifm_save, sizeof(*ifm));
+
 	addattr_nest_end(n, data);
 	return argc - 1 - err;
 }
-- 
1.7.10.4