Re: [PATCH] delta-islands: fix segfault when freeing island marks

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 20, 2023, 07:38 UTC
Message-ID: <Y/MjdRq2dTbqh0a/@ncase>
In-Reply-To: <xmqqfsb5qzqj.fsf@gitster.g>

On Thu, Feb 16, 2023 at 09:36:20AM -0800, Junio C Hamano wrote:

Show 17 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > In 647982bb71 (delta-islands: free island_marks and bitmaps, 2023-02-03)
> > we have introduced logic to free `island_marks` in order to reduce heap
> > memory usage in git-pack-objects(1). This commit is causing segfaults in
> > the case where this Git command does not load delta islands at all, e.g.
> > when reading object IDs from standard input. One such crash can be hit
> > when using repacking multi-pack-indices with delta islands enabled.
> >
> > The root cause of this bug is that we unconditionally dereference the
> > `island_marks` variable even in the case where it is a `NULL` pointer,
> > which is fixed by making it conditional. Note that we still leave the
> > logic in place to set the pointer to `-1` to detect use-after-free bugs
> > even when there are no loaded island marks at all.
> > ---
> 
> Missing sign-off?

Oops, right. Please feel free to add it when pulling:

    Signed-off-by: Patrick Steinhardt <ps@pks.im>

If you prefer I'd also be happy to send another iteration of this patch, potentially including a direct test for git-pack-objects(1). I'm still not sure whether it'd be worth it though given that it's implicitly covered by the higher-level test that checks for the segfault by using git-multi-pack-index(1).

Patrick