[PATCH][Binutils] Always skip only 1 byte for CIE version 1's return address register.

[PATCH][Binutils] Always skip only 1 byte for CIE version 1's return address register.

[Tamar Christina]

[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]

Hi All,

According to the specification for the CIE entries, when the CIE version is 1 then
the return address register field is always 1 byte.  Readelf does this correctly in
read_cie in dwarf.c but ld does this incorrectly and always tries to read a
skip_leb128.  If the value here has the top bit set then ld will incorrectly read
at least another byte, causing either an assert failure or an incorrect address to
be used in eh_frame.

I'm not sure how to generate a generic test for this as I'd need to write assembly,
and it's a bit hard to trigger. Essentially the relocated value needs to start with
something that & 0x70 != 0x10 while trying to write a personality.

build on native hardware and regtested on
  aarch64-none-elf, aarch64-none-elf (32 bit host),
  aarch64-none-linux-gnu, aarch64-none-linux-gnu (32 bit host)

Cross-compiled and regtested on
  aarch64-none-linux-gnu, aarch64_be-none-linux-gnu

and no issues.

Ok for master?

Thanks,
Tamar

bfd/ChangeLog:

2019-02-28  Tamar Christina  <tamar.christina@arm.com>

	* elf-eh-frame.c (_bfd_elf_write_section_eh_frame): Correct CIE parse.

-- 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rb10753.patch --]
[-- Type: text/x-diff; name="rb10753.patch", Size: 1215 bytes --]

diff --git a/bfd/elf-eh-frame.c b/bfd/elf-eh-frame.c
index a13e81ebb861129a50ac92390ffb2980111f06c1..15745550772da5241e7270f6552eb4cf3b82edef 100644
--- a/bfd/elf-eh-frame.c
+++ b/bfd/elf-eh-frame.c
@@ -1993,7 +1993,7 @@ _bfd_elf_write_section_eh_frame (bfd *abfd,
 	      || ent->u.cie.per_encoding_relative)
 	    {
 	      char *aug;
-	      unsigned int action, extra_string, extra_data;
+	      unsigned int version, action, extra_string, extra_data;
 	      unsigned int per_width, per_encoding;
 
 	      /* Need to find 'R' or 'L' augmentation's argument and modify
@@ -2005,12 +2005,16 @@ _bfd_elf_write_section_eh_frame (bfd *abfd,
 	      extra_data = extra_augmentation_data_bytes (ent);
 
 	      /* Skip length, id and version.  */
-	      buf += 9;
+	      buf += 8;
+	      version = (unsigned int)(*(char*) buf++);
 	      aug = (char *) buf;
 	      buf += strlen (aug) + 1;
 	      skip_leb128 (&buf, end);
 	      skip_leb128 (&buf, end);
-	      skip_leb128 (&buf, end);
+	      if (version == 1)
+		skip_bytes (&buf, end, 1);
+	      else
+		skip_leb128 (&buf, end);
 	      if (*aug == 'z')
 		{
 		  /* The uleb128 will always be a single byte for the kind

Re: [PATCH][Binutils] Always skip only 1 byte for CIE version 1's return address register.

[Alan Modra]

On Thu, Feb 28, 2019 at 07:14:50PM +0000, Tamar Christina wrote:
> -	      buf += 9;
> +	      buf += 8;
> +	      version = (unsigned int)(*(char*) buf++);

OK, but without any of the casts in the above line.  Plain
	      version = *buf++;
is better.  (Casts to char possibly lead to host dependent behaviour
since char is signed on some hosts, unsigned on others.  Fairly
obviously this cast won't cause trouble, but the casts are entirely
unnecessary.)

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH]Always skip only 1 byte for CIE version 1's return address register.

[Andreas Schwab]

On Feb 28 2019, Tamar Christina <Tamar.Christina@arm.com> wrote:

> @@ -2005,12 +2005,16 @@ _bfd_elf_write_section_eh_frame (bfd *abfd,
>  	      extra_data = extra_augmentation_data_bytes (ent);
>  
>  	      /* Skip length, id and version.  */
> -	      buf += 9;
> +	      buf += 8;
> +	      version = (unsigned int)(*(char*) buf++);

The comment no longer matches.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

RE: [PATCH]Always skip only 1 byte for CIE version 1's return address register.

[Tamar Christina]

Hi Andreas,

Thanks! I had noticed it after sending it out and will update it in the patch before committing.

Regards,
Tamar

> -----Original Message-----
> From: Andreas Schwab <schwab@linux-m68k.org>
> Sent: Friday, March 1, 2019 11:01
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: binutils@sourceware.org; nd <nd@arm.com>
> Subject: Re: [PATCH]Always skip only 1 byte for CIE version 1's return address
> register.
> 
> On Feb 28 2019, Tamar Christina <Tamar.Christina@arm.com> wrote:
> 
> > @@ -2005,12 +2005,16 @@ _bfd_elf_write_section_eh_frame (bfd *abfd,
> >  	      extra_data = extra_augmentation_data_bytes (ent);
> >
> >  	      /* Skip length, id and version.  */
> > -	      buf += 9;
> > +	      buf += 8;
> > +	      version = (unsigned int)(*(char*) buf++);
> 
> The comment no longer matches.
> 
> Andreas.
> 
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."