RE: [patch] Fix crash in read_pe_exported_syms

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

  I am not in the position of approving the patch,
but I must confess that this error is probably due to 
the change I committed for this file (rev 1.19):
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/coff-pe-read.c?cvsroot=src

  I still didn't really get a fully correct picture of the correct way to handle 
the cleanups, but I think that your analysis is correct and that this patch should
be approved by a global maintainer.

  This error probably explains partly the long thread about
crashes that we got with my patch for a  while and that retarded its inclusion...
We probably fixed only part of the issues...

  Thanks for finding this out,
and sorry for the troubles.

Pierre Muller

PS: It would also be better that this goes in before branching 7.6!

 

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Corinna Vinschen
> Envoyé : samedi 2 mars 2013 12:02
> À : gdb-patches@sourceware.org
> Objet : [patch] Fix crash in read_pe_exported_syms
> 
> Hi,
> 
> when running GDB from current CVS on a PE/COFF target, and if this
> target has no debug symbols, nor any exported symbols, then GDB crashes
> with a SEGV in the first do_cleanup called from coff_symfile_read.
> 
> The reason is that read_pe_exported_syms creates two cleanup handlers,
> one of them referring to a symbol on the local stack:
> 
>   struct read_pe_section_data *section_data;
>   [...]
>   section_data = xzalloc (...)
>   make_cleanup (free_current_contents, &section_data);
> 
> but then returns from the function early in three different scenarios
> without calling do_cleanup.  The subsequent do_cleanup call in
> coff_symfile_read now tries to dereference from an invalid stack address
> and ultimately crashes.
> 
> Below is a patch.  Ok to apply?
> 
> 
> Thanks,
> Corinna
> 
> 
> 	* coff-pe-read.c (read_pe_exported_syms): Don't return without
> 	calling do_cleanup.
> 
> 
> Index: coff-pe-read.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/coff-pe-read.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 coff-pe-read.c
> --- coff-pe-read.c	1 Jan 2013 06:32:40 -0000	1.23
> +++ coff-pe-read.c	2 Mar 2013 11:00:42 -0000
> @@ -379,7 +379,7 @@ read_pe_exported_syms (struct objfile *o
>        /* This is not a recognized PE format file.  Abort now, because
>  	 the code is untested on anything else.  *FIXME* test on
>  	 further architectures and loosen or remove this test.  */
> -      return;
> +      goto cleanup;
>      }
> 
>    /* Get pe_header, optional header and numbers of export entries.  */
> @@ -392,7 +392,7 @@ read_pe_exported_syms (struct objfile *o
> 
>    if (num_entries < 1)		/* No exports.  */
>      {
> -      return;
> +      goto cleanup;
>      }
>    if (is_pe64)
>      {
> @@ -448,7 +448,7 @@ read_pe_exported_syms (struct objfile *o
>    if (export_size == 0)
>      {
>        /* Empty export table.  */
> -      return;
> +      goto cleanup;
>      }
> 
>    /* Scan sections and store the base and size of the relevant
> @@ -614,6 +614,7 @@ read_pe_exported_syms (struct objfile *o
>      fprintf_unfiltered (gdb_stdlog, _("Finished reading \"%s\", exports
> %ld,"
>  			" forwards %ld, total %ld/%ld.\n"), dll_name, nbnormal,
>  			nbforward, nbnormal + nbforward, nexp);
> +cleanup:
>    /* Discard expdata and section_data.  */
>    do_cleanups (back_to);
>  }
> 
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat