ld/pdb: Avoid double-free on malformed symbol data

Message ID 20260930193657.733-4-oleg.tolmatcev@gmail.com
State New
Headers
Series ld/pdb: Avoid double-free on malformed symbol data |

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-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Oleg Tolmatcev Sept. 30, 2026, 7:36 p.m. UTC
  When handle_debugs_section fails, create_module_info_substream frees
the file arrays for all modules, including the current one.  Do not free
the current array in populate_module_stream as well.

ld/
	* pdb.c (populate_module_stream): Leave mod_source->files cleanup to
	the caller.
---
 ld/pdb.c | 1 -
 1 file changed, 1 deletion(-)
  

Comments

Jan Beulich Oct. 2, 2026, 9:58 a.m. UTC | #1
On 30.09.2026 21:36, Oleg Tolmatcev wrote:
> When handle_debugs_section fails, create_module_info_substream frees
> the file arrays for all modules, including the current one.  Do not free
> the current array in populate_module_stream as well.

Isn't the same true then ...

> --- a/ld/pdb.c
> +++ b/ld/pdb.c
> @@ -3955,7 +3955,6 @@ populate_module_stream (bfd *stream, bfd *mod, uint32_t *sym_byte_size,
>  		{
>  		  free (c13_info);
>  		  free (syms);
> -		  free (mod_source->files);
>  		  free (map);
>  		  return false;
>  		}

... a few lines up from here, when handle_debugt_section() fails? The
freeing there is actually yet more odd, as mod_source isn't passed to
the function (so cannot have changed, at least apparently).

Jan
  
Oleg Tolmatcev Oct. 2, 2026, 5:38 p.m. UTC | #2
пт, 2 окт. 2026 г. в 11:58, Jan Beulich <jbeulich@suse.com>:
>
> On 30.09.2026 21:36, Oleg Tolmatcev wrote:
> > When handle_debugs_section fails, create_module_info_substream frees
> > the file arrays for all modules, including the current one.  Do not free
> > the current array in populate_module_stream as well.
>
> Isn't the same true then ...
>
> > --- a/ld/pdb.c
> > +++ b/ld/pdb.c
> > @@ -3955,7 +3955,6 @@ populate_module_stream (bfd *stream, bfd *mod, uint32_t *sym_byte_size,
> >               {
> >                 free (c13_info);
> >                 free (syms);
> > -               free (mod_source->files);
> >                 free (map);
> >                 return false;
> >               }
>
> ... a few lines up from here, when handle_debugt_section() fails? The
> freeing there is actually yet more odd, as mod_source isn't passed to
> the function (so cannot have changed, at least apparently).
>
> Jan

Yes, good catch, I will send a v2 which removes the other free() too.

Oleg
  

Patch

diff --git a/ld/pdb.c b/ld/pdb.c
index 434bdb197ca..9988fd94f51 100644
--- a/ld/pdb.c
+++ b/ld/pdb.c
@@ -3955,7 +3955,6 @@  populate_module_stream (bfd *stream, bfd *mod, uint32_t *sym_byte_size,
 		{
 		  free (c13_info);
 		  free (syms);
-		  free (mod_source->files);
 		  free (map);
 		  return false;
 		}