Re: bug report: "git pack-redundant --all" crash in minimize()

On Tue, Dec 15, 2020 at 06:34:53PM +0100, Daniel C. Klauer wrote:

I'm getting the following crash from "git pack-redundant --all" (output
from valgrind):

==14070== Invalid read of size 8
==14070==    at 0x18F165: minimize (pack-redundant.c:399)
==14070==    by 0x18F165: cmd_pack_redundant (pack-redundant.c:622)
==14070==    by 0x1242D3: run_builtin (git.c:444)
==14070==    by 0x1242D3: handle_builtin (git.c:674)
==14070==    by 0x125393: run_argv (git.c:741)
==14070==    by 0x125393: cmd_main (git.c:872)
==14070==    by 0x123E7D: main (common-main.c:52)
==14070==  Address 0x10 is not stack'd, malloc'd or (recently) free'd

Commands to reproduce:

mkdir new
cd new
git init
touch foo.txt
git add foo.txt
git commit -m "first commit"
git gc
git pack-redundant --all

Thanks for an easy reproduction. This bisects to 3011177640
(pack-redundant: delay creation of unique_objects, 2019-02-02).

I suspect the fix is just:

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 3e70f2a4c1..68afcfeb7b 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c

@@ -396,7 +396,7 @@ static void minimize(struct pack_list **min)
 
 	pl = local_packs;
 	while (pl) {
-		if (pl->unique_objects->size)
+		if (pl->unique_objects && pl->unique_objects->size)
 			pack_list_insert(&unique, pl);
 		else
 			pack_list_insert(&non_unique, pl);

but I didn't look closely (author cc'd).

The reason this showed up is that Yocto's build tool, bitbake, sometimes
invokes this git command on some of the repositories it downloaded. This
issue was apparently also reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=1803506

We had a discussion not too long ago[1] about whether we should
deprecate and remove pack-redundant, as it's not generally useful. I'm
curious if you have any more context on why it's used in that tool.

-Peff

[1] https://lore.kernel.org/git/20200825172214.GC1414394@coredump.intra.peff.net/ (local)