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

Re: [BUG] git stash show -p with invalid option aborts with double-free in show_stash() (strvec_clear)

From
Jeff King <peff@peff.net>
Date
Sep 19, 2025, 19:56 UTC
Message-ID
<20250919195626.GA2750626@coredump.intra.peff.net>
In-Reply-To
<xmqqbjn65mxa.fsf@gitster.g>
On Fri, Sep 19, 2025 at 11:15:13AM -0700, Junio C Hamano wrote:
Show 15 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > But really, I do not know that we need to NULL the whole thing. We have
> > given the caller the reduced argc. The only argv invariant we are
> > violating is that argv[argc] should be NULL (or in this case,
> > argv[left]). Anything after argv+left should be considered
> > uninitialized. So just:
> >
> >   argv[left] = NULL;
> >
> > would be enough, I'd think.
> 
> Even when strvec was passed and more than one element was eaten
> after parsing?  strvec_clear() goes by .nr not stopping at the first
> NULL IIRC.

Yes, there is a big can of worms here. ;) It turns out that many spots with strvecs were relying on leaving these entries untouched, and so setting any of them to NULL causes leaks.

I think I've got it mostly worked out, but that's why I haven't sent patches yet. Stay tuned.

-Peff
Previous: Junio C HamanoNext: Jeff King
Message 9 of 26 in “[BUG] git stash show -p with invalid option aborts with double-free in show_stash() (strvec_clear)”
  1. Lauri NiskanenSep 19, 2025
  2. Kristoffer HaugsbakkSep 19, 2025
  3. Junio C HamanoSep 19, 2025
  4. Jeff KingSep 19, 2025
  5. Junio C HamanoSep 19, 2025
  6. Junio C HamanoSep 19, 2025
  7. Jeff KingSep 19, 2025
  8. Junio C HamanoSep 19, 2025
  9. Jeff KingSep 19, 2025
  10. 0/6 fixing double-frees and leaks via setup_revisions()Jeff King, Sep 19, 2025
  11. 1/6 stash: tell setup_revisions() to free our allocated stringsJeff King, Sep 19, 2025
  12. Junio C HamanoSep 22, 2025
  13. Jeff KingSep 22, 2025
  14. Junio C HamanoSep 22, 2025
  15. Jeff KingSep 22, 2025
  16. Junio C HamanoSep 22, 2025
  17. Jeff KingSep 23, 2025
  18. 2/6 revision: manage memory ownership of argv in setup_revisions()Jeff King, Sep 19, 2025
  19. 3/6 revision: add wrapper to setup_revisions() from a strvecJeff King, Sep 19, 2025
  20. Eric SunshineSep 20, 2025
  21. Jeff KingSep 20, 2025
  22. 4/6 treewide: use setup_revisions_from_strvec() when we have a strvecJeff King, Sep 19, 2025
  23. 5/6 treewide: pass strvecs around for setup_revisions_from_strvec()Jeff King, Sep 19, 2025
  24. Jeff KingSep 19, 2025
  25. 6/6 revision: retain argv NULL invariant in setup_revisions()Jeff King, Sep 19, 2025
  26. Jeff KingSep 19, 2025

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.