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

Mike Frysinger vapier at gentoo.org
Tue Oct 21 12:34:08 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.  Limit the printf lengths.

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): Add explicit length limits to ifr_name/ifr_newname
printfs.  Split out SIOCSIFNAME from SIOCGIFNAME and display ifr_newname.
---
v2
	- use %.*s rather than a local buffer

 sock.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/sock.c b/sock.c
index dca9bfd..e4f304c 100644
--- a/sock.c
+++ b/sock.c
@@ -128,15 +128,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)
-				tprintf(", {ifr_index=%d, ifr_name=???}", ifr.ifr_ifindex);
-			else
-				tprintf(", {ifr_name=\"%s\", ???}", ifr.ifr_name);
-		} else if (code == SIOCGIFNAME || code == SIOCSIFNAME)
-			tprintf(", {ifr_index=%d, ifr_name=\"%s\"}",
-				ifr.ifr_ifindex, ifr.ifr_name);
-		else {
-			tprintf(", {ifr_name=\"%s\", ", ifr.ifr_name);
+			if (code == SIOCGIFNAME) {
+				tprintf(", {ifr_index=%d, ifr_name=???}",
+					ifr.ifr_ifindex);
+			} else {
+				tprintf(", {ifr_name=\"%.*s\", ",
+					IFNAMSIZ, ifr.ifr_name);
+
+				if (code == SIOCSIFNAME)
+					tprintf("ifr_newname=\"%.*s\"}",
+						IFNAMSIZ, ifr.ifr_newname);
+				else
+					tprintf("???}");
+			}
+		} else if (code == SIOCGIFNAME) {
+			tprintf(", {ifr_index=%d, ifr_name=\"%.*s\"}",
+				ifr.ifr_ifindex, IFNAMSIZ, ifr.ifr_name);
+		} else {
+			tprintf(", {ifr_name=\"%.*s\", ",
+				IFNAMSIZ, ifr.ifr_name);
 			switch (code) {
 			case SIOCGIFINDEX:
 				tprintf("ifr_index=%d", ifr.ifr_ifindex);
@@ -189,6 +199,10 @@ sock_ioctl(struct tcb *tcp, long code, long arg)
 			case SIOCSIFMTU:
 				tprintf("ifr_mtu=%d", ifr.ifr_mtu);
 				break;
+			case SIOCSIFNAME:
+				tprintf("ifr_newname=\"%.*s\"",
+					IFNAMSIZ, ifr.ifr_newname);
+				break;
 			case SIOCGIFSLAVE:
 			case SIOCSIFSLAVE:
 				tprintf("ifr_slave=\"%s\"", ifr.ifr_slave);
@@ -237,8 +251,7 @@ 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);
+				tprintf("{\"%.*s\", {", IFNAMSIZ, ifra[i].ifr_newname);
 				if (verbose(tcp)) {
 					printxval(addrfams,
 						  ifra[i].ifr_addr.sa_family,
-- 
2.1.2





More information about the Strace-devel mailing list