[v2,1/1] gdb: Fix segfault with a big .dynamic section size

  Consider a binary with an erroneous size of the .dynamic section:

$ readelf a.out
...
  [24] .dynamic          DYNAMIC          0000000000004c20  00003c20
       000000fffffffa40  0000000000000010  WA       7     0     8
...

This binary causes a segfault in GDB.  GDB is trying to write the .dynamic
section into memory allocated on the stack with alloca().  However, the
allocation silently fails and the subsequent access to the memory is
causing the segfault. (On my node at least.)

Stack allocation is a bad idea for something of variable size that GDB has
no control over.  So I changed the code to heap allocation.

In addition, I changed the type of sect_size to the type that bfd actually
returns.

There should be no user visible change after this.
---
 gdb/solib.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
  

> -----Original Message-----
> From: Felix Willgerodt <felix.willgerodt@intel.com>
> Sent: Dienstag, 14. November 2023 16:42
> To: gdb-patches@sourceware.org; tom@tromey.com; keiths@redhat.com
> Subject: [PATCH v2 1/1] gdb: Fix segfault with a big .dynamic section size
> 
> Consider a binary with an erroneous size of the .dynamic section:
> 
> $ readelf a.out
> ...
>   [24] .dynamic          DYNAMIC          0000000000004c20  00003c20
>        000000fffffffa40  0000000000000010  WA       7     0     8
> ...
> 
> This binary causes a segfault in GDB.  GDB is trying to write the .dynamic
> section into memory allocated on the stack with alloca().  However, the
> allocation silently fails and the subsequent access to the memory is
> causing the segfault. (On my node at least.)
> 
> Stack allocation is a bad idea for something of variable size that GDB has
> no control over.  So I changed the code to heap allocation.
> 
> In addition, I changed the type of sect_size to the type that bfd actually
> returns.
> 
> There should be no user visible change after this.
> ---
>  gdb/solib.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/solib.c b/gdb/solib.c
> index b9fb911a810..d055bea562d 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -1501,7 +1501,8 @@ int
>  gdb_bfd_scan_elf_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr,
>  			 CORE_ADDR *ptr_addr)
>  {
> -  int arch_size, step, sect_size;
> +  int arch_size, step;
> +  bfd_size_type sect_size;
>    long current_dyntag;
>    CORE_ADDR dyn_ptr, dyn_addr;
>    gdb_byte *bufend, *bufstart, *buf;
> @@ -1546,7 +1547,8 @@ gdb_bfd_scan_elf_dyntag (const int desired_dyntag,
> bfd *abfd, CORE_ADDR *ptr,
>    /* Read in .dynamic from the BFD.  We will get the actual value
>       from memory later.  */
>    sect_size = bfd_section_size (sect);
> -  buf = bufstart = (gdb_byte *) alloca (sect_size);
> +  gdb::byte_vector buffer (sect_size);
> +  buf = bufstart = buffer.data ();
>    if (!bfd_get_section_contents (abfd, sect,
>  				 buf, 0, sect_size))
>      return 0;
> --
> 2.34.1
> 

Hi Tom,

Sorry to ping so early again, but I see that you approved v1 here:
https://sourceware.org/pipermail/gdb-patches/2023-November/204074.html

Since I didn't change much from v1 (commit msg and the variable type),
can I just merge this v2?

Thanks,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

>>>>> Willgerodt, Felix <felix.willgerodt@intel.com> writes:

> Sorry to ping so early again, but I see that you approved v1 here:
> https://sourceware.org/pipermail/gdb-patches/2023-November/204074.html

> Since I didn't change much from v1 (commit msg and the variable type),
> can I just merge this v2?

Yes, sorry about any confusion.

Tom
  

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Montag, 20. November 2023 17:22
> To: Willgerodt, Felix <felix.willgerodt@intel.com>
> Cc: gdb-patches@sourceware.org; tom@tromey.com; keiths@redhat.com
> Subject: Re: [PATCH v2 1/1] gdb: Fix segfault with a big .dynamic section size
> 
> >>>>> Willgerodt, Felix <felix.willgerodt@intel.com> writes:
> 
> > Sorry to ping so early again, but I see that you approved v1 here:
> > https://sourceware.org/pipermail/gdb-patches/2023-November/204074.html
> 
> > Since I didn't change much from v1 (commit msg and the variable type),
> > can I just merge this v2?
> 
> Yes, sorry about any confusion.
> 
> Tom

Thanks, I just pushed this.

Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928