elf: Check corrupt VTENTRY relocation addend

  Check corrupt VTENTRY relocation addend to avoid linker crash on

  h->u2.vtable->used[addend >> log_file_align] = true;

PR ld/33452
* elflink.c (bfd_elf_gc_record_vtentry): Return false if VTENTRY
relocation addend is too large.
  

On Tue, Sep 23, 2025 at 10:16:50AM +0800, H.J. Lu wrote:
> Check corrupt VTENTRY relocation addend to avoid linker crash on
> 
>   h->u2.vtable->used[addend >> log_file_align] = true;
> 
> PR ld/33452
> * elflink.c (bfd_elf_gc_record_vtentry): Return false if VTENTRY
> relocation addend is too large.
> 
> 
> -- 
> H.J.

> From d604dda058572d6f12937e6499df2558a0ca896a Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Tue, 23 Sep 2025 05:35:09 +0800
> Subject: [PATCH 2/4] elf: Check corrupt VTENTRY relocation addend
> 
> Check corrupt VTENTRY relocation addend to avoid linker crash on
> 
>   h->u2.vtable->used[addend >> log_file_align] = true;
> 
> 	PR ld/33452
> 	* elflink.c (bfd_elf_gc_record_vtentry): Return false if VTENTRY
> 	relocation addend is too large.
> 
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  bfd/elflink.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index 66982f82b94..f366e8f569e 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -14895,6 +14895,15 @@ bfd_elf_gc_record_vtentry (bfd *abfd, asection *sec,
>  	}
>        size = (size + file_align - 1) & -file_align;
>  
> +      if (addend > size)

For this to happen you must have had an overflow in prior expressions
calculating size from addend, I think.  Perhaps it might be better to
limit vtentry reloc addends to something a lot smaller than -8ul
(which I think is effectively what your patch does), to prevent insane
vtable memory allocation.  What is a reasonable limit to c++ vtable
size?

> +	{
> +	  /* xgettext:c-format */
> +	  _bfd_error_handler (_("%pB: section '%pA': corrupt VTENTRY relocation"),
> +			      abfd, sec);
> +	  bfd_set_error (bfd_error_bad_value);
> +	  return false;
> +	}
> +
>        /* Allocate one extra entry for use as a "done" flag for the
>  	 consolidation pass.  */
>        bytes = ((size >> log_file_align) + 1) * sizeof (bool);
> -- 
> 2.51.0
>
  

On 23.09.2025 13:57, Alan Modra wrote:
> On Tue, Sep 23, 2025 at 10:16:50AM +0800, H.J. Lu wrote:
>> --- a/bfd/elflink.c
>> +++ b/bfd/elflink.c
>> @@ -14895,6 +14895,15 @@ bfd_elf_gc_record_vtentry (bfd *abfd, asection *sec,
>>  	}
>>        size = (size + file_align - 1) & -file_align;
>>  
>> +      if (addend > size)
> 
> For this to happen you must have had an overflow in prior expressions
> calculating size from addend, I think.  Perhaps it might be better to
> limit vtentry reloc addends to something a lot smaller than -8ul
> (which I think is effectively what your patch does), to prevent insane
> vtable memory allocation.  What is a reasonable limit to c++ vtable
> size?

Can we really build in a heuristic like that? Arbitrarily complex class
hierarchies can have arbitrarily large vtables, I suppose.

Jan
  

On Tue, Sep 23, 2025 at 04:42:51PM +0200, Jan Beulich wrote:
> On 23.09.2025 13:57, Alan Modra wrote:
> > On Tue, Sep 23, 2025 at 10:16:50AM +0800, H.J. Lu wrote:
> >> --- a/bfd/elflink.c
> >> +++ b/bfd/elflink.c
> >> @@ -14895,6 +14895,15 @@ bfd_elf_gc_record_vtentry (bfd *abfd, asection *sec,
> >>  	}
> >>        size = (size + file_align - 1) & -file_align;
> >>  
> >> +      if (addend > size)
> > 
> > For this to happen you must have had an overflow in prior expressions
> > calculating size from addend, I think.  Perhaps it might be better to
> > limit vtentry reloc addends to something a lot smaller than -8ul
> > (which I think is effectively what your patch does), to prevent insane
> > vtable memory allocation.  What is a reasonable limit to c++ vtable
> > size?
> 
> Can we really build in a heuristic like that? Arbitrarily complex class
> hierarchies can have arbitrarily large vtables, I suppose.

We can in this case.  I doubt anyone would notice if gas
.vtable_inherit and .vtable_entry disappeared along with all the
support for VTINHERIT and VTENTRY relocs.  See gcc commit a0c8285b03a4.
I am committing the following patch.


PR 33452 SEGV in bfd_elf_gc_record_vtentry

Limit addends on vtentry relocs, otherwise ld might attempt to
allocate a stupidly large array.  This also fixes the expression
overflow leading to pr33452.  A vtable of 33M entries on a 64-bit
host is surely large enough, especially considering that VTINHERIT
and VTENTRY relocations are to support -fvtable-gc that disappeared
from gcc over 20 years ago.

	PR ld/33452
	* elflink.c (bfd_elf_gc_record_vtentry): Sanity check addend.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 54f0d6e957e..0a0456177c2 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -14865,7 +14865,7 @@ bfd_elf_gc_record_vtentry (bfd *abfd, asection *sec,
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   unsigned int log_file_align = bed->s->log_file_align;
 
-  if (!h)
+  if (!h || addend > 1u << 28)
     {
       /* xgettext:c-format */
       _bfd_error_handler (_("%pB: section '%pA': corrupt VTENTRY entry"),
  

On 25.09.2025 02:39, Alan Modra wrote:
> On Tue, Sep 23, 2025 at 04:42:51PM +0200, Jan Beulich wrote:
>> On 23.09.2025 13:57, Alan Modra wrote:
>>> On Tue, Sep 23, 2025 at 10:16:50AM +0800, H.J. Lu wrote:
>>>> --- a/bfd/elflink.c
>>>> +++ b/bfd/elflink.c
>>>> @@ -14895,6 +14895,15 @@ bfd_elf_gc_record_vtentry (bfd *abfd, asection *sec,
>>>>  	}
>>>>        size = (size + file_align - 1) & -file_align;
>>>>  
>>>> +      if (addend > size)
>>>
>>> For this to happen you must have had an overflow in prior expressions
>>> calculating size from addend, I think.  Perhaps it might be better to
>>> limit vtentry reloc addends to something a lot smaller than -8ul
>>> (which I think is effectively what your patch does), to prevent insane
>>> vtable memory allocation.  What is a reasonable limit to c++ vtable
>>> size?
>>
>> Can we really build in a heuristic like that? Arbitrarily complex class
>> hierarchies can have arbitrarily large vtables, I suppose.
> 
> We can in this case.  I doubt anyone would notice if gas
> .vtable_inherit and .vtable_entry disappeared along with all the
> support for VTINHERIT and VTENTRY relocs.  See gcc commit a0c8285b03a4.
> I am committing the following patch.
> 
> 
> PR 33452 SEGV in bfd_elf_gc_record_vtentry
> 
> Limit addends on vtentry relocs, otherwise ld might attempt to
> allocate a stupidly large array.  This also fixes the expression
> overflow leading to pr33452.  A vtable of 33M entries on a 64-bit
> host is surely large enough, especially considering that VTINHERIT
> and VTENTRY relocations are to support -fvtable-gc that disappeared
> from gcc over 20 years ago.

Oh, I didn't know this was only historic functionality.

Jan