Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: pu

I'd like to update the iucode-tool package in Debian stable with
cherry-picked fixes from upstrean iucode-tool v1.1.1.

These changes fix issues found by Coverity scan, including a buffer overrun
which causes an out-of-bounds dword write to an array, and some issues on
error paths.

debdiff diffstat:
 debian/changelog                                                               
|   17 ++
 debian/patches/0001-iucode_tool-cosmetic-fix-for-CID-72168.patch               
|   29 +++++
 debian/patches/0002-iucode_tool-cosmetic-fix-for-CID-72166.patch               
|   25 ++++
 debian/patches/0003-iucode_tool-avoid-closing-already-closed-file-handle.patch 
|   29 +++++
 debian/patches/0004-iucode_tool-simplify-fd-tracking-in-scan_system_proc.patch 
|   57 ++++++++++
 debian/patches/0005-iucode_tool-cosmetic-fix-for-CID-72164.patch               
|   25 ++++
 debian/patches/0006-iucode_tool-fix-memory-leak-in-load_intel_microcode_.patch 
|   39 ++++++
 debian/patches/0007-iucode_tool-rework-error-path-of-load_intel_microcod.patch 
|   38 ++++++
 debian/patches/0008-iucode_tool-fix-out-of-bounds-array-access-in-load_i.patch 
|   31 +++++
 debian/patches/series                                                          
|    8 +
 10 files changed, 298 insertions(+)

I've attached the full debdiff output.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
diff -Nru iucode-tool-0.8.3/debian/changelog iucode-tool-0.8.3/debian/changelog
--- iucode-tool-0.8.3/debian/changelog	2012-08-27 21:29:36.000000000 -0300
+++ iucode-tool-0.8.3/debian/changelog	2014-11-30 16:32:41.000000000 -0200
@@ -1,3 +1,20 @@
+iucode-tool (0.8.3-2) stable; urgency=medium
+
+  * cherry-pick fixes from upstream v1.1.1
+    * Add eight new patches cherry-picked from upstream iucode-tool
+      version 1.1.1, fixing several issues found by Coverity scan,
+      including one for an out-of-bounds array write to the heap:
+      + 0001-iucode_tool-cosmetic-fix-for-CID-72168.patch
+      + 0002-iucode_tool-cosmetic-fix-for-CID-72166.patch
+      + 0003-iucode_tool-avoid-closing-already-closed-file-handle.patch
+      + 0004-iucode_tool-simplify-fd-tracking-in-scan_system_proc.patch
+      + 0005-iucode_tool-cosmetic-fix-for-CID-72164.patch
+      + 0006-iucode_tool-fix-memory-leak-in-load_intel_microcode_.patch
+      + 0007-iucode_tool-rework-error-path-of-load_intel_microcod.patch
+      + 0008-iucode_tool-fix-out-of-bounds-array-access-in-load_i.patch
+
+ -- Henrique de Moraes Holschuh <h...@debian.org>  Sun, 30 Nov 2014 16:28:33 -0200
+
 iucode-tool (0.8.3-1) unstable; urgency=low
 
   * New upstream release
diff -Nru iucode-tool-0.8.3/debian/patches/0001-iucode_tool-cosmetic-fix-for-CID-72168.patch iucode-tool-0.8.3/debian/patches/0001-iucode_tool-cosmetic-fix-for-CID-72168.patch
--- iucode-tool-0.8.3/debian/patches/0001-iucode_tool-cosmetic-fix-for-CID-72168.patch	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/0001-iucode_tool-cosmetic-fix-for-CID-72168.patch	2014-11-30 16:21:33.000000000 -0200
@@ -0,0 +1,29 @@
+From: Henrique de Moraes Holschuh <h...@hmh.eng.br>
+Date: Tue, 28 Oct 2014 11:07:14 -0200
+Subject: iucode_tool: cosmetic fix for CID 72168
+
+Remove test for !arg.  The argument to -t is not optional and argp will
+abort before we reach that branch, so the test is not going to trigger.
+
+Alternatively, we could keep the defensive programming, but we'd have to
+add a bug guard arg in argp_error with a (arg)? arg:"none";
+
+Fixes: Coverity CID 72168
+(cherry picked from commit a3919ad8a238ba2453770dd6681ac757854461f7)
+---
+ iucode_tool.c |    2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/iucode_tool.c b/iucode_tool.c
+index d40e3a2..4bfa167 100644
+--- a/iucode_tool.c
++++ b/iucode_tool.c
+@@ -1917,7 +1917,7 @@ static error_t cmdline_do_parse_arg(int key, char *arg,
+ 		break;
+ 
+ 	case 't':
+-		if (!arg || strlen(arg) > 1)
++		if (strlen(arg) > 1)
+ 			argp_error(state, "unknown file type: %s\n", arg);
+ 		switch (*arg) {
+ 		case 'd': /* .dat */
diff -Nru iucode-tool-0.8.3/debian/patches/0002-iucode_tool-cosmetic-fix-for-CID-72166.patch iucode-tool-0.8.3/debian/patches/0002-iucode_tool-cosmetic-fix-for-CID-72166.patch
--- iucode-tool-0.8.3/debian/patches/0002-iucode_tool-cosmetic-fix-for-CID-72166.patch	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/0002-iucode_tool-cosmetic-fix-for-CID-72166.patch	2014-11-30 16:21:33.000000000 -0200
@@ -0,0 +1,25 @@
+From: Henrique de Moraes Holschuh <h...@hmh.eng.br>
+Date: Tue, 28 Oct 2014 11:11:57 -0200
+Subject: iucode_tool: cosmetic fix for CID 72166
+
+argp_state_help() will not return, as we do NOT use ARGP_NO_EXIT,
+still, add a break after it to keep Coverity happy.
+
+Fixes: Coverity CID 72166
+(cherry picked from commit 014c04690f1c808500994225c1b1cbca2391c3a3)
+---
+ iucode_tool.c |    1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/iucode_tool.c b/iucode_tool.c
+index 4bfa167..0204096 100644
+--- a/iucode_tool.c
++++ b/iucode_tool.c
+@@ -1901,6 +1901,7 @@ static error_t cmdline_do_parse_arg(int key, char *arg,
+ 	switch (key) {
+ 	case 'h':
+ 		argp_state_help(state, stdout, ARGP_HELP_STD_HELP);
++		break; /* usually not reached */
+ 
+ 	case 'q':
+ 		verbosity = 0;
diff -Nru iucode-tool-0.8.3/debian/patches/0003-iucode_tool-avoid-closing-already-closed-file-handle.patch iucode-tool-0.8.3/debian/patches/0003-iucode_tool-avoid-closing-already-closed-file-handle.patch
--- iucode-tool-0.8.3/debian/patches/0003-iucode_tool-avoid-closing-already-closed-file-handle.patch	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/0003-iucode_tool-avoid-closing-already-closed-file-handle.patch	2014-11-30 16:21:33.000000000 -0200
@@ -0,0 +1,29 @@
+From: Henrique de Moraes Holschuh <h...@hmh.eng.br>
+Date: Tue, 28 Oct 2014 11:18:51 -0200
+Subject: iucode_tool: avoid closing already closed file handle
+
+Guard against close() of an already closed fd.  This could be triggered by
+the error paths between the close and the fd variable being reassigned.
+
+Fixes: Coverity CID 72163
+(cherry picked from commit cea69de2ad568f6b84000ce7903ea17df54ed959)
+---
+ iucode_tool.c |    4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/iucode_tool.c b/iucode_tool.c
+index 0204096..b6f7d90 100644
+--- a/iucode_tool.c
++++ b/iucode_tool.c
+@@ -728,8 +728,10 @@ static int load_intel_microcode(const char *path,
+ 
+ 		err = 0;
+ 
+-		if (fd != -1)
++		if (fd != -1) {
+ 			close(fd);
++			fd = -1;
++		}
+ 
+ 		if (dir) {
+ 			errno = 0;
diff -Nru iucode-tool-0.8.3/debian/patches/0004-iucode_tool-simplify-fd-tracking-in-scan_system_proc.patch iucode-tool-0.8.3/debian/patches/0004-iucode_tool-simplify-fd-tracking-in-scan_system_proc.patch
--- iucode-tool-0.8.3/debian/patches/0004-iucode_tool-simplify-fd-tracking-in-scan_system_proc.patch	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/0004-iucode_tool-simplify-fd-tracking-in-scan_system_proc.patch	2014-11-30 16:21:33.000000000 -0200
@@ -0,0 +1,57 @@
+From: Henrique de Moraes Holschuh <h...@hmh.eng.br>
+Date: Tue, 28 Oct 2014 11:28:03 -0200
+Subject: iucode_tool: simplify fd tracking in scan_system_processors
+
+Simplify cpuid_fd tracking in scan_system_processors(), this removes a
+dead-code close() found by Coverity.
+
+Fixes: Coverity CID 72159
+(cherry picked from commit 363c50afa0a6430ab754cd50745d66f37135e249)
+---
+ iucode_tool.c |   10 ++--------
+ 1 file changed, 2 insertions(+), 8 deletions(-)
+
+diff --git a/iucode_tool.c b/iucode_tool.c
+index b6f7d90..03c82f8 100644
+--- a/iucode_tool.c
++++ b/iucode_tool.c
+@@ -1524,15 +1524,12 @@ static int scan_system_processors(void)
+ 	uint32_t cpuid_buf[8];
+ 	struct microcode_filter_entry *uc_cpu = NULL;
+ 
+-	int cpuid_fd = -1;
++	int cpuid_fd;
+ 	unsigned int i = 0;
+ 	unsigned int ncpu = 0;
+ 	int rc = 0;
+ 
+ 	while (1) {
+-		if (cpuid_fd != -1)
+-			close(cpuid_fd);
+-
+ 		snprintf(cpuid_device, sizeof(cpuid_device),
+ 			 CPUID_DEVICE_BASE, i);
+ 		cpuid_fd = open(cpuid_device, O_RDONLY);
+@@ -1554,11 +1551,11 @@ static int scan_system_processors(void)
+ 			print_err("%s: access to CPUID(0) and CPUID(1) failed",
+ 				  cpuid_device);
+ 			/* this is in the should not happen list, so abort */
++			close(cpuid_fd);
+ 			rc = 1;
+ 			goto err_out;
+ 		}
+ 		close(cpuid_fd);
+-		cpuid_fd = -1;
+ 
+ 		ncpu++;
+ 
+@@ -1618,9 +1615,6 @@ static int scan_system_processors(void)
+ 	print_msg(2, "checked the signature of %u processor(s)", ncpu);
+ 
+ err_out:
+-	if (cpuid_fd != -1)
+-		close(cpuid_fd);
+-
+ 	if (uc_cpu)
+ 		free_filter_list(uc_cpu);
+ 
diff -Nru iucode-tool-0.8.3/debian/patches/0005-iucode_tool-cosmetic-fix-for-CID-72164.patch iucode-tool-0.8.3/debian/patches/0005-iucode_tool-cosmetic-fix-for-CID-72164.patch
--- iucode-tool-0.8.3/debian/patches/0005-iucode_tool-cosmetic-fix-for-CID-72164.patch	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/0005-iucode_tool-cosmetic-fix-for-CID-72164.patch	2014-11-30 16:21:33.000000000 -0200
@@ -0,0 +1,25 @@
+From: Henrique de Moraes Holschuh <h...@hmh.eng.br>
+Date: Tue, 28 Oct 2014 11:36:03 -0200
+Subject: iucode_tool: cosmetic fix for CID 72164
+
+argp_error() never returns.  Add a breaks that is never going to be
+reached to keep coverity happy.
+
+Fixes: Coverity CID 72164
+(cherry picked from commit eb2bc7a94dff26c3b528ca0c2ad8387aba184dc2)
+---
+ iucode_tool.c |    1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/iucode_tool.c b/iucode_tool.c
+index 03c82f8..3bf4604 100644
+--- a/iucode_tool.c
++++ b/iucode_tool.c
+@@ -1988,6 +1988,7 @@ static error_t cmdline_do_parse_arg(int key, char *arg,
+ 			break; /* success */
+ 		case EINVAL:
+ 			argp_error(state, "invalid filter: %s", arg);
++			break; /* not reached */
+ 		default:
+ 			argp_failure(state, EXIT_SWFAILURE, rc,
+ 				     "could not add filter '%s'", arg);
diff -Nru iucode-tool-0.8.3/debian/patches/0006-iucode_tool-fix-memory-leak-in-load_intel_microcode_.patch iucode-tool-0.8.3/debian/patches/0006-iucode_tool-fix-memory-leak-in-load_intel_microcode_.patch
--- iucode-tool-0.8.3/debian/patches/0006-iucode_tool-fix-memory-leak-in-load_intel_microcode_.patch	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/0006-iucode_tool-fix-memory-leak-in-load_intel_microcode_.patch	2014-11-30 16:21:33.000000000 -0200
@@ -0,0 +1,39 @@
+From: Henrique de Moraes Holschuh <h...@hmh.eng.br>
+Date: Tue, 28 Oct 2014 12:06:29 -0200
+Subject: iucode_tool: fix memory leak in load_intel_microcode_file
+
+We would leak mcb (the entire microcode bundle payload) in the error
+path if add_intel_microcode_bundle() failed.
+
+While at it, remove the test for mcb && mcb_length, as mcb_length will
+not be zero when mcb is not NULL, and we assert() for a valid mcb_length
+in add_intel_microcode_bundle() anyway.
+
+Fixes: Coverity CID 72161
+(cherry picked from commit 0120392a59b257d9bbab4390eb737982e1397b03)
+---
+ iucode_tool.c |    4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/iucode_tool.c b/iucode_tool.c
+index 3bf4604..7617314 100644
+--- a/iucode_tool.c
++++ b/iucode_tool.c
+@@ -664,7 +664,7 @@ static int __load_intel_microcode_file(int fd,
+ 		goto err_out;
+ 	}
+ 
+-	if (!err && mcb && mcb_length) {
++	if (!err && mcb) {
+ 		last_bundle_id++;
+ 		err = add_intel_microcode_bundle(fn, last_bundle_id,
+ 						 mcb, mcb_length);
+@@ -691,6 +691,8 @@ err_out:
+ 	else if (fd != -1)
+ 		close(fd);
+ 
++	free(mcb);
++
+ 	return err;
+ }
+ 
diff -Nru iucode-tool-0.8.3/debian/patches/0007-iucode_tool-rework-error-path-of-load_intel_microcod.patch iucode-tool-0.8.3/debian/patches/0007-iucode_tool-rework-error-path-of-load_intel_microcod.patch
--- iucode-tool-0.8.3/debian/patches/0007-iucode_tool-rework-error-path-of-load_intel_microcod.patch	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/0007-iucode_tool-rework-error-path-of-load_intel_microcod.patch	2014-11-30 16:21:33.000000000 -0200
@@ -0,0 +1,38 @@
+From: Henrique de Moraes Holschuh <h...@hmh.eng.br>
+Date: Tue, 28 Oct 2014 13:05:15 -0200
+Subject: iucode_tool: rework error path of load_intel_microcode_bin
+
+Simplify the error path of load_intel_microcode_bin, so that it can't
+leak memory should something impossible happen (such as read() returning
+-1 with a zero errno).
+
+This is a cosmetic fix to make Coverity happy.
+
+Fixes: Coverity CID 72167, CID 72169
+(cherry picked from commit a483d60f18aa212bcfa3605386f23b31d4cd442a)
+---
+ iucode_tool.c |   10 +++++-----
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/iucode_tool.c b/iucode_tool.c
+index 7617314..1222352 100644
+--- a/iucode_tool.c
++++ b/iucode_tool.c
+@@ -619,12 +619,12 @@ static int __load_intel_microcode_bin(int fd, void **mcb, size_t *mcb_length)
+ 	*mcb = rp;
+ 	*mcb_length = mcb_size;
+ 
++	return 0;
++
+ err_out:
+-	if (err) {
+-		free(mcb_buffer);
+-		*mcb = NULL;
+-		*mcb_length = 0;
+-	}
++	free(mcb_buffer);
++	*mcb = NULL;
++	*mcb_length = 0;
+ 
+ 	return err;
+ }
diff -Nru iucode-tool-0.8.3/debian/patches/0008-iucode_tool-fix-out-of-bounds-array-access-in-load_i.patch iucode-tool-0.8.3/debian/patches/0008-iucode_tool-fix-out-of-bounds-array-access-in-load_i.patch
--- iucode-tool-0.8.3/debian/patches/0008-iucode_tool-fix-out-of-bounds-array-access-in-load_i.patch	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/0008-iucode_tool-fix-out-of-bounds-array-access-in-load_i.patch	2014-11-30 16:21:33.000000000 -0200
@@ -0,0 +1,31 @@
+From: Henrique de Moraes Holschuh <h...@hmh.eng.br>
+Date: Tue, 28 Oct 2014 13:47:16 -0200
+Subject: iucode_tool: fix out-of-bounds array access in
+ load_intel_microcode_dat
+
+Fix an off-by-one error in the realloc() logic, which would cause an
+one-dword write past the end of the buffer.  Note that this bug won't be
+triggered by the current data files distributed by Intel, as they are
+smaller than the initial buffer size.
+
+This one got past Valgrind, but Coverity nailed it.
+
+Fixes: Coverity CID 72165
+(cherry picked from commit cbf2c8ebf3717056131a361d6060ab13b6894785)
+---
+ iucode_tool.c |    2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/iucode_tool.c b/iucode_tool.c
+index 1222352..e7bb8ed 100644
+--- a/iucode_tool.c
++++ b/iucode_tool.c
+@@ -477,7 +477,7 @@ static int __load_intel_microcode_dat(FILE *fp, void **mcb, size_t *mcb_length)
+ 
+ 			/* we did read a value */
+ 			pos++;
+-			if (unlikely(mcb_buflen < pos)) {
++			if (unlikely(mcb_buflen <= pos)) {
+ 				/* expand buffer */
+ 				mcb_buflen += buffer_size_granularity;
+ 				rp = realloc(mcb_buffer,
diff -Nru iucode-tool-0.8.3/debian/patches/series iucode-tool-0.8.3/debian/patches/series
--- iucode-tool-0.8.3/debian/patches/series	1969-12-31 21:00:00.000000000 -0300
+++ iucode-tool-0.8.3/debian/patches/series	2014-11-30 16:22:04.000000000 -0200
@@ -0,0 +1,8 @@
+0001-iucode_tool-cosmetic-fix-for-CID-72168.patch
+0002-iucode_tool-cosmetic-fix-for-CID-72166.patch
+0003-iucode_tool-avoid-closing-already-closed-file-handle.patch
+0004-iucode_tool-simplify-fd-tracking-in-scan_system_proc.patch
+0005-iucode_tool-cosmetic-fix-for-CID-72164.patch
+0006-iucode_tool-fix-memory-leak-in-load_intel_microcode_.patch
+0007-iucode_tool-rework-error-path-of-load_intel_microcod.patch
+0008-iucode_tool-fix-out-of-bounds-array-access-in-load_i.patch

Reply via email to