git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 10/9] ref-filter: fix leak with unterminated %(if) atoms

From
Jeff King <peff@peff.net>
Date
Sep 10, 2024, 07:12 UTC
Message-ID
<20240910071222.GA1466029@coredump.intra.peff.net>
In-Reply-To
<4faf815b780218769520561ecf3abca384a2ee6c.1725951400.git.ps@pks.im>
On Tue, Sep 10, 2024 at 08:57:15AM +0200, Patrick Steinhardt wrote:
Show 14 quoted lines
> When parsing `%(if)` atoms we expect a few other atoms to exist to
> complete it, like `%(then)` and `%(end)`. Whether or not we have seen
> these other atoms is tracked in an allocated `if_then_else` structure,
> which gets free'd by the `if_then_else_handler()` once we have parsed
> the complete conditional expression.
> 
> This results in a memory leak when the `%(if)` atom is not terminated
> correctly and thus incomplete. We never end up executing its handler and
> thus don't end up freeing the structure.
> 
> Plug this memory leak by introducing a new `at_end_data_free` callback
> function. If set, we'll execute it in `pop_stack_element()` and pass it
> the `at_end_data` variable with the intent to free its state. Wire it up
> for the `%(if)` atom accordingly.
Ah, thanks for explaining. The patch makes much more sense now. :)
In particular, this:
Show 9 quoted lines
> @@ -1169,6 +1170,8 @@ static void pop_stack_element(struct ref_formatting_stack **stack)
>  	if (prev)
>  		strbuf_addbuf(&prev->output, &current->output);
>  	strbuf_release(&current->output);
> +	if (current->at_end_data_free)
> +		current->at_end_data_free(current->at_end_data);
>  	free(current);
>  	*stack = prev;
>  }
which frees on pop, replaces the manual:
Show 6 quoted lines
> @@ -1228,15 +1231,13 @@ static void if_then_else_handler(struct ref_formatting_stack **stack)
>  	}
>  
>  	*stack = cur;
> -	free(if_then_else);
>  }
free that was happening in the success case.
I think putting this on top of my series makes sense.
-Peff
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 22 of 27 in “ref-filter %(trailer) fixes”
  1. 0/9 ref-filter %(trailer) fixesJeff King, Sep 9, 2024
  2. 1/9 t6300: drop newline from wrapped test titleJeff King, Sep 9, 2024
  3. 2/9 ref-filter: avoid extra copies of payload/signatureJeff King, Sep 9, 2024
  4. Patrick SteinhardtSep 10, 2024
  5. Jeff KingSep 10, 2024
  6. 3/9 ref-filter: strip signature when parsing tag trailersJeff King, Sep 9, 2024
  7. Patrick SteinhardtSep 10, 2024
  8. Jeff KingSep 10, 2024
  9. 4/9 ref-filter: drop useless cast in trailers_atom_parser()Jeff King, Sep 9, 2024
  10. 5/9 ref-filter: store ref_trailer_buf data per-atomJeff King, Sep 9, 2024
  11. Patrick SteinhardtSep 10, 2024
  12. 6/9 ref-filter: fix leak of %(trailers) "argbuf"Jeff King, Sep 9, 2024
  13. Patrick SteinhardtSep 10, 2024
  14. Jeff KingSep 10, 2024
  15. 7/9 ref-filter: fix leak with %(describe) argumentsJeff King, Sep 9, 2024
  16. 8/9 ref-filter: fix leak when formatting %(push:remoteref)Jeff King, Sep 9, 2024
  17. Patrick SteinhardtSep 10, 2024
  18. 9/9 ref-filter: add ref_format_clear() functionJeff King, Sep 9, 2024
  19. Patrick SteinhardtSep 10, 2024
  20. Jeff KingSep 10, 2024
  21. 10/9 ref-filter: fix leak with unterminated %(if) atomsPatrick Steinhardt, Sep 10, 2024
  22. Jeff KingSep 10, 2024
  23. Junio C HamanoSep 10, 2024
  24. Patrick SteinhardtSep 12, 2024
  25. Jeff KingSep 12, 2024
  26. Patrick SteinhardtSep 12, 2024
  27. Junio C HamanoSep 12, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.