All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] binutils: Fix CVE-2017-6965 and CVE-2017-6966
@ 2017-04-10 10:34 Yuanjie Huang
  2017-04-10 11:02 ` ✗ patchtest: failure for " Patchwork
  2017-04-10 22:10 ` [PATCH] " Richard Purdie
  0 siblings, 2 replies; 7+ messages in thread
From: Yuanjie Huang @ 2017-04-10 10:34 UTC (permalink / raw)
  To: openembedded-core

Backport upstream commit to address vulnerabilities:

[BZ 21137] -- https://sourceware.org/bugzilla/show_bug.cgi?id=21137

Fix readelf writing to illegal addresses whilst processing corrupt input
files containing symbol-difference relocations.

	PR binutils/21137
	* readelf.c (target_specific_reloc_handling): Add end parameter.
	Check for buffer overflow before writing relocated values.
	(apply_relocations): Pass end to target_specific_reloc_handling.

[BZ 21139] -- https://sourceware.org/bugzilla/show_bug.cgi?id=21139

Fix read-after-free error in readelf when processing multiple, relocated
sections in an MSP430 binary.

	PR binutils/21139
	* readelf.c (target_specific_reloc_handling): Add num_syms
	parameter.  Check for symbol table overflow before accessing
	symbol value.  If reloc pointer is NULL, discard all saved state.
	(apply_relocations): Pass num_syms to target_specific_reloc_handling.
	Call target_specific_reloc_handling with a NULL reloc pointer
	after processing all of the relocs.

Signed-off-by: Yuanjie Huang <yuanjie.huang@windriver.com>
---
 meta/recipes-devtools/binutils/binutils-2.27.inc   |   2 +
 .../binutils/binutils/CVE-2017-6965.patch          | 127 +++++++++++
 .../binutils/binutils/CVE-2017-6966.patch          | 240 +++++++++++++++++++++
 3 files changed, 369 insertions(+)
 create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2017-6965.patch
 create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2017-6966.patch

diff --git a/meta/recipes-devtools/binutils/binutils-2.27.inc b/meta/recipes-devtools/binutils/binutils-2.27.inc
index a7cdb6f1d4..f98fef9e02 100644
--- a/meta/recipes-devtools/binutils/binutils-2.27.inc
+++ b/meta/recipes-devtools/binutils/binutils-2.27.inc
@@ -39,6 +39,8 @@ SRC_URI = "\
      file://0016-Fix-seg-fault-in-ARM-linker-when-trying-to-parse-a-b.patch \
      file://0017-Fix-the-generation-of-alignment-frags-in-code-sectio.patch \
      file://0001-ppc-apuinfo-for-spe-parsed-incorrectly.patch \
+     file://CVE-2017-6965.patch \
+     file://CVE-2017-6966.patch \
 "
 S  = "${WORKDIR}/git"
 
diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2017-6965.patch b/meta/recipes-devtools/binutils/binutils/CVE-2017-6965.patch
new file mode 100644
index 0000000000..85f7f98fe0
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/CVE-2017-6965.patch
@@ -0,0 +1,127 @@
+From 6f898c17b1d6f6a29a05ca6de31f0fc8f52cfbfe Mon Sep 17 00:00:00 2001
+From: Nick Clifton <nickc@redhat.com>
+Date: Mon, 13 Feb 2017 13:08:32 +0000
+Subject: [PATCH 1/2] Fix readelf writing to illegal addresses whilst
+ processing corrupt input files containing symbol-difference relocations.
+
+	PR binutils/21137
+	* readelf.c (target_specific_reloc_handling): Add end parameter.
+	Check for buffer overflow before writing relocated values.
+	(apply_relocations): Pass end to target_specific_reloc_handling.
+
+(cherry pick from commit 03f7786e2f440b9892b1c34a58fb26222ce1b493)
+Upstream-Status: Backport [master]
+CVE: CVE-2017-6965
+
+Signed-off-by: Yuanjie Huang <yuanjie.huang@windriver.com>
+---
+ binutils/ChangeLog |  7 +++++++
+ binutils/readelf.c | 30 +++++++++++++++++++++++++-----
+ 2 files changed, 32 insertions(+), 5 deletions(-)
+
+diff --git a/binutils/ChangeLog b/binutils/ChangeLog
+index 995de87dc3..154b797a29 100644
+--- a/binutils/ChangeLog
++++ b/binutils/ChangeLog
+@@ -5,6 +5,13 @@
+ 	Check for buffer overflow before writing relocated values.
+ 	(apply_relocations): Pass end to target_specific_reloc_handling.
+ 
++2017-02-13  Nick Clifton  <nickc@redhat.com>
++
++	PR binutils/21137
++	* readelf.c (target_specific_reloc_handling): Add end parameter.
++	Check for buffer overflow before writing relocated values.
++	(apply_relocations): Pass end to target_specific_reloc_handling.
++
+ 2016-08-03  Tristan Gingold  <gingold@adacore.com>
+ 
+ 	* configure: Regenerate.
+diff --git a/binutils/readelf.c b/binutils/readelf.c
+index d31558c3b4..220671f76f 100644
+--- a/binutils/readelf.c
++++ b/binutils/readelf.c
+@@ -11345,6 +11345,7 @@ process_syminfo (FILE * file ATTRIBUTE_UNUSED)
+ static bfd_boolean
+ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 				unsigned char *     start,
++				unsigned char *     end,
+ 				Elf_Internal_Sym *  symtab)
+ {
+   unsigned int reloc_type = get_reloc_type (reloc->r_info);
+@@ -11384,13 +11385,19 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	  handle_sym_diff:
+ 	    if (saved_sym != NULL)
+ 	      {
++		int reloc_size = reloc_type == 1 ? 4 : 2;
+ 		bfd_vma value;
+ 
+ 		value = reloc->r_addend
+ 		  + (symtab[get_reloc_symindex (reloc->r_info)].st_value
+ 		     - saved_sym->st_value);
+ 
+-		byte_put (start + reloc->r_offset, value, reloc_type == 1 ? 4 : 2);
++		if (start + reloc->r_offset + reloc_size >= end)
++		  /* PR 21137 */
++		  error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"),
++			 start + reloc->r_offset + reloc_size, end);
++		else
++		  byte_put (start + reloc->r_offset, value, reloc_size);
+ 
+ 		saved_sym = NULL;
+ 		return TRUE;
+@@ -11421,13 +11428,18 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	  case 2: /* R_MN10300_16 */
+ 	    if (saved_sym != NULL)
+ 	      {
++		int reloc_size = reloc_type == 1 ? 4 : 2;
+ 		bfd_vma value;
+ 
+ 		value = reloc->r_addend
+ 		  + (symtab[get_reloc_symindex (reloc->r_info)].st_value
+ 		     - saved_sym->st_value);
+ 
+-		byte_put (start + reloc->r_offset, value, reloc_type == 1 ? 4 : 2);
++		if (start + reloc->r_offset + reloc_size >= end)
++		  error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"),
++			 start + reloc->r_offset + reloc_size, end);
++		else
++		  byte_put (start + reloc->r_offset, value, reloc_size);
+ 
+ 		saved_sym = NULL;
+ 		return TRUE;
+@@ -11462,12 +11474,20 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	    break;
+ 
+ 	  case 0x41: /* R_RL78_ABS32.  */
+-	    byte_put (start + reloc->r_offset, value, 4);
++	    if (start + reloc->r_offset + 4 >= end)
++	      error (_("RL78 sym diff reloc writes past end of section (%p vs %p)\n"),
++		     start + reloc->r_offset + 2, end);
++	    else
++	      byte_put (start + reloc->r_offset, value, 4);
+ 	    value = 0;
+ 	    return TRUE;
+ 
+ 	  case 0x43: /* R_RL78_ABS16.  */
+-	    byte_put (start + reloc->r_offset, value, 2);
++	    if (start + reloc->r_offset + 2 >= end)
++	      error (_("RL78 sym diff reloc writes past end of section (%p vs %p)\n"),
++		     start + reloc->r_offset + 2, end);
++	    else
++	      byte_put (start + reloc->r_offset, value, 2);
+ 	    value = 0;
+ 	    return TRUE;
+ 
+@@ -12074,7 +12094,7 @@ apply_relocations (void *                     file,
+ 
+ 	  reloc_type = get_reloc_type (rp->r_info);
+ 
+-	  if (target_specific_reloc_handling (rp, start, symtab))
++	  if (target_specific_reloc_handling (rp, start, end, symtab))
+ 	    continue;
+ 	  else if (is_none_reloc (reloc_type))
+ 	    continue;
+-- 
+2.11.0
+
diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2017-6966.patch b/meta/recipes-devtools/binutils/binutils/CVE-2017-6966.patch
new file mode 100644
index 0000000000..5e364ef69c
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/CVE-2017-6966.patch
@@ -0,0 +1,240 @@
+From 310e2cdc0a46ef62602097f5c21c393571e76df4 Mon Sep 17 00:00:00 2001
+From: Nick Clifton <nickc@redhat.com>
+Date: Mon, 13 Feb 2017 14:03:22 +0000
+Subject: [PATCH 2/2] Fix read-after-free error in readelf when processing
+ multiple, relocated sections in an MSP430 binary.
+
+	PR binutils/21139
+	* readelf.c (target_specific_reloc_handling): Add num_syms
+	parameter.  Check for symbol table overflow before accessing
+	symbol value.  If reloc pointer is NULL, discard all saved state.
+	(apply_relocations): Pass num_syms to target_specific_reloc_handling.
+	Call target_specific_reloc_handling with a NULL reloc pointer
+	after processing all of the relocs.
+
+(cherry pick from commit f84ce13b6708801ca1d6289b7c4003e2f5a6d7f9)
+Upstream-Status: Backport [master]
+CVE: CVE-2017-6966
+
+Signed-off-by: Yuanjie Huang <yuanjie.huang@windriver.com>
+---
+ binutils/ChangeLog |  10 +++++
+ binutils/readelf.c | 109 +++++++++++++++++++++++++++++++++++++++++------------
+ 2 files changed, 94 insertions(+), 25 deletions(-)
+
+diff --git a/binutils/ChangeLog b/binutils/ChangeLog
+index 154b797a29..aef0a51f19 100644
+--- a/binutils/ChangeLog
++++ b/binutils/ChangeLog
+@@ -1,5 +1,15 @@
+ 2017-02-13  Nick Clifton  <nickc@redhat.com>
+ 
++	PR binutils/21139
++	* readelf.c (target_specific_reloc_handling): Add num_syms
++	parameter.  Check for symbol table overflow before accessing
++	symbol value.  If reloc pointer is NULL, discard all saved state.
++	(apply_relocations): Pass num_syms to target_specific_reloc_handling.
++	Call target_specific_reloc_handling with a NULL reloc pointer
++	after processing all of the relocs.
++
++2017-02-13  Nick Clifton  <nickc@redhat.com>
++
+ 	PR binutils/21137
+ 	* readelf.c (target_specific_reloc_handling): Add end parameter.
+ 	Check for buffer overflow before writing relocated values.
+diff --git a/binutils/readelf.c b/binutils/readelf.c
+index 220671f76f..2b6cef1638 100644
+--- a/binutils/readelf.c
++++ b/binutils/readelf.c
+@@ -11340,15 +11340,27 @@ process_syminfo (FILE * file ATTRIBUTE_UNUSED)
+ 
+ /* Check to see if the given reloc needs to be handled in a target specific
+    manner.  If so then process the reloc and return TRUE otherwise return
+-   FALSE.  */
++   FALSE.
++
++   If called with reloc == NULL, then this is a signal that reloc processing
++   for the current section has finished, and any saved state should be
++   discarded.  */
+ 
+ static bfd_boolean
+ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 				unsigned char *     start,
+ 				unsigned char *     end,
+-				Elf_Internal_Sym *  symtab)
++				Elf_Internal_Sym *  symtab,
++				unsigned long       num_syms)
+ {
+-  unsigned int reloc_type = get_reloc_type (reloc->r_info);
++  unsigned int reloc_type = 0;
++  unsigned long sym_index = 0;
++
++  if (reloc)
++    {
++      reloc_type = get_reloc_type (reloc->r_info);
++      sym_index = get_reloc_symindex (reloc->r_info);
++    }
+ 
+   switch (elf_header.e_machine)
+     {
+@@ -11357,13 +11369,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+       {
+ 	static Elf_Internal_Sym * saved_sym = NULL;
+ 
++	if (reloc == NULL)
++	  {
++	    saved_sym = NULL;
++	    return TRUE;
++	  }
++
+ 	switch (reloc_type)
+ 	  {
+ 	  case 10: /* R_MSP430_SYM_DIFF */
+ 	    if (uses_msp430x_relocs ())
+ 	      break;
+ 	  case 21: /* R_MSP430X_SYM_DIFF */
+-	    saved_sym = symtab + get_reloc_symindex (reloc->r_info);
++	    /* PR 21139.  */
++	    if (sym_index >= num_syms)
++	      error (_("MSP430 SYM_DIFF reloc contains invalid symbol index %lu\n"),
++		     sym_index);
++	    else
++	      saved_sym = symtab + sym_index;
+ 	    return TRUE;
+ 
+ 	  case 1: /* R_MSP430_32 or R_MSP430_ABS32 */
+@@ -11388,16 +11411,21 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 		int reloc_size = reloc_type == 1 ? 4 : 2;
+ 		bfd_vma value;
+ 
+-		value = reloc->r_addend
+-		  + (symtab[get_reloc_symindex (reloc->r_info)].st_value
+-		     - saved_sym->st_value);
+-
+-		if (start + reloc->r_offset + reloc_size >= end)
+-		  /* PR 21137 */
+-		  error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"),
+-			 start + reloc->r_offset + reloc_size, end);
++		if (sym_index >= num_syms)
++		  error (_("MSP430 reloc contains invalid symbol index %lu\n"),
++			 sym_index);
+ 		else
+-		  byte_put (start + reloc->r_offset, value, reloc_size);
++		  {
++		    value = reloc->r_addend + (symtab[sym_index].st_value
++					       - saved_sym->st_value);
++
++		    if (start + reloc->r_offset + reloc_size >= end)
++		      /* PR 21137 */
++		      error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"),
++			     start + reloc->r_offset + reloc_size, end);
++		    else
++		      byte_put (start + reloc->r_offset, value, reloc_size);
++		  }
+ 
+ 		saved_sym = NULL;
+ 		return TRUE;
+@@ -11417,13 +11445,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+       {
+ 	static Elf_Internal_Sym * saved_sym = NULL;
+ 
++	if (reloc == NULL)
++	  {
++	    saved_sym = NULL;
++	    return TRUE;
++	  }
++
+ 	switch (reloc_type)
+ 	  {
+ 	  case 34: /* R_MN10300_ALIGN */
+ 	    return TRUE;
+ 	  case 33: /* R_MN10300_SYM_DIFF */
+-	    saved_sym = symtab + get_reloc_symindex (reloc->r_info);
++	    if (sym_index >= num_syms)
++	      error (_("MN10300_SYM_DIFF reloc contains invalid symbol index %lu\n"),
++		     sym_index);
++	    else
++	      saved_sym = symtab + sym_index;
+ 	    return TRUE;
++
+ 	  case 1: /* R_MN10300_32 */
+ 	  case 2: /* R_MN10300_16 */
+ 	    if (saved_sym != NULL)
+@@ -11431,15 +11470,20 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 		int reloc_size = reloc_type == 1 ? 4 : 2;
+ 		bfd_vma value;
+ 
+-		value = reloc->r_addend
+-		  + (symtab[get_reloc_symindex (reloc->r_info)].st_value
+-		     - saved_sym->st_value);
+-
+-		if (start + reloc->r_offset + reloc_size >= end)
+-		  error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"),
+-			 start + reloc->r_offset + reloc_size, end);
++		if (sym_index >= num_syms)
++		  error (_("MN10300 reloc contains invalid symbol index %lu\n"),
++			 sym_index);
+ 		else
+-		  byte_put (start + reloc->r_offset, value, reloc_size);
++		  {
++		    value = reloc->r_addend + (symtab[sym_index].st_value
++					       - saved_sym->st_value);
++
++		    if (start + reloc->r_offset + reloc_size >= end)
++		      error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"),
++			     start + reloc->r_offset + reloc_size, end);
++		    else
++		      byte_put (start + reloc->r_offset, value, reloc_size);
++		  }
+ 
+ 		saved_sym = NULL;
+ 		return TRUE;
+@@ -11459,12 +11503,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	static bfd_vma saved_sym2 = 0;
+ 	static bfd_vma value;
+ 
++	if (reloc == NULL)
++	  {
++	    saved_sym1 = saved_sym2 = 0;
++	    return TRUE;
++	  }
++
+ 	switch (reloc_type)
+ 	  {
+ 	  case 0x80: /* R_RL78_SYM.  */
+ 	    saved_sym1 = saved_sym2;
+-	    saved_sym2 = symtab[get_reloc_symindex (reloc->r_info)].st_value;
+-	    saved_sym2 += reloc->r_addend;
++	    if (sym_index >= num_syms)
++	      error (_("RL78_SYM reloc contains invalid symbol index %lu\n"),
++		     sym_index);
++	    else
++	      {
++		saved_sym2 = symtab[sym_index].st_value;
++		saved_sym2 += reloc->r_addend;
++	      }
+ 	    return TRUE;
+ 
+ 	  case 0x83: /* R_RL78_OPsub.  */
+@@ -12094,7 +12150,7 @@ apply_relocations (void *                     file,
+ 
+ 	  reloc_type = get_reloc_type (rp->r_info);
+ 
+-	  if (target_specific_reloc_handling (rp, start, end, symtab))
++	  if (target_specific_reloc_handling (rp, start, end, symtab, num_syms))
+ 	    continue;
+ 	  else if (is_none_reloc (reloc_type))
+ 	    continue;
+@@ -12190,6 +12246,9 @@ apply_relocations (void *                     file,
+ 	}
+ 
+       free (symtab);
++      /* Let the target specific reloc processing code know that
++	 we have finished with these relocs.  */
++      target_specific_reloc_handling (NULL, NULL, NULL, NULL, 0);
+ 
+       if (relocs_return)
+ 	{
+-- 
+2.11.0
+
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✗ patchtest: failure for binutils: Fix CVE-2017-6965 and CVE-2017-6966
  2017-04-10 10:34 [PATCH] binutils: Fix CVE-2017-6965 and CVE-2017-6966 Yuanjie Huang
@ 2017-04-10 11:02 ` Patchwork
  2017-04-10 22:10 ` [PATCH] " Richard Purdie
  1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-04-10 11:02 UTC (permalink / raw)
  To: Yuanjie Huang; +Cc: openembedded-core

== Series Details ==

Series: binutils: Fix CVE-2017-6965 and CVE-2017-6966
Revision: 1
URL   : https://patchwork.openembedded.org/series/6259/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Patch            binutils: Fix CVE-2017-6965 and CVE-2017-6966
 Issue             Missing or incorrectly formatted CVE tag in commit message [test_cve_presence_in_commit_message] 
  Suggested fix    Include a "CVE-xxxx-xxxx" tag in the commit message

* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at 901659a51c)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] binutils: Fix CVE-2017-6965 and CVE-2017-6966
  2017-04-10 10:34 [PATCH] binutils: Fix CVE-2017-6965 and CVE-2017-6966 Yuanjie Huang
  2017-04-10 11:02 ` ✗ patchtest: failure for " Patchwork
@ 2017-04-10 22:10 ` Richard Purdie
  2017-04-11  6:18   ` Yuanjie Huang
  1 sibling, 1 reply; 7+ messages in thread
From: Richard Purdie @ 2017-04-10 22:10 UTC (permalink / raw)
  To: Yuanjie Huang, openembedded-core

On Mon, 2017-04-10 at 03:34 -0700, Yuanjie Huang wrote:
> meta/recipes-devtools/binutils/binutils-2.27.inc

Given master contains 2.28, I don't think this patch was based on or
tested against master?

Cheers,

Richard


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] binutils: Fix CVE-2017-6965 and CVE-2017-6966
  2017-04-10 22:10 ` [PATCH] " Richard Purdie
@ 2017-04-11  6:18   ` Yuanjie Huang
  2017-04-11  7:00     ` [PATCH V2] " Yuanjie Huang
  2017-04-11  7:28     ` [PATCH] " Richard Purdie
  0 siblings, 2 replies; 7+ messages in thread
From: Yuanjie Huang @ 2017-04-11  6:18 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core



On 04/11/2017 06:10 AM, Richard Purdie wrote:
> On Mon, 2017-04-10 at 03:34 -0700, Yuanjie Huang wrote:
>> meta/recipes-devtools/binutils/binutils-2.27.inc
> Given master contains 2.28, I don't think this patch was based on or
> tested against master?
Not yet, I will work on that, and will send the patch for master later. 
Our current product is based on Morty, so I fixed it first.

Cheers,
Yuanjie
> Cheers,
>
> Richard



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH V2] binutils: Fix CVE-2017-6965 and CVE-2017-6966
  2017-04-11  6:18   ` Yuanjie Huang
@ 2017-04-11  7:00     ` Yuanjie Huang
  2017-04-11  7:28     ` [PATCH] " Richard Purdie
  1 sibling, 0 replies; 7+ messages in thread
From: Yuanjie Huang @ 2017-04-11  7:00 UTC (permalink / raw)
  To: openembedded-core

Backport upstream commit to address vulnerabilities:

CVE: CVE-2017-6965
[BZ 21137] -- https://sourceware.org/bugzilla/show_bug.cgi?id=21137

Fix readelf writing to illegal addresses whilst processing corrupt input
files containing symbol-difference relocations.

	PR binutils/21137
	* readelf.c (target_specific_reloc_handling): Add end parameter.
	Check for buffer overflow before writing relocated values.
	(apply_relocations): Pass end to target_specific_reloc_handling.

CVE: CVE-2017-6966
[BZ 21139] -- https://sourceware.org/bugzilla/show_bug.cgi?id=21139

Fix read-after-free error in readelf when processing multiple, relocated
sections in an MSP430 binary.

	PR binutils/21139
	* readelf.c (target_specific_reloc_handling): Add num_syms
	parameter.  Check for symbol table overflow before accessing
	symbol value.  If reloc pointer is NULL, discard all saved state.
	(apply_relocations): Pass num_syms to target_specific_reloc_handling.
	Call target_specific_reloc_handling with a NULL reloc pointer
	after processing all of the relocs.

Signed-off-by: Yuanjie Huang <yuanjie.huang@windriver.com>
---
 meta/recipes-devtools/binutils/binutils-2.28.inc   |   2 +
 .../binutils/binutils/CVE-2017-6965.patch          | 124 +++++++++++
 .../binutils/binutils/CVE-2017-6966.patch          | 241 +++++++++++++++++++++
 3 files changed, 367 insertions(+)
 create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2017-6965.patch
 create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2017-6966.patch

diff --git a/meta/recipes-devtools/binutils/binutils-2.28.inc b/meta/recipes-devtools/binutils/binutils-2.28.inc
index 76b81b04ca..7585da1ca9 100644
--- a/meta/recipes-devtools/binutils/binutils-2.28.inc
+++ b/meta/recipes-devtools/binutils/binutils-2.28.inc
@@ -35,6 +35,8 @@ SRC_URI = "\
      file://0014-fix-the-incorrect-assembling-for-ppc-wait-mnemonic.patch \
      file://0015-sync-with-OE-libtool-changes.patch \
      file://0016-Detect-64-bit-MIPS-targets.patch \
+     file://CVE-2017-6965.patch \
+     file://CVE-2017-6966.patch \
 "
 S  = "${WORKDIR}/git"
 
diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2017-6965.patch b/meta/recipes-devtools/binutils/binutils/CVE-2017-6965.patch
new file mode 100644
index 0000000000..1334c9444d
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/CVE-2017-6965.patch
@@ -0,0 +1,124 @@
+From bdc5166c274b842f83f8328e7cfaaf80fd29934e Mon Sep 17 00:00:00 2001
+From: Nick Clifton <nickc@redhat.com>
+Date: Mon, 13 Feb 2017 13:08:32 +0000
+Subject: [PATCH 1/2] Fix readelf writing to illegal addresses whilst
+ processing corrupt input files containing symbol-difference relocations.
+
+	PR binutils/21137
+	* readelf.c (target_specific_reloc_handling): Add end parameter.
+	Check for buffer overflow before writing relocated values.
+	(apply_relocations): Pass end to target_specific_reloc_handling.
+
+(cherry pick from commit 03f7786e2f440b9892b1c34a58fb26222ce1b493)
+Upstream-Status: Backport [master]
+CVE: CVE-2017-6965
+
+Signed-off-by: Yuanjie Huang <yuanjie.huang@windriver.com>
+---
+ binutils/ChangeLog |  7 +++++++
+ binutils/readelf.c | 30 +++++++++++++++++++++++++-----
+ 2 files changed, 32 insertions(+), 5 deletions(-)
+
+diff --git a/binutils/ChangeLog b/binutils/ChangeLog
+index f21867f98c..e789a3b99b 100644
+--- a/binutils/ChangeLog
++++ b/binutils/ChangeLog
+@@ -1,3 +1,10 @@
++2017-02-13  Nick Clifton  <nickc@redhat.com>
++
++	PR binutils/21137
++	* readelf.c (target_specific_reloc_handling): Add end parameter.
++	Check for buffer overflow before writing relocated values.
++	(apply_relocations): Pass end to target_specific_reloc_handling.
++
+ 2017-03-02  Tristan Gingold  <gingold@adacore.com>
+ 
+ 	* configure: Regenerate.
+diff --git a/binutils/readelf.c b/binutils/readelf.c
+index b5f577f5a1..8cdaae3b8c 100644
+--- a/binutils/readelf.c
++++ b/binutils/readelf.c
+@@ -11585,6 +11585,7 @@ process_syminfo (FILE * file ATTRIBUTE_UNUSED)
+ static bfd_boolean
+ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 				unsigned char *     start,
++				unsigned char *     end,
+ 				Elf_Internal_Sym *  symtab)
+ {
+   unsigned int reloc_type = get_reloc_type (reloc->r_info);
+@@ -11625,13 +11626,19 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	  handle_sym_diff:
+ 	    if (saved_sym != NULL)
+ 	      {
++		int reloc_size = reloc_type == 1 ? 4 : 2;
+ 		bfd_vma value;
+ 
+ 		value = reloc->r_addend
+ 		  + (symtab[get_reloc_symindex (reloc->r_info)].st_value
+ 		     - saved_sym->st_value);
+ 
+-		byte_put (start + reloc->r_offset, value, reloc_type == 1 ? 4 : 2);
++		if (start + reloc->r_offset + reloc_size >= end)
++		  /* PR 21137 */
++		  error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"),
++			 start + reloc->r_offset + reloc_size, end);
++		else
++		  byte_put (start + reloc->r_offset, value, reloc_size);
+ 
+ 		saved_sym = NULL;
+ 		return TRUE;
+@@ -11662,13 +11669,18 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	  case 2: /* R_MN10300_16 */
+ 	    if (saved_sym != NULL)
+ 	      {
++		int reloc_size = reloc_type == 1 ? 4 : 2;
+ 		bfd_vma value;
+ 
+ 		value = reloc->r_addend
+ 		  + (symtab[get_reloc_symindex (reloc->r_info)].st_value
+ 		     - saved_sym->st_value);
+ 
+-		byte_put (start + reloc->r_offset, value, reloc_type == 1 ? 4 : 2);
++		if (start + reloc->r_offset + reloc_size >= end)
++		  error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"),
++			 start + reloc->r_offset + reloc_size, end);
++		else
++		  byte_put (start + reloc->r_offset, value, reloc_size);
+ 
+ 		saved_sym = NULL;
+ 		return TRUE;
+@@ -11703,12 +11715,20 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	    break;
+ 
+ 	  case 0x41: /* R_RL78_ABS32.  */
+-	    byte_put (start + reloc->r_offset, value, 4);
++	    if (start + reloc->r_offset + 4 >= end)
++	      error (_("RL78 sym diff reloc writes past end of section (%p vs %p)\n"),
++		     start + reloc->r_offset + 2, end);
++	    else
++	      byte_put (start + reloc->r_offset, value, 4);
+ 	    value = 0;
+ 	    return TRUE;
+ 
+ 	  case 0x43: /* R_RL78_ABS16.  */
+-	    byte_put (start + reloc->r_offset, value, 2);
++	    if (start + reloc->r_offset + 2 >= end)
++	      error (_("RL78 sym diff reloc writes past end of section (%p vs %p)\n"),
++		     start + reloc->r_offset + 2, end);
++	    else
++	      byte_put (start + reloc->r_offset, value, 2);
+ 	    value = 0;
+ 	    return TRUE;
+ 
+@@ -12325,7 +12345,7 @@ apply_relocations (void *                     file,
+ 
+ 	  reloc_type = get_reloc_type (rp->r_info);
+ 
+-	  if (target_specific_reloc_handling (rp, start, symtab))
++	  if (target_specific_reloc_handling (rp, start, end, symtab))
+ 	    continue;
+ 	  else if (is_none_reloc (reloc_type))
+ 	    continue;
+-- 
+2.11.0
+
diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2017-6966.patch b/meta/recipes-devtools/binutils/binutils/CVE-2017-6966.patch
new file mode 100644
index 0000000000..dd58df5fbf
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/CVE-2017-6966.patch
@@ -0,0 +1,241 @@
+From 383ec757d27652448d1511169e1133f486abf54f Mon Sep 17 00:00:00 2001
+From: Nick Clifton <nickc@redhat.com>
+Date: Mon, 13 Feb 2017 14:03:22 +0000
+Subject: [PATCH] Fix read-after-free error in readelf when processing
+ multiple, relocated sections in an MSP430 binary.
+
+	PR binutils/21139
+	* readelf.c (target_specific_reloc_handling): Add num_syms
+	parameter.  Check for symbol table overflow before accessing
+	symbol value.  If reloc pointer is NULL, discard all saved state.
+	(apply_relocations): Pass num_syms to target_specific_reloc_handling.
+	Call target_specific_reloc_handling with a NULL reloc pointer
+	after processing all of the relocs.
+
+(cherry pick from commit f84ce13b6708801ca1d6289b7c4003e2f5a6d7f9)
+Upstream-Status: Backport [master]
+CVE: CVE-2017-6966
+
+Signed-off-by: Yuanjie Huang <yuanjie.huang@windriver.com>
+---
+ binutils/ChangeLog |  10 +++++
+ binutils/readelf.c | 109 +++++++++++++++++++++++++++++++++++++++++------------
+ 2 files changed, 94 insertions(+), 25 deletions(-)
+
+diff --git a/binutils/ChangeLog b/binutils/ChangeLog
+index e789a3b99b..bd63c8a0d8 100644
+--- a/binutils/ChangeLog
++++ b/binutils/ChangeLog
+@@ -1,5 +1,15 @@
+ 2017-02-13  Nick Clifton  <nickc@redhat.com>
+ 
++	PR binutils/21139
++	* readelf.c (target_specific_reloc_handling): Add num_syms
++	parameter.  Check for symbol table overflow before accessing
++	symbol value.  If reloc pointer is NULL, discard all saved state.
++	(apply_relocations): Pass num_syms to target_specific_reloc_handling.
++	Call target_specific_reloc_handling with a NULL reloc pointer
++	after processing all of the relocs.
++
++2017-02-13  Nick Clifton  <nickc@redhat.com>
++
+ 	PR binutils/21137
+ 	* readelf.c (target_specific_reloc_handling): Add end parameter.
+ 	Check for buffer overflow before writing relocated values.
+diff --git a/binutils/readelf.c b/binutils/readelf.c
+index 8cdaae3b8c..7c158c6342 100644
+--- a/binutils/readelf.c
++++ b/binutils/readelf.c
+@@ -11580,15 +11580,27 @@ process_syminfo (FILE * file ATTRIBUTE_UNUSED)
+ 
+ /* Check to see if the given reloc needs to be handled in a target specific
+    manner.  If so then process the reloc and return TRUE otherwise return
+-   FALSE.  */
++   FALSE.
++
++   If called with reloc == NULL, then this is a signal that reloc processing
++   for the current section has finished, and any saved state should be
++   discarded.  */
+ 
+ static bfd_boolean
+ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 				unsigned char *     start,
+ 				unsigned char *     end,
+-				Elf_Internal_Sym *  symtab)
++				Elf_Internal_Sym *  symtab,
++				unsigned long       num_syms)
+ {
+-  unsigned int reloc_type = get_reloc_type (reloc->r_info);
++  unsigned int reloc_type = 0;
++  unsigned long sym_index = 0;
++
++  if (reloc)
++    {
++      reloc_type = get_reloc_type (reloc->r_info);
++      sym_index = get_reloc_symindex (reloc->r_info);
++    }
+ 
+   switch (elf_header.e_machine)
+     {
+@@ -11597,6 +11609,12 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+       {
+ 	static Elf_Internal_Sym * saved_sym = NULL;
+ 
++	if (reloc == NULL)
++	  {
++	    saved_sym = NULL;
++	    return TRUE;
++	  }
++
+ 	switch (reloc_type)
+ 	  {
+ 	  case 10: /* R_MSP430_SYM_DIFF */
+@@ -11604,7 +11622,12 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	      break;
+ 	    /* Fall through.  */
+ 	  case 21: /* R_MSP430X_SYM_DIFF */
+-	    saved_sym = symtab + get_reloc_symindex (reloc->r_info);
++	    /* PR 21139.  */
++	    if (sym_index >= num_syms)
++	      error (_("MSP430 SYM_DIFF reloc contains invalid symbol index %lu\n"),
++		     sym_index);
++	    else
++	      saved_sym = symtab + sym_index;
+ 	    return TRUE;
+ 
+ 	  case 1: /* R_MSP430_32 or R_MSP430_ABS32 */
+@@ -11629,16 +11652,21 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 		int reloc_size = reloc_type == 1 ? 4 : 2;
+ 		bfd_vma value;
+ 
+-		value = reloc->r_addend
+-		  + (symtab[get_reloc_symindex (reloc->r_info)].st_value
+-		     - saved_sym->st_value);
+-
+-		if (start + reloc->r_offset + reloc_size >= end)
+-		  /* PR 21137 */
+-		  error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"),
+-			 start + reloc->r_offset + reloc_size, end);
++		if (sym_index >= num_syms)
++		  error (_("MSP430 reloc contains invalid symbol index %lu\n"),
++			 sym_index);
+ 		else
+-		  byte_put (start + reloc->r_offset, value, reloc_size);
++		  {
++		    value = reloc->r_addend + (symtab[sym_index].st_value
++					       - saved_sym->st_value);
++
++		    if (start + reloc->r_offset + reloc_size >= end)
++		      /* PR 21137 */
++		      error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"),
++			     start + reloc->r_offset + reloc_size, end);
++		    else
++		      byte_put (start + reloc->r_offset, value, reloc_size);
++		  }
+ 
+ 		saved_sym = NULL;
+ 		return TRUE;
+@@ -11658,13 +11686,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+       {
+ 	static Elf_Internal_Sym * saved_sym = NULL;
+ 
++	if (reloc == NULL)
++	  {
++	    saved_sym = NULL;
++	    return TRUE;
++	  }
++
+ 	switch (reloc_type)
+ 	  {
+ 	  case 34: /* R_MN10300_ALIGN */
+ 	    return TRUE;
+ 	  case 33: /* R_MN10300_SYM_DIFF */
+-	    saved_sym = symtab + get_reloc_symindex (reloc->r_info);
++	    if (sym_index >= num_syms)
++	      error (_("MN10300_SYM_DIFF reloc contains invalid symbol index %lu\n"),
++		     sym_index);
++	    else
++	      saved_sym = symtab + sym_index;
+ 	    return TRUE;
++
+ 	  case 1: /* R_MN10300_32 */
+ 	  case 2: /* R_MN10300_16 */
+ 	    if (saved_sym != NULL)
+@@ -11672,15 +11711,20 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 		int reloc_size = reloc_type == 1 ? 4 : 2;
+ 		bfd_vma value;
+ 
+-		value = reloc->r_addend
+-		  + (symtab[get_reloc_symindex (reloc->r_info)].st_value
+-		     - saved_sym->st_value);
+-
+-		if (start + reloc->r_offset + reloc_size >= end)
+-		  error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"),
+-			 start + reloc->r_offset + reloc_size, end);
++		if (sym_index >= num_syms)
++		  error (_("MN10300 reloc contains invalid symbol index %lu\n"),
++			 sym_index);
+ 		else
+-		  byte_put (start + reloc->r_offset, value, reloc_size);
++		  {
++		    value = reloc->r_addend + (symtab[sym_index].st_value
++					       - saved_sym->st_value);
++
++		    if (start + reloc->r_offset + reloc_size >= end)
++		      error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"),
++			     start + reloc->r_offset + reloc_size, end);
++		    else
++		      byte_put (start + reloc->r_offset, value, reloc_size);
++		  }
+ 
+ 		saved_sym = NULL;
+ 		return TRUE;
+@@ -11700,12 +11744,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
+ 	static bfd_vma saved_sym2 = 0;
+ 	static bfd_vma value;
+ 
++	if (reloc == NULL)
++	  {
++	    saved_sym1 = saved_sym2 = 0;
++	    return TRUE;
++	  }
++
+ 	switch (reloc_type)
+ 	  {
+ 	  case 0x80: /* R_RL78_SYM.  */
+ 	    saved_sym1 = saved_sym2;
+-	    saved_sym2 = symtab[get_reloc_symindex (reloc->r_info)].st_value;
+-	    saved_sym2 += reloc->r_addend;
++	    if (sym_index >= num_syms)
++	      error (_("RL78_SYM reloc contains invalid symbol index %lu\n"),
++		     sym_index);
++	    else
++	      {
++		saved_sym2 = symtab[sym_index].st_value;
++		saved_sym2 += reloc->r_addend;
++	      }
+ 	    return TRUE;
+ 
+ 	  case 0x83: /* R_RL78_OPsub.  */
+@@ -12345,7 +12401,7 @@ apply_relocations (void *                     file,
+ 
+ 	  reloc_type = get_reloc_type (rp->r_info);
+ 
+-	  if (target_specific_reloc_handling (rp, start, end, symtab))
++	  if (target_specific_reloc_handling (rp, start, end, symtab, num_syms))
+ 	    continue;
+ 	  else if (is_none_reloc (reloc_type))
+ 	    continue;
+@@ -12441,6 +12497,9 @@ apply_relocations (void *                     file,
+ 	}
+ 
+       free (symtab);
++      /* Let the target specific reloc processing code know that
++	 we have finished with these relocs.  */
++      target_specific_reloc_handling (NULL, NULL, NULL, NULL, 0);
+ 
+       if (relocs_return)
+ 	{
+-- 
+2.11.0
+
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] binutils: Fix CVE-2017-6965 and CVE-2017-6966
  2017-04-11  6:18   ` Yuanjie Huang
  2017-04-11  7:00     ` [PATCH V2] " Yuanjie Huang
@ 2017-04-11  7:28     ` Richard Purdie
  2017-04-11  8:50       ` Yuanjie Huang
  1 sibling, 1 reply; 7+ messages in thread
From: Richard Purdie @ 2017-04-11  7:28 UTC (permalink / raw)
  To: Yuanjie Huang, openembedded-core

On Tue, 2017-04-11 at 14:18 +0800, Yuanjie Huang wrote:
> 
> On 04/11/2017 06:10 AM, Richard Purdie wrote:
> > 
> > On Mon, 2017-04-10 at 03:34 -0700, Yuanjie Huang wrote:
> > > 
> > > meta/recipes-devtools/binutils/binutils-2.27.inc
> > Given master contains 2.28, I don't think this patch was based on
> > or
> > tested against master?
> Not yet, I will work on that, and will send the patch for master
> later. 
> Our current product is based on Morty, so I fixed it first.

If a patch is against morty, you need to say that when you post it.

The patch will not be accepted until master has the issue resolved. 

If the issue isn't present in master, you should mention that.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] binutils: Fix CVE-2017-6965 and CVE-2017-6966
  2017-04-11  7:28     ` [PATCH] " Richard Purdie
@ 2017-04-11  8:50       ` Yuanjie Huang
  0 siblings, 0 replies; 7+ messages in thread
From: Yuanjie Huang @ 2017-04-11  8:50 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core



On 04/11/2017 03:28 PM, Richard Purdie wrote:
> On Tue, 2017-04-11 at 14:18 +0800, Yuanjie Huang wrote:
>> On 04/11/2017 06:10 AM, Richard Purdie wrote:
>>> On Mon, 2017-04-10 at 03:34 -0700, Yuanjie Huang wrote:
>>>> meta/recipes-devtools/binutils/binutils-2.27.inc
>>> Given master contains 2.28, I don't think this patch was based on
>>> or
>>> tested against master?
>> Not yet, I will work on that, and will send the patch for master
>> later.
>> Our current product is based on Morty, so I fixed it first.
> If a patch is against morty, you need to say that when you post it.
>
> The patch will not be accepted until master has the issue resolved.
>
> If the issue isn't present in master, you should mention that.
I sent that mail without realizing that I forgot the morty tag, now both 
patches for morty and master are sent.

Best,
Yuanjie

> Cheers,
>
> Richard



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-04-11  8:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-10 10:34 [PATCH] binutils: Fix CVE-2017-6965 and CVE-2017-6966 Yuanjie Huang
2017-04-10 11:02 ` ✗ patchtest: failure for " Patchwork
2017-04-10 22:10 ` [PATCH] " Richard Purdie
2017-04-11  6:18   ` Yuanjie Huang
2017-04-11  7:00     ` [PATCH V2] " Yuanjie Huang
2017-04-11  7:28     ` [PATCH] " Richard Purdie
2017-04-11  8:50       ` Yuanjie Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.