[v2] elf: Set this_idx to -1 for invalid relocations

Message ID CAMe9rOoKvk2rF289Ei4mYh5z7afkWS1a9uacY1fu9iZobJ1iSA@mail.gmail.com
State New
Headers
Series [v2] elf: Set this_idx to -1 for invalid relocations |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed

Commit Message

H.J. Lu Aug. 2, 2026, 12:55 a.m. UTC
  On Sun, Aug 2, 2026 at 6:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> When invalid relocation is reported by elf_link_read_relocs_from_section
> called from lang_gc_sections:
>
> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> offset 0x4 in section `.text.get_tls[get_tls]'
>
> the same error is reported again:
>
> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> offset 0x4 in section `.text.get_tls[get_tls]'
>
> from lang_check_relocs since lang_gc_sections error doesn't stop linker.
> Update elf_link_read_relocs_from_section to set this_idx to -1 for invalid
> relocations and return false if this_idx < 0.
>
> PR ld/34448
> * elf-bfd.h (bfd_elf_section_data): Update comments for this_idx.
> * elflink.c (elf_link_read_relocs_from_section): Return false if
> this_idx < 0.  Set this_idx to -1 for invalid relocations.
>

Changes in v2:

1.  Use ELF_SECTION_WITH_INVALID_RELOCATION instead of
-1.
2. Update _bfd_elf_section_from_bfd_section to check this_idx < 0.
3. Call bfd_set_error when this_idx < 0.
  

Comments

Alan Modra Aug. 2, 2026, 11:43 p.m. UTC | #1
On Sun, Aug 02, 2026 at 08:55:00AM +0800, H.J. Lu wrote:
> On Sun, Aug 2, 2026 at 6:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > When invalid relocation is reported by elf_link_read_relocs_from_section
> > called from lang_gc_sections:
> >
> > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > offset 0x4 in section `.text.get_tls[get_tls]'
> >
> > the same error is reported again:
> >
> > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > offset 0x4 in section `.text.get_tls[get_tls]'

Using this_idx as a flag is a bad idea.

I also think that no one should be too concerned about error messages
from fuzzed object files, so if you are going to fix this minor
problem do so in a way that won't potentially break the linker.
  
H.J. Lu Aug. 2, 2026, 11:58 p.m. UTC | #2
On Mon, Aug 3, 2026 at 7:43 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Sun, Aug 02, 2026 at 08:55:00AM +0800, H.J. Lu wrote:
> > On Sun, Aug 2, 2026 at 6:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > When invalid relocation is reported by elf_link_read_relocs_from_section
> > > called from lang_gc_sections:
> > >
> > > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > > offset 0x4 in section `.text.get_tls[get_tls]'
> > >
> > > the same error is reported again:
> > >
> > > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > > offset 0x4 in section `.text.get_tls[get_tls]'
>
> Using this_idx as a flag is a bad idea.
>
> I also think that no one should be too concerned about error messages
> from fuzzed object files, so if you are going to fix this minor
> problem do so in a way that won't potentially break the linker.
>

Then should linker stop when seeing a fuzzed object file, instead of keep
going and crash later?
  
Alan Modra Aug. 3, 2026, 12:51 p.m. UTC | #3
On Mon, Aug 03, 2026 at 07:58:27AM +0800, H.J. Lu wrote:
> On Mon, Aug 3, 2026 at 7:43 AM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Sun, Aug 02, 2026 at 08:55:00AM +0800, H.J. Lu wrote:
> > > On Sun, Aug 2, 2026 at 6:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > When invalid relocation is reported by elf_link_read_relocs_from_section
> > > > called from lang_gc_sections:
> > > >
> > > > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > > > offset 0x4 in section `.text.get_tls[get_tls]'
> > > >
> > > > the same error is reported again:
> > > >
> > > > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > > > offset 0x4 in section `.text.get_tls[get_tls]'
> >
> > Using this_idx as a flag is a bad idea.
> >
> > I also think that no one should be too concerned about error messages
> > from fuzzed object files, so if you are going to fix this minor
> > problem do so in a way that won't potentially break the linker.
> >
> 
> Then should linker stop when seeing a fuzzed object file, instead of keep
> going and crash later?

How is this relevant to a patch about duplicate error messages?
  
H.J. Lu Aug. 3, 2026, 10:20 p.m. UTC | #4
On Mon, Aug 3, 2026 at 8:51 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Aug 03, 2026 at 07:58:27AM +0800, H.J. Lu wrote:
> > On Mon, Aug 3, 2026 at 7:43 AM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Sun, Aug 02, 2026 at 08:55:00AM +0800, H.J. Lu wrote:
> > > > On Sun, Aug 2, 2026 at 6:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > >
> > > > > When invalid relocation is reported by elf_link_read_relocs_from_section
> > > > > called from lang_gc_sections:
> > > > >
> > > > > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > > > > offset 0x4 in section `.text.get_tls[get_tls]'
> > > > >
> > > > > the same error is reported again:
> > > > >
> > > > > ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> > > > > offset 0x4 in section `.text.get_tls[get_tls]'
> > >
> > > Using this_idx as a flag is a bad idea.
> > >
> > > I also think that no one should be too concerned about error messages
> > > from fuzzed object files, so if you are going to fix this minor
> > > problem do so in a way that won't potentially break the linker.
> > >
> >
> > Then should linker stop when seeing a fuzzed object file, instead of keep
> > going and crash later?
>
> How is this relevant to a patch about duplicate error messages?
>

If we don't care too much about fuzzed object files, why doesn't linker
stop when seeing them?
  
Jan Beulich Aug. 4, 2026, 7:42 a.m. UTC | #5
On 04.08.2026 00:20, H.J. Lu wrote:
> On Mon, Aug 3, 2026 at 8:51 PM Alan Modra <amodra@gmail.com> wrote:
>>
>> On Mon, Aug 03, 2026 at 07:58:27AM +0800, H.J. Lu wrote:
>>> On Mon, Aug 3, 2026 at 7:43 AM Alan Modra <amodra@gmail.com> wrote:
>>>>
>>>> On Sun, Aug 02, 2026 at 08:55:00AM +0800, H.J. Lu wrote:
>>>>> On Sun, Aug 2, 2026 at 6:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>
>>>>>> When invalid relocation is reported by elf_link_read_relocs_from_section
>>>>>> called from lang_gc_sections:
>>>>>>
>>>>>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
>>>>>> offset 0x4 in section `.text.get_tls[get_tls]'
>>>>>>
>>>>>> the same error is reported again:
>>>>>>
>>>>>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
>>>>>> offset 0x4 in section `.text.get_tls[get_tls]'
>>>>
>>>> Using this_idx as a flag is a bad idea.
>>>>
>>>> I also think that no one should be too concerned about error messages
>>>> from fuzzed object files, so if you are going to fix this minor
>>>> problem do so in a way that won't potentially break the linker.
>>>>
>>>
>>> Then should linker stop when seeing a fuzzed object file, instead of keep
>>> going and crash later?
>>
>> How is this relevant to a patch about duplicate error messages?
> 
> If we don't care too much about fuzzed object files, why doesn't linker
> stop when seeing them?

How would you (non-heuristically, i.e. entirely reliably) tell a fuzzed object
from a "real" one?

Jan
  
H.J. Lu Aug. 4, 2026, 8:57 a.m. UTC | #6
On Tue, Aug 4, 2026 at 3:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 04.08.2026 00:20, H.J. Lu wrote:
> > On Mon, Aug 3, 2026 at 8:51 PM Alan Modra <amodra@gmail.com> wrote:
> >>
> >> On Mon, Aug 03, 2026 at 07:58:27AM +0800, H.J. Lu wrote:
> >>> On Mon, Aug 3, 2026 at 7:43 AM Alan Modra <amodra@gmail.com> wrote:
> >>>>
> >>>> On Sun, Aug 02, 2026 at 08:55:00AM +0800, H.J. Lu wrote:
> >>>>> On Sun, Aug 2, 2026 at 6:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>>>>
> >>>>>> When invalid relocation is reported by elf_link_read_relocs_from_section
> >>>>>> called from lang_gc_sections:
> >>>>>>
> >>>>>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> >>>>>> offset 0x4 in section `.text.get_tls[get_tls]'
> >>>>>>
> >>>>>> the same error is reported again:
> >>>>>>
> >>>>>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
> >>>>>> offset 0x4 in section `.text.get_tls[get_tls]'
> >>>>
> >>>> Using this_idx as a flag is a bad idea.
> >>>>
> >>>> I also think that no one should be too concerned about error messages
> >>>> from fuzzed object files, so if you are going to fix this minor
> >>>> problem do so in a way that won't potentially break the linker.
> >>>>
> >>>
> >>> Then should linker stop when seeing a fuzzed object file, instead of keep
> >>> going and crash later?
> >>
> >> How is this relevant to a patch about duplicate error messages?
> >
> > If we don't care too much about fuzzed object files, why doesn't linker
> > stop when seeing them?
>
> How would you (non-heuristically, i.e. entirely reliably) tell a fuzzed object
> from a "real" one?
>
> Jan

When linker reaches the following code

      if (nsyms > 0)
        {
          if ((size_t) r_symndx >= nsyms)
            {
              _bfd_error_handler
                /* xgettext:c-format */
                (_("%pB: bad reloc symbol index (%#" PRIx64 " >= %#lx)"
                   " for offset %#" PRIx64 " in section `%pA'"),
                 abfd, (uint64_t) r_symndx, (unsigned long) nsyms,
                 (uint64_t) irela->r_offset, sec);
              elf_section_data (sec)->this_idx
                = ELF_SECTION_WITH_INVALID_RELOCATION;
              bfd_set_error (bfd_error_bad_value);
              return false;
            }
        }
      else if (r_symndx != STN_UNDEF)
        {
          _bfd_error_handler
            /* xgettext:c-format */
            (_("%pB: non-zero symbol index (%#" PRIx64 ")"
               " for offset %#" PRIx64 " in section `%pA'"
               " when the object file has no symbol table"),
             abfd, (uint64_t) r_symndx,
             (uint64_t) irela->r_offset, sec);
          elf_section_data (sec)->this_idx
            = ELF_SECTION_WITH_INVALID_RELOCATION;
          bfd_set_error (bfd_error_bad_value);
          return false;
        }

in elf_link_read_relocs_from_section, the input isn't a valid
ELF object file.   It could be a fuzzed object.
  
Jan Beulich Aug. 4, 2026, 10:25 a.m. UTC | #7
On 04.08.2026 10:57, H.J. Lu wrote:
> On Tue, Aug 4, 2026 at 3:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 04.08.2026 00:20, H.J. Lu wrote:
>>> On Mon, Aug 3, 2026 at 8:51 PM Alan Modra <amodra@gmail.com> wrote:
>>>>
>>>> On Mon, Aug 03, 2026 at 07:58:27AM +0800, H.J. Lu wrote:
>>>>> On Mon, Aug 3, 2026 at 7:43 AM Alan Modra <amodra@gmail.com> wrote:
>>>>>>
>>>>>> On Sun, Aug 02, 2026 at 08:55:00AM +0800, H.J. Lu wrote:
>>>>>>> On Sun, Aug 2, 2026 at 6:18 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>
>>>>>>>> When invalid relocation is reported by elf_link_read_relocs_from_section
>>>>>>>> called from lang_gc_sections:
>>>>>>>>
>>>>>>>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
>>>>>>>> offset 0x4 in section `.text.get_tls[get_tls]'
>>>>>>>>
>>>>>>>> the same error is reported again:
>>>>>>>>
>>>>>>>> ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for
>>>>>>>> offset 0x4 in section `.text.get_tls[get_tls]'
>>>>>>
>>>>>> Using this_idx as a flag is a bad idea.
>>>>>>
>>>>>> I also think that no one should be too concerned about error messages
>>>>>> from fuzzed object files, so if you are going to fix this minor
>>>>>> problem do so in a way that won't potentially break the linker.
>>>>>>
>>>>>
>>>>> Then should linker stop when seeing a fuzzed object file, instead of keep
>>>>> going and crash later?
>>>>
>>>> How is this relevant to a patch about duplicate error messages?
>>>
>>> If we don't care too much about fuzzed object files, why doesn't linker
>>> stop when seeing them?
>>
>> How would you (non-heuristically, i.e. entirely reliably) tell a fuzzed object
>> from a "real" one?
>>
>> Jan
> 
> When linker reaches the following code
> 
>       if (nsyms > 0)
>         {
>           if ((size_t) r_symndx >= nsyms)
>             {
>               _bfd_error_handler
>                 /* xgettext:c-format */
>                 (_("%pB: bad reloc symbol index (%#" PRIx64 " >= %#lx)"
>                    " for offset %#" PRIx64 " in section `%pA'"),
>                  abfd, (uint64_t) r_symndx, (unsigned long) nsyms,
>                  (uint64_t) irela->r_offset, sec);
>               elf_section_data (sec)->this_idx
>                 = ELF_SECTION_WITH_INVALID_RELOCATION;
>               bfd_set_error (bfd_error_bad_value);
>               return false;
>             }
>         }
>       else if (r_symndx != STN_UNDEF)
>         {
>           _bfd_error_handler
>             /* xgettext:c-format */
>             (_("%pB: non-zero symbol index (%#" PRIx64 ")"
>                " for offset %#" PRIx64 " in section `%pA'"
>                " when the object file has no symbol table"),
>              abfd, (uint64_t) r_symndx,
>              (uint64_t) irela->r_offset, sec);
>           elf_section_data (sec)->this_idx
>             = ELF_SECTION_WITH_INVALID_RELOCATION;
>           bfd_set_error (bfd_error_bad_value);
>           return false;
>         }
> 
> in elf_link_read_relocs_from_section, the input isn't a valid
> ELF object file.   It could be a fuzzed object.

Right. And whether to continue after such an error is indeed a policy
question. I'm inclined to say that upon encountering broken input, a
fatal error is pretty reasonable.

Jan
  

Patch

From 1629249aa6f4fc211e4008249ee534dd617dac62 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 2 Aug 2026 06:09:12 +0800
Subject: [PATCH v2] elf: Set this_idx to -1 for invalid relocations

When invalid relocation is reported by elf_link_read_relocs_from_section
called from lang_gc_sections:

ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for offset 0x4 in section `.text.get_tls[get_tls]'

the same error is reported again:

ld: pr34448-bug_18.o: bad reloc symbol index (0xf2000005 >= 0x13) for offset 0x4 in section `.text.get_tls[get_tls]'

from lang_check_relocs since lang_gc_sections error doesn't stop linker.
Update elf_link_read_relocs_from_section to set this_idx to -1 for invalid
relocations and return false if this_idx < 0.

	PR ld/34448
	* elf-bfd.h (ELF_SECTION_WITH_INVALID_RELOCATION): New.
	(bfd_elf_section_data): Update comments for this_idx.
	* elf.c (_bfd_elf_section_from_bfd_section): Return SHN_BAD if
	this_idx < 0.
	* elflink.c (elf_link_read_relocs_from_section): Return false if
	this_idx < 0.  Set this_idx to ELF_SECTION_WITH_INVALID_RELOCATION
	for invalid relocations.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 bfd/elf-bfd.h |  6 +++++-
 bfd/elf.c     | 13 ++++++++++---
 bfd/elflink.c | 12 ++++++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index b354f77f421..736b6e4120e 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1889,7 +1889,11 @@  struct bfd_elf_section_data
      with this section, if any.  */
   struct bfd_elf_section_reloc_data rel, rela;
 
-  /* The ELF section number of this section.  */
+  /* The ELF section number of this section.  A value of < 0 means that
+     some contents in the section are invalid:
+     -(1 << 0): Section with invalid relocation.
+   */
+#define ELF_SECTION_WITH_INVALID_RELOCATION	-(1 << 0)
   int this_idx;
 
   /* Used by the backend linker when generating a shared library to
diff --git a/bfd/elf.c b/bfd/elf.c
index ea127919b9f..bda31ca2fff 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -7303,9 +7303,16 @@  _bfd_elf_section_from_bfd_section (bfd *abfd, struct bfd_section *asect)
   elf_backend_data *bed;
   unsigned int sec_index;
 
-  if (elf_section_data (asect) != NULL
-      && elf_section_data (asect)->this_idx != 0)
-    return elf_section_data (asect)->this_idx;
+  if (elf_section_data (asect) != NULL)
+    {
+      if (elf_section_data (asect)->this_idx > 0)
+	return elf_section_data (asect)->this_idx;
+      else if (elf_section_data (asect)->this_idx < 0)
+	{
+	  bfd_set_error (bfd_error_nonrepresentable_section);
+	  return SHN_BAD;
+	}
+    }
 
   if (bfd_is_abs_section (asect))
     sec_index = SHN_ABS;
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 0af9837a28c..29157804339 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -2808,6 +2808,14 @@  elf_link_read_relocs_from_section (bfd *abfd,
   size_t nsyms;
   void *external_relocs = *external_relocs_addr;
 
+  /* Return false if some relocations in the section are invalid during
+     the previous call.  */
+  if (elf_section_data (sec)->this_idx < 0)
+    {
+      bfd_set_error (bfd_error_bad_value);
+      return false;
+    }
+
   /* Position ourselves at the start of the section.  */
   if (bfd_seek (abfd, shdr->sh_offset, SEEK_SET) != 0)
     return false;
@@ -2858,6 +2866,8 @@  elf_link_read_relocs_from_section (bfd *abfd,
 		   " for offset %#" PRIx64 " in section `%pA'"),
 		 abfd, (uint64_t) r_symndx, (unsigned long) nsyms,
 		 (uint64_t) irela->r_offset, sec);
+	      elf_section_data (sec)->this_idx
+		= ELF_SECTION_WITH_INVALID_RELOCATION;
 	      bfd_set_error (bfd_error_bad_value);
 	      return false;
 	    }
@@ -2871,6 +2881,8 @@  elf_link_read_relocs_from_section (bfd *abfd,
 	       " when the object file has no symbol table"),
 	     abfd, (uint64_t) r_symndx,
 	     (uint64_t) irela->r_offset, sec);
+	  elf_section_data (sec)->this_idx
+	    = ELF_SECTION_WITH_INVALID_RELOCATION;
 	  bfd_set_error (bfd_error_bad_value);
 	  return false;
 	}
-- 
2.55.0