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

Re: [PATCH] [RFC] sparse index: fix use-after-free bug in cache_tree_verify()

From
Derrick Stolee <stolee@gmail.com>
Date
Oct 6, 2021, 20:43 UTC
Message-ID
<7ef4f68c-8c32-86ea-f8a6-cd673ed281c7@gmail.com>
In-Reply-To
<xmqq1r4yrmhp.fsf@gitster.g>
On 10/6/2021 3:17 PM, Junio C Hamano wrote:
Show 40 quoted lines
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> @@ -907,6 +917,9 @@ void cache_tree_verify(struct repository *r, struct index_state *istate)
>>  
>>  	if (!istate->cache_tree)
>>  		return;
>> -	verify_one(r, istate, istate->cache_tree, &path);
>> +	if (verify_one(r, istate, istate->cache_tree, &path)) {
>> +		strbuf_reset(&path);
>> +		verify_one(r, istate, istate->cache_tree, &path);
>> +	}
>>  	strbuf_release(&path);
>>  }
> 
> This is just a style thing, but I would find it easier to follow if
> it just recursed into itself, i.e.
> 
> -	verify_one(...);
> +	if (verify_one(...))
> +		cache_tree_verify(r, istate);
> 
> or
> 
> -	verify_one(...);
> +	again:
> +	if (verify_one(...))
> +		strbuf_reset(&path);
> +		goto again;
> }	}
> 
> On the other hand, if the new code wants to say "I would retry at
> most once, otherwise there is something wrong in me", then
> 
>> -	verify_one(r, istate, istate->cache_tree, &path);
>> +	if (verify_one(r, istate, istate->cache_tree, &path)) {
>> +		strbuf_reset(&path);
>> +		if (verify_one(r, istate, istate->cache_tree, &path))
>> +			BUG("...");
>> +	}
> 
> would be better.
I'm in favor of this second option.

Thanks, -Stolee

Previous: Junio C HamanoNext: Phillip Wood via GitGitGadget
Message 6 of 26 in “[RFC] sparse index: fix use-after-free bug in cache_tree_verify()”
  1. [RFC] sparse index: fix use-after-free bug in cache_tree_verify()Phillip Wood via GitGitGadget, Oct 6, 2021
  2. Derrick StoleeOct 6, 2021
  3. Phillip WoodOct 6, 2021
  4. Derrick StoleeOct 6, 2021
  5. Junio C HamanoOct 6, 2021
  6. Derrick StoleeOct 6, 2021
  7. sparse index: fix use-after-free bug in cache_tree_verify()Phillip Wood via GitGitGadget, Oct 7, 2021
  8. Derrick StoleeOct 7, 2021
  9. Phillip WoodOct 7, 2021
  10. Derrick StoleeOct 7, 2021
  11. Phillip WoodOct 7, 2021
  12. Derrick StoleeOct 7, 2021
  13. Phillip WoodOct 7, 2021
  14. sparse index: fix use-after-free bug in cache_tree_verify()Phillip Wood via GitGitGadget, Oct 7, 2021
  15. Junio C HamanoOct 7, 2021
  16. Phillip WoodOct 8, 2021
  17. Derrick StoleeOct 8, 2021
  18. Junio C HamanoOct 8, 2021
  19. Phillip WoodOct 14, 2021
  20. Junio C HamanoOct 14, 2021
  21. Bagas SanjayaOct 8, 2021
  22. Phillip WoodOct 14, 2021
  23. sparse index: fix use-after-free bug in cache_tree_verify()Phillip Wood via GitGitGadget, Oct 16, 2021
  24. Junio C HamanoOct 17, 2021
  25. Derrick StoleeOct 17, 2021
  26. Phillip WoodOct 18, 2021

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.