Re: [PATCH] revision: fix missing null for freed memory
Show 31 quoted lines
>
> Le 11 févr. 2025 à 16:29, Jeff King <peff@peff.net> a écrit :
>
> On Tue, Feb 11, 2025 at 03:22:28PM -0500, D. Ben Knoble wrote:
>
>> 2.{30,35}.0 fails to recognize --no-graph, so I checked "git log --grep no-graph
>> origin/master" with "git describe --contains" and decided that 2.36.0 was first
>> release recognizing --no-graph, but it didn't build for me (possibly an issue on
>> my end). I got 2.37.0 built, and it was "good," so that's where I started.
>>
>> Here's my "bisect run" script.
>>
>> #! /bin/sh -x
>> make || exit 125
>> # segfault has exit >128
>> ./bin-wrappers/git --no-pager log -2 --graph --no-graph --patch
>> --cc || exit 1
>
> I don't think this is quite enough. The problem is a use-after-free, so
> the behavior is undefined. Depending on whether that heap block is
> reused, it might work just fine, or output garbage data, or segfault.
>
> I'd have _thought_ it would usually just segfault, but it almost always
> just output garbage for me. Building with:
>
> make SANITIZE=address,undefined
>
> is a good way to get reliable results for this kind of memory error.
> Doing that shows that v2.37.0 is actually bad. And bisecting shows that
> it has been broken since 087c745833 (log: add a --no-graph option,
> 2022-02-11), which is not too surprising.Ah, fun, that’s more like what I was expecting. And thanks for the advice!
Show 16 quoted lines
>
>> The --cc is important, since this repro logs from where the bisect is! Without
>> it, if the head commits are both merges (likely), the repro will accidentally
>> mark the commit as good when looking further for a commit with a patch will
>> fail. Omitting -2 might work, too, but that makes "git log" take longer.
>
> I've also run into non-determinism when bisecting like this, because my
> test command depends on the value of HEAD. The best solution here is to
> just feed a stable tip to git-log. I bisected on:
>
> git log --graph --no-graph --patch origin >/dev/null
>
> (I didn't need "-2" because good commits failed with "unrecognized
> argument" and bad ones were killed by ASan immediately ;) ).
>
> -Peff
Ben Knoble <ben.knoble@gmail.com>