diff mbox series

RFA: Fix potential memory leak in gen-sframe.c

Message ID 87pl073358.fsf@redhat.com
State New
Headers
Series RFA: Fix potential memory leak in gen-sframe.c |

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

Nick Clifton July 28, 2026, 8:54 a.m. UTC 
  Hi Indu,

  Is the patch below OK ?  It updates the sframe_xlate_ctx_cleanup()
  function so that it also zeroes out the other fields in the
  sframe_xlate_ctx structure.

  Disclaimer: I used an AI tool (claude) to locate the problem being
  fixed by this patch, but I wrote the patch myself.  Ie I did not use
  AI to write the patch.

  The problem reported looks like this:

    1. sframe_do_fde() (line 2490) populates xlate_ctx->first_fre with
       allocated FRE entries and sets xlate_ctx->num_xlate_fres > 0.
       
    2. On failure for a signal frame (line 2491-2496),
       sframe_xlate_ctx_cleanup() frees the FRE chain (line 1096) but
       does not null first_fre or reset num_xlate_fres. Then err is
       forced to SFRAME_XLATE_OK.
       
    3. Since err is now OK, sframe_xlate_ctx_finalize() (line 2507)
       copies the dangling first_fre pointer and stale count into the
       output SFrame FDE structure.
       
    4. The FDE with dangling pointer is linked into the output list
       (lines 2508-2509) and its freed data is later written to the
       output .sframe section.

  So, do you agree with this analysis and is the proposed patch a
  sufficient solution ?
  
Cheers
  Nick

Comments

Jens Remus July 29, 2026, 4:37 p.m. UTC | #1 
On 7/28/2026 10:54 AM, Nick Clifton wrote:
> Hi Indu,
> 
>   Is the patch below OK ?  It updates the sframe_xlate_ctx_cleanup()
>   function so that it also zeroes out the other fields in the
>   sframe_xlate_ctx structure.
> 
>   Disclaimer: I used an AI tool (claude) to locate the problem being
>   fixed by this patch, but I wrote the patch myself.  Ie I did not use
>   AI to write the patch.
> 
>   The problem reported looks like this:
> 
>     1. sframe_do_fde() (line 2490) populates xlate_ctx->first_fre with
>        allocated FRE entries and sets xlate_ctx->num_xlate_fres > 0.
>        
>     2. On failure for a signal frame (line 2491-2496),
>        sframe_xlate_ctx_cleanup() frees the FRE chain (line 1096) but
>        does not null first_fre or reset num_xlate_fres. Then err is
>        forced to SFRAME_XLATE_OK.
>        
>     3. Since err is now OK, sframe_xlate_ctx_finalize() (line 2507)
>        copies the dangling first_fre pointer and stale count into the
>        output SFrame FDE structure.
>        
>     4. The FDE with dangling pointer is linked into the output list
>        (lines 2508-2509) and its freed data is later written to the
>        output .sframe section.
> 
>   So, do you agree with this analysis and is the proposed patch a
>   sufficient solution ?

Not Indu, but replying as I was involved in SFrame V3 development:

I can confirm that above causes a segmentation fault and below fixes it.

See my just sent "[PATCH 0/2] gas: sframe: Fix for flefible FDE and new
test":
https://inbox.sourceware.org/binutils/20260729163445.2563223-1-jremus@linux.ibm.com/

> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index b5c8f649bea..6d1cafa63bf 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -1094,6 +1094,9 @@ static void
>  sframe_xlate_ctx_cleanup (struct sframe_xlate_ctx *xlate_ctx)
>  {
>    sframe_row_entry_free (xlate_ctx->first_fre);
> +  xlate_ctx->first_fre = NULL;
> +  xlate_ctx->last_fre = NULL;
> +  xlate_ctx->num_xlate_fres = 0;
>    XDELETE (xlate_ctx->remember_fre);
>    xlate_ctx->remember_fre = NULL;
>    XDELETE (xlate_ctx->cur_fre);
Maybe also reset the flexible FDE flag:

	xlate_ctx->flex_p = false;

Regards,
Jens
Nick Clifton July 31, 2026, 8:08 a.m. UTC | #2 
Hi Jens,

> Maybe also reset the flexible FDE flag:
> 
> 	xlate_ctx->flex_p = false;
Good idea.  I have applied that change with that addition.

Cheers
   Nick
diff mbox series

Patch

diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index b5c8f649bea..6d1cafa63bf 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -1094,6 +1094,9 @@  static void
 sframe_xlate_ctx_cleanup (struct sframe_xlate_ctx *xlate_ctx)
 {
   sframe_row_entry_free (xlate_ctx->first_fre);
+  xlate_ctx->first_fre = NULL;
+  xlate_ctx->last_fre = NULL;
+  xlate_ctx->num_xlate_fres = 0;
   XDELETE (xlate_ctx->remember_fre);
   xlate_ctx->remember_fre = NULL;
   XDELETE (xlate_ctx->cur_fre);