[ARM] Avoid dereferencing null pointers

Message ID CAKdteOYqxMCpOrBnH_N8JmJZzLgMwxZx0ywkh_DDPUvub9OPKA@mail.gmail.com
State New
Headers show
Series
  • [ARM] Avoid dereferencing null pointers
Related show

Commit Message

Christophe Lyon Oct. 23, 2018, 2:21 p.m.
Hi,

While building an ARM FDPIC toolchain with a compiler generating Thumb
code, I face a couple of null pointer dereferences in cmse_scan().

When browsing ld-uClibc.so.1, all the external symbols have no info in
sym_hashes (sym_hashes[X] == NULL), and when handling libgcc_s.so.1
for the 2nd time in the same command, sym_hashes == NULL.

I don't know why this doesn't happen with a compiler generating Arm
code (ie. why the symbol tables are handled differently), but the
attached small patch prevents the linker from crashing.

OK?

Thanks,

Christophe

2018-10-23  Christophe Lyon  <christophe.lyon@linaro.org>

	* elf32-arm.c (cmse_scan): Avoid dereferencing NULL pointers.

Comments

Alan Modra Oct. 23, 2018, 10:16 p.m. | #1
On Tue, Oct 23, 2018 at 04:21:41PM +0200, Christophe Lyon wrote:
> Hi,

> 

> While building an ARM FDPIC toolchain with a compiler generating Thumb

> code, I face a couple of null pointer dereferences in cmse_scan().

> 

> When browsing ld-uClibc.so.1, all the external symbols have no info in

> sym_hashes (sym_hashes[X] == NULL), and when handling libgcc_s.so.1

> for the 2nd time in the same command, sym_hashes == NULL.

> 

> I don't know why this doesn't happen with a compiler generating Arm

> code (ie. why the symbol tables are handled differently), but the

> attached small patch prevents the linker from crashing.

> 

> OK?


No, this is just papering over the real problem.  You need to find out
why the sym hashes are not being set up (or are being overwritten).

-- 
Alan Modra
Australia Development Lab, IBM
Thomas Preudhomme Oct. 24, 2018, 9:37 a.m. | #2
Hi Christophe,

I'm a bit surprised cmse_scan is run at all in your case. Where you
targeting an M profile core?

Regarding sym_hashes[X] being null, under what conditions can a global
symbol have a null hash?

Best regards,

Thomas
On Tue, 23 Oct 2018 at 23:16, Alan Modra <amodra@gmail.com> wrote:
>

> On Tue, Oct 23, 2018 at 04:21:41PM +0200, Christophe Lyon wrote:

> > Hi,

> >

> > While building an ARM FDPIC toolchain with a compiler generating Thumb

> > code, I face a couple of null pointer dereferences in cmse_scan().

> >

> > When browsing ld-uClibc.so.1, all the external symbols have no info in

> > sym_hashes (sym_hashes[X] == NULL), and when handling libgcc_s.so.1

> > for the 2nd time in the same command, sym_hashes == NULL.

> >

> > I don't know why this doesn't happen with a compiler generating Arm

> > code (ie. why the symbol tables are handled differently), but the

> > attached small patch prevents the linker from crashing.

> >

> > OK?

>

> No, this is just papering over the real problem.  You need to find out

> why the sym hashes are not being set up (or are being overwritten).

>

> --

> Alan Modra

> Australia Development Lab, IBM
Alan Modra Oct. 24, 2018, 11:36 a.m. | #3
On Wed, Oct 24, 2018 at 10:37:59AM +0100, Thomas Preudhomme wrote:
> Hi Christophe,

> 

> I'm a bit surprised cmse_scan is run at all in your case. Where you

> targeting an M profile core?

> 

> Regarding sym_hashes[X] being null, under what conditions can a global

> symbol have a null hash?


I sent a little more info to Christophe privately, after I thought a
little more about the problem.  Guess I should have sent it to the
list.

You can have sym_hashes[n] being 0 when you have an as-needed library
that wasn't needed (it's loaded but then unloaded).

Note this elflink.c code:
  if ((elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0)
    {
      unsigned int i;

      /* Restore the symbol table.  */
      old_ent = (char *) old_tab + tabsize;
      memset (elf_sym_hashes (abfd), 0,
	      extsymcount * sizeof (struct elf_link_hash_entry *));

The memset zaps all the sym_hashes, because after restoring the symbol
table to as it was before loading the as-needed library, the symbol
pointers are no longer valid.

The patch I suggest instead of the one Christophe posted is:

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 2c321bbcb6..5adec5e473 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -6449,7 +6449,8 @@ elf32_arm_size_stubs (bfd *output_bfd,
 	  asection *section;
 	  Elf_Internal_Sym *local_syms = NULL;
 
-	  if (!is_arm_elf (input_bfd))
+	  if (!is_arm_elf (input_bfd)
+	      || (elf_dyn_lib_class (input_bfd) & DYN_AS_NEEDED) != 0)
 	    continue;
 
 	  num_a8_relocs = 0;

-- 
Alan Modra
Australia Development Lab, IBM
Christophe Lyon Oct. 24, 2018, 12:49 p.m. | #4
On Wed, 24 Oct 2018 at 13:36, Alan Modra <amodra@gmail.com> wrote:
>

> On Wed, Oct 24, 2018 at 10:37:59AM +0100, Thomas Preudhomme wrote:

> > Hi Christophe,

> >

> > I'm a bit surprised cmse_scan is run at all in your case. Where you

> > targeting an M profile core?

> >

> > Regarding sym_hashes[X] being null, under what conditions can a global

> > symbol have a null hash?

>

> I sent a little more info to Christophe privately, after I thought a

> little more about the problem.  Guess I should have sent it to the

> list.

>

> You can have sym_hashes[n] being 0 when you have an as-needed library

> that wasn't needed (it's loaded but then unloaded).

>

> Note this elflink.c code:

>   if ((elf_dyn_lib_class (abfd) & DYN_AS_NEEDED) != 0)

>     {

>       unsigned int i;

>

>       /* Restore the symbol table.  */

>       old_ent = (char *) old_tab + tabsize;

>       memset (elf_sym_hashes (abfd), 0,

>               extsymcount * sizeof (struct elf_link_hash_entry *));

>

> The memset zaps all the sym_hashes, because after restoring the symbol

> table to as it was before loading the as-needed library, the symbol

> pointers are no longer valid.

>

> The patch I suggest instead of the one Christophe posted is:

>

> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c

> index 2c321bbcb6..5adec5e473 100644

> --- a/bfd/elf32-arm.c

> +++ b/bfd/elf32-arm.c

> @@ -6449,7 +6449,8 @@ elf32_arm_size_stubs (bfd *output_bfd,

>           asection *section;

>           Elf_Internal_Sym *local_syms = NULL;

>

> -         if (!is_arm_elf (input_bfd))

> +         if (!is_arm_elf (input_bfd)

> +             || (elf_dyn_lib_class (input_bfd) & DYN_AS_NEEDED) != 0)

>             continue;

>

>           num_a8_relocs = 0;

>


Thank you very much Alan for your time, explanations, and patch: I can
confirm it works for me. Can you commit it?

> --

> Alan Modra

> Australia Development Lab, IBM

Patch

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 9c61181..6ea348b 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -5939,7 +5939,16 @@  cmse_scan (bfd *input_bfd, struct elf32_arm_link_hash_table *htab,
 	}
       else
 	{
+	  /* No hash table, stop iterating.  */
+	  if (sym_hashes == NULL)
+	    break;
+
 	  cmse_hash = elf32_arm_hash_entry (sym_hashes[i - ext_start]);
+
+	  /* Avoid dereferencing if info is not present.  */
+	  if (cmse_hash == NULL)
+	    continue;
+
 	  sym_name = (char *) cmse_hash->root.root.root.string;
 
 	  /* Not a special symbol.  */