Subject: Re: Infinite loop + memory leak in annotate_refs_with_symref_info

From: Taylor Blau <[email-masked]>
Date: Tue, 11 Apr 2023 16:25:13 -0400

Hi Jonas,

On Tue, Apr 11, 2023 at 10:53:59PM +0300, Jonas Haag wrote:
> Hello!
>
> There is an infinite loop with an accompanying memory leak in
> annotate_refs_with_symref_info that was introduced in Git 2.28 (I
> think in commit 2c6a403: “connect: add function to parse multiple v1
> capability values”).

I'm not familiar with Klaus and don't have it installed, but a couple of
questions: were you able to reproduce this result with any other forges
or tools, and were you able to confirm that 2c6a403 is the culprit via a
bisection?

In case the answer to the latter question is "yes", I cc'd brian carlson
on this thread, since they are the original author of that patch.

> I believe the bug in Git is in connect.c, function
> parse_feature_value, in the updating of `*offset`: It doesn’t seem to
> take into account that `feature_list` has already been offset by
> `*offset`. I believe the update needs to use `*offset +=` instead of
> `*offset =`. When I make this change, the infinite loop seems to go
> away, and cloning via Klaus/Dulwich will fail with “invalid index-pack
> output”. Cloning from github.com works, although I’m not sure if
> that’s a relevant smoke test in this case.

I'm not sure I understand. Looking at the relevant bits in
connect.c::parse_feature_value(), it all seems correct to me, since the
beginning of `feature_list` is adjusted by the current value of
`*offset`.

> [1] https://github.com/jonashaag/klaus

Thanks,
Taylor