[PATCH] sock: clean up handling of ifr_name/ifr_newname

Mike Frysinger vapier at gentoo.org
Sun Oct 19 02:36:00 UTC 2014


The ifr name fields of the ifreq structure might not be NUL terminated.
If the user makes an ioctl call where they aren't, then strace ends up
reading random content from its own stack.

Instead, let's create a local buffer of the exact right size (and NUL
terminated), copy the user buffer into it, and then printf from that.

Further, the decoding of SIOCSIFNAME is incorrect.  It does not use
the ifr_index field to look things up, but ifr_name.

* sock.c (sock_ioctl): Declare name buffer.  Copy ifr_name/ifr_newname
into that and tprintf out.  Split out SIOCSIFNAME from SIOCGIFNAME and
display ifr_newname.
---
 sock.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/sock.c b/sock.c
index dca9bfd..2cd2315 100644
--- a/sock.c
+++ b/sock.c
@@ -58,6 +58,9 @@ sock_ioctl(struct tcb *tcp, long code, long arg)
 	struct ifconf ifc;
 	const char *str = NULL;
 	unsigned char *bytes;
+	/* The user might not NUL delim the field, so do it ourselves. */
+	char name[IFNAMSIZ + 1];
+	name[IFNAMSIZ] = '\0';
 
 	if (entering(tcp)) {
 		if (code == SIOCGIFCONF) {
@@ -128,15 +131,25 @@ sock_ioctl(struct tcb *tcp, long code, long arg)
 		if (umove(tcp, tcp->u_arg[2], &ifr) < 0)
 			tprintf(", %#lx", tcp->u_arg[2]);
 		else if (syserror(tcp)) {
-			if (code == SIOCGIFNAME || code == SIOCSIFNAME)
+			if (code == SIOCGIFNAME) {
 				tprintf(", {ifr_index=%d, ifr_name=???}", ifr.ifr_ifindex);
-			else
-				tprintf(", {ifr_name=\"%s\", ???}", ifr.ifr_name);
-		} else if (code == SIOCGIFNAME || code == SIOCSIFNAME)
+			} else {
+				memcpy(name, ifr.ifr_name, IFNAMSIZ);
+				tprintf(", {ifr_name=\"%s\", ", name);
+
+				if (code == SIOCSIFNAME) {
+					memcpy(name, ifr.ifr_newname, IFNAMSIZ);
+					tprintf("ifr_newname=\"%s\"}", name);
+				} else
+					tprintf("???}");
+			}
+		} else if (code == SIOCGIFNAME) {
+			memcpy(name, ifr.ifr_name, IFNAMSIZ);
 			tprintf(", {ifr_index=%d, ifr_name=\"%s\"}",
-				ifr.ifr_ifindex, ifr.ifr_name);
-		else {
-			tprintf(", {ifr_name=\"%s\", ", ifr.ifr_name);
+				ifr.ifr_ifindex, name);
+		} else {
+			memcpy(name, ifr.ifr_name, IFNAMSIZ);
+			tprintf(", {ifr_name=\"%s\", ", name);
 			switch (code) {
 			case SIOCGIFINDEX:
 				tprintf("ifr_index=%d", ifr.ifr_ifindex);
@@ -189,6 +202,10 @@ sock_ioctl(struct tcb *tcp, long code, long arg)
 			case SIOCSIFMTU:
 				tprintf("ifr_mtu=%d", ifr.ifr_mtu);
 				break;
+			case SIOCSIFNAME:
+				memcpy(name, ifr.ifr_newname, IFNAMSIZ);
+				tprintf("ifr_newname=\"%s\"", name);
+				break;
 			case SIOCGIFSLAVE:
 			case SIOCSIFSLAVE:
 				tprintf("ifr_slave=\"%s\"", ifr.ifr_slave);
@@ -237,8 +254,8 @@ sock_ioctl(struct tcb *tcp, long code, long arg)
 			for (i = 0; i < nifra; ++i ) {
 				if (i > 0)
 					tprints(", ");
-				tprintf("{\"%s\", {",
-					ifra[i].ifr_name);
+				memcpy(name, ifra[i].ifr_newname, IFNAMSIZ);
+				tprintf("{\"%s\", {", name);
 				if (verbose(tcp)) {
 					printxval(addrfams,
 						  ifra[i].ifr_addr.sa_family,
-- 
2.1.2





More information about the Strace-devel mailing list