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

From: Derrick Stolee <stolee@gmail.com>
Date: Oct 6, 2021, 14:19 UTC
Message-ID: <96320021-66dc-966f-2222-25d308247596@gmail.com>
In-Reply-To: <cfd6f82d-01c3-d6be-a535-76255e599c89@gmail.com>

On 10/6/21 10:01 AM, Phillip Wood wrote:

Show 13 quoted lines

> Hi Stolee
> 
> On 06/10/2021 12:20, Derrick Stolee wrote:
>> In particular, prime_cache_tree_rec() does not stop at the sparse-checkout
>> cone, so the cache tree is the full size at that point.
>>
>> When the verify_one() method reaches these nodes that are outside of the
>> cone, index_name_pos() triggers the index expansion in a way that the
>> cache-tree that is restricted to the sparse-checkout cone does not.
>>
>> Hopefully that helps clear up _why_ this happens.
> 
> It does thanks - we end up with a full cache tree but a sparse index

That's a short-and-sweet way to describe it.

Show 9 quoted lines

>> There is a remaining issue that "git rebase --apply" will be a lot slower
>> than "git rebase --merge" because of this construction of a cache-tree
>> that is much larger than necessary.
>>
>> I will make note of this as a potential improvement for the future.
> 
> I think I'm going to remove the call to prime_cache_tree(). Correct me if I'm wrong but as I understand it unpack_trees() updates the cache tree so the call to prime_cache_tree() is not needed (I think it was copied from builtin/rebase.c which does need to call prime_cache_tree() if it has updated a few paths rather than the whole top-level tree). In any case I've just noticed that one of Victoria's patches[1] looks like it fixes prime_cache_tree() with a sparse index.
> 
> [1] https://lore.kernel.org/git/78cd85d8dcc790251ce8235e649902cf6adf091a.1633440057.git.gitgitgadget@gmail.com/

Of course it does! I'm losing track of all the ongoing work in the sparse index as I've been distracted and out of it for a while. It's in good hands.

Thanks, -Stolee