From: Kyle Ackerman <kackerman0102@gmail.com>
Subject: got_pack_meta->dtab leak
To: gameoftrees@openbsd.org
Date: Mon, 23 Sep 2024 22:07:08 -0500

Here is the kdump of the leak

******** Start dump got *******
M=8 I=1 F=1 U=1 J=2 R=0 X=0 C=0xea4f16f8 cache=0 G=4096
Leak report:
                 f     sum      #    avg
     0x9aa994ca530      64      1     64 addr2line -e /usr/lib/libc.so.100.3 0x89530 dupgl /usr/src/lib/libc/locale/setlocale.c:0 
     0x9aa994b3a7a     112      7     16 addr2line -e /usr/lib/libc.so.100.3 0x72a7a _libc_strdup /usr/src/lib/libc/string/strdup.c:45 
     0x9aac9948491     192      3     64 addr2line -e /usr/lib/libutil.so.18.0 0x10491 ibuf_open /usr/src/lib/libutil/imsg-buffer.c:49 
     0x9aac99484a6     224      3     74 addr2line -e /usr/lib/libutil.so.18.0 0x104a6 ibuf_open /usr/src/lib/libutil/imsg-buffer.c:51 
     0x9a878ec0596     800      5    160 addr2line -e /home/kyle/bin/got 0xb7596 alloc_meta /home/kyle/src/got/got/../lib/pack_create.c:86 
     0x9a878ebdb3c      32      1     32 addr2line -e /home/kyle/bin/got 0xb4b3c got_deltify_init_mem /home/kyle/src/got/got/../lib/deltify.c:420 
     0x9a878ebdb9a    7168      1   7168 addr2line -e /home/kyle/bin/got 0xb4b9a got_deltify_init_mem /home/kyle/src/got/got/../lib/deltify.c:426 
     0x9a878ebdbf0     512      1    512 addr2line -e /home/kyle/bin/got 0xb4bf0 got_deltify_init_mem /home/kyle/src/got/got/../lib/deltify.c:433 

******** End dump got *******

Here is a patch to free the leaked `got_delta_table` entries.

diff /home/kyle/src/got
commit - 370f10653490c6d8f3e587869c96c100535edc9f
path + /home/kyle/src/got
blob - 637ec2cd3baf5cc77c6593f1630808a6e0b7b58e
file + lib/pack_create.c
--- lib/pack_create.c
+++ lib/pack_create.c
@@ -114,8 +114,11 @@ free_nmeta(struct got_pack_meta **meta, int nmeta)
 {
 	int i;
 
-	for (i = 0; i < nmeta; i++)
+	for (i = 0; i < nmeta; i++) {
 		clear_meta(meta[i]);
+		got_deltify_free(meta[i]->dtab);
+	}
+
 	free(meta);
 }
 
@@ -691,10 +694,6 @@ pick_deltas(struct got_pack_meta **meta, int nmeta, in
 		raw = NULL;
 	}
 done:
-	for (i = MAX(0, nmeta - max_base_candidates); i < nmeta; i++) {
-		got_deltify_free(meta[i]->dtab);
-		meta[i]->dtab = NULL;
-	}
 	if (raw)
 		got_object_raw_close(raw);
 	if (base_raw)


Here is a kdump after the patch is applied:

******** Start dump got *******
M=8 I=1 F=1 U=1 J=2 R=0 X=0 C=0x16c5e853 cache=0 G=4096
Leak report:
                 f     sum      #    avg
     0x29878573530      64      1     64 addr2line -e /usr/lib/libc.so.100.3 0x89530 dupgl /usr/src/lib/libc/locale/setlocale.c:0 
     0x2987855ca7a     112      7     16 addr2line -e /usr/lib/libc.so.100.3 0x72a7a _libc_strdup /usr/src/lib/libc/string/strdup.c:45 
     0x298be168491     192      3     64 addr2line -e /usr/lib/libutil.so.18.0 0x10491 ibuf_open /usr/src/lib/libutil/imsg-buffer.c:49 
     0x298be1684a6     224      3     74 addr2line -e /usr/lib/libutil.so.18.0 0x104a6 ibuf_open /usr/src/lib/libutil/imsg-buffer.c:51 
     0x29601a6a596     800      5    160 addr2line -e /home/kyle/bin/got 0xb7596 alloc_meta /home/kyle/src/got/got/../lib/pack_create.c:86 

******** End dump got *******

The only calls to `free_nmeta` are in `got_pack_create` so it doesn't
hurt to put `got_deltify_free` there.  My working theory on why the
existing logic leaks is that the first `nmeta - max_base_candidates` are
not free'ed in `got_pack_create`.


All regressions passed on my end.  Thoughts/comments/suggestions?