netdev - [ethtool PATCH v3 03/12] ethtool.c: fix dump

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]

Message-Id: <1457138533-2417-4-git-send-email-ddecotig@gmail.com>
Date:	Fri,  4 Mar 2016 16:42:04 -0800
From:	David Decotigny <ddecotig@...il.com>
To:	netdev@...r.kernel.org
Cc:	Jeff Garzik <jgarzik@...ox.com>,
	Ben Hutchings <ben@...adent.org.uk>,
	David Miller <davem@...hat.com>,
	Vidya Sagar Ravipati <vidya@...ulusnetworks.com>,
	Joe Perches <joe@...ches.com>,
	David Decotigny <decot@...glers.com>
Subject: [ethtool PATCH v3 03/12] ethtool.c: fix dump_regs heap corruption

From: David Decotigny <decot@...glers.com>

The 'regs' pointer is owned by do_gregs(), but updated internally inside
dump_regs() without propagating it back to do_gregs(): later free(regs)
in do_gregs() reclaims the wrong area. This commit moves the realloc()
inside do_gregs().


Signed-off-by: David Decotigny <decot@...glers.com>
---
 ethtool.c | 46 +++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 8a93dd1..c64b962 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -994,7 +994,6 @@ void dump_hex(FILE *file, const u8 *data, int len, int offset)
 }
 
 static int dump_regs(int gregs_dump_raw, int gregs_dump_hex,
-		     const char *gregs_dump_file,
 		     struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 {
 	int i;
@@ -1004,25 +1003,6 @@ static int dump_regs(int gregs_dump_raw, int gregs_dump_hex,
 		return 0;
 	}
 
-	if (gregs_dump_file) {
-		FILE *f = fopen(gregs_dump_file, "r");
-		struct stat st;
-		size_t nread;
-
-		if (!f || fstat(fileno(f), &st) < 0) {
-			fprintf(stderr, "Can't open '%s': %s\n",
-				gregs_dump_file, strerror(errno));
-			return -1;
-		}
-
-		regs = realloc(regs, sizeof(*regs) + st.st_size);
-		regs->len = st.st_size;
-		nread = fread(regs->data, regs->len, 1, f);
-		fclose(f);
-		if (1 != nread)
-			return -1;
-	}
-
 	if (!gregs_dump_hex)
 		for (i = 0; i < ARRAY_SIZE(driver_list); i++)
 			if (!strncmp(driver_list[i].name, info->driver,
@@ -2711,7 +2691,31 @@ static int do_gregs(struct cmd_context *ctx)
 		free(regs);
 		return 74;
 	}
-	if (dump_regs(gregs_dump_raw, gregs_dump_hex, gregs_dump_file,
+
+	if ((!gregs_dump_raw) && (NULL != gregs_dump_file)) {
+		/* overwrite reg values from file dump */
+		FILE *f = fopen(gregs_dump_file, "r");
+		struct stat st;
+		size_t nread;
+
+		if (!f || fstat(fileno(f), &st) < 0) {
+			fprintf(stderr, "Can't open '%s': %s\n",
+				gregs_dump_file, strerror(errno));
+			free(regs);
+			return 75;
+		}
+
+		regs = realloc(regs, sizeof(*regs) + st.st_size);
+		regs->len = st.st_size;
+		nread = fread(regs->data, regs->len, 1, f);
+		fclose(f);
+		if (1 != nread) {
+			free(regs);
+			return 75;
+		}
+        }
+
+	if (dump_regs(gregs_dump_raw, gregs_dump_hex,
 		      &drvinfo, regs) < 0) {
 		fprintf(stderr, "Cannot dump registers\n");
 		free(regs);
-- 
2.7.0.rc3.207.g0ac5344