From: Kyle Ackerman Subject: Re: ID metadata leak To: gameoftrees@openbsd.org Date: Mon, 23 Sep 2024 21:30:43 -0500 Kyle Ackerman writes: > Hello all, > > I have found a memory leak. The command I used to test this is > `got send`. > > > Here are the backtraces: > > MALLOC_OPTIONS=D > ******** Start dump got ******* > Leak report: > f sum # avg > 0x5f362c98380 64 1 64 addr2line -e /usr/lib/libc.so.100.3 0x97380 dupgl /usr/src/lib/libc/locale/setlocale.c:0 > 0x5f362cb67ea 112 7 16 addr2line -e /usr/lib/libc.so.100.3 0xb57ea _libc_strdup /usr/src/lib/libc/string/strdup.c:45 > 0x5f362c698fc 65536 1 65536 addr2line -e /usr/lib/libc.so.100.3 0x688fc __smakebuf /usr/src/lib/libc/stdio/makebuf.c:0 > 0x5f35f0b7861 128 2 64 addr2line -e /usr/lib/libutil.so.18.0 0xd861 ibuf_open /usr/src/lib/libutil/imsg-buffer.c:49 > 0x5f35f0b7876 144 2 72 addr2line -e /usr/lib/libutil.so.18.0 0xd876 ibuf_open /usr/src/lib/libutil/imsg-buffer.c:51 > 0x5f0ba4445a6 160 1 160 addr2line -e /home/kyle/bin/got 0xb75a6 alloc_meta /home/kyle/src/got/got/../lib/pack_create.c:86 > 0x5f0ba4445a6 160 1 160 addr2line -e /home/kyle/bin/got 0xb75a6 alloc_meta /home/kyle/src/got/got/../lib/pack_create.c:86 > 0x5f0ba4445a6 160 1 160 addr2line -e /home/kyle/bin/got 0xb75a6 alloc_meta /home/kyle/src/got/got/../lib/pack_create.c:86 > > ******** End dump got ******* > > MALLOC_OPTIONS=4 > ******** Start dump got ******* > Leak report: > f sum # avg > 0x4f971d1f861 128 2 64 addr2line -e /usr/lib/libutil.so.18.0 0xd861 ibuf_open /usr/src/lib/libutil/imsg-buffer.c:49 > 0x4f971d1b545 - - - addr2line -e /usr/lib/libutil.so.18.0 0x9545 imsg_get /usr/src/lib/libutil/imsg.c:156 > 0x4f72780d328 - - - addr2line -e /home/kyle/bin/got 0x65328 got_privsep_recv_imsg /home/kyle/src/got/got/../lib/privsep.c:139 > 0x4f72780f850 - - - addr2line -e /home/kyle/bin/got 0x67850 got_privsep_recv_send_remote_refs /home/kyle/src/got/got/../lib/privsep.c:951 > 0x4f971d1f876 144 2 72 addr2line -e /usr/lib/libutil.so.18.0 0xd876 ibuf_open /usr/src/lib/libutil/imsg-buffer.c:51 > 0x4f971d1b545 - - - addr2line -e /usr/lib/libutil.so.18.0 0x9545 imsg_get /usr/src/lib/libutil/imsg.c:156 > 0x4f72780d328 - - - addr2line -e /home/kyle/bin/got 0x65328 got_privsep_recv_imsg /home/kyle/src/got/got/../lib/privsep.c:139 > 0x4f72780f850 - - - addr2line -e /home/kyle/bin/got 0x67850 got_privsep_recv_send_remote_refs /home/kyle/src/got/got/../lib/privsep.c:951 > 0x4f72785f5a6 160 1 160 addr2line -e /home/kyle/bin/got 0xb75a6 alloc_meta /home/kyle/src/got/got/../lib/pack_create.c:86 > 0x4f72785f39d - - - addr2line -e /home/kyle/bin/got 0xb739d got_pack_add_object /home/kyle/src/got/got/../lib/pack_create.c:748 > 0x4f727862c1b - - - addr2line -e /home/kyle/bin/got 0xbac1b load_commit /home/kyle/src/got/got/../lib/pack_create.c:965 > 0x4f727861105 - - - addr2line -e /home/kyle/bin/got 0xb9105 load_object_ids /home/kyle/src/got/got/../lib/pack_create.c:1390 > 0x4f72785f5a6 160 1 160 addr2line -e /home/kyle/bin/got 0xb75a6 alloc_meta /home/kyle/src/got/got/../lib/pack_create.c:86 > 0x4f72785f39d - - - addr2line -e /home/kyle/bin/got 0xb739d got_pack_add_object /home/kyle/src/got/got/../lib/pack_create.c:748 > 0x4f72785fe51 - - - addr2line -e /home/kyle/bin/got 0xb7e51 got_pack_load_tree /home/kyle/src/got/got/../lib/pack_create.c:897 > 0x4f727862d19 - - - addr2line -e /home/kyle/bin/got 0xbad19 load_commit /home/kyle/src/got/got/../lib/pack_create.c:973 > 0x4f72785f5a6 160 1 160 addr2line -e /home/kyle/bin/got 0xb75a6 alloc_meta /home/kyle/src/got/got/../lib/pack_create.c:86 > 0x4f72785f39d - - - addr2line -e /home/kyle/bin/got 0xb739d got_pack_add_object /home/kyle/src/got/got/../lib/pack_create.c:748 > 0x4f72785faad - - - addr2line -e /home/kyle/bin/got 0xb7aad got_pack_load_tree_entries /home/kyle/src/got/got/../lib/pack_create.c:828 > 0x4f72785ff8f - - - addr2line -e /home/kyle/bin/got 0xb7f8f got_pack_load_tree /home/kyle/src/got/got/../lib/pack_create.c:915 > 0x4f96ca1d8fc 65536 1 65536 addr2line -e /usr/lib/libc.so.100.3 0x688fc __smakebuf /usr/src/lib/libc/stdio/makebuf.c:0 > 0x4f96c9f8a3f - - - addr2line -e /usr/lib/libc.so.100.3 0x43a3f __swsetup /usr/src/lib/libc/stdio/wsetup.c:82 > 0x4f96ca961f4 - - - addr2line -e /usr/lib/libc.so.100.3 0xe11f4 __vfprintf /usr/src/lib/libc/stdio/vfprintf.c:477 > 0x4f96ca96153 - - - addr2line -e /usr/lib/libc.so.100.3 0xe1153 _libc_vfprintf /usr/src/lib/libc/stdio/vfprintf.c:279 > 0x4f96ca4c380 64 1 64 addr2line -e /usr/lib/libc.so.100.3 0x97380 dupgl /usr/src/lib/libc/locale/setlocale.c:0 > 0x4f7277c44d6 - - - addr2line -e /home/kyle/bin/got 0x1c4d6 main /home/kyle/src/got/got/got.c:227 > 0x4f7277c42bb - - - addr2line -e /home/kyle/bin/got 0x1c2bb _start ??:0 > 0x4f96ca6a7ea 16 1 16 addr2line -e /usr/lib/libc.so.100.3 0xb57ea _libc_strdup /usr/src/lib/libc/string/strdup.c:45 > 0x4f96ca4c3a1 - - - addr2line -e /usr/lib/libc.so.100.3 0x973a1 dupgl /usr/src/lib/libc/locale/setlocale.c:0 > 0x4f7277c44d6 - - - addr2line -e /home/kyle/bin/got 0x1c4d6 main /home/kyle/src/got/got/got.c:227 > 0x4f7277c42bb - - - addr2line -e /home/kyle/bin/got 0x1c2bb _start ??:0 > 0x4f96ca6a7ea 16 1 16 addr2line -e /usr/lib/libc.so.100.3 0xb57ea _libc_strdup /usr/src/lib/libc/string/strdup.c:45 > 0x4f96ca4c550 - - - addr2line -e /usr/lib/libc.so.100.3 0x97550 dupgl /usr/src/lib/libc/locale/setlocale.c:46 > 0x4f7277c44d6 - - - addr2line -e /home/kyle/bin/got 0x1c4d6 main /home/kyle/src/got/got/got.c:227 > 0x4f7277c42bb - - - addr2line -e /home/kyle/bin/got 0x1c2bb _start ??:0 > 0x4f96ca6a7ea 16 1 16 addr2line -e /usr/lib/libc.so.100.3 0xb57ea _libc_strdup /usr/src/lib/libc/string/strdup.c:45 > 0x4f96ca4c594 - - - addr2line -e /usr/lib/libc.so.100.3 0x97594 dupgl /usr/src/lib/libc/locale/setlocale.c:46 > 0x4f7277c44d6 - - - addr2line -e /home/kyle/bin/got 0x1c4d6 main /home/kyle/src/got/got/got.c:227 > 0x4f7277c42bb - - - addr2line -e /home/kyle/bin/got 0x1c2bb _start ??:0 > 0x4f96ca6a7ea 16 1 16 addr2line -e /usr/lib/libc.so.100.3 0xb57ea _libc_strdup /usr/src/lib/libc/string/strdup.c:45 > 0x4f96ca4c5b9 - - - addr2line -e /usr/lib/libc.so.100.3 0x975b9 dupgl /usr/src/lib/libc/locale/setlocale.c:46 > 0x4f7277c44d6 - - - addr2line -e /home/kyle/bin/got 0x1c4d6 main /home/kyle/src/got/got/got.c:227 > 0x4f7277c42bb - - - addr2line -e /home/kyle/bin/got 0x1c2bb _start ??:0 > 0x4f96ca6a7ea 16 1 16 addr2line -e /usr/lib/libc.so.100.3 0xb57ea _libc_strdup /usr/src/lib/libc/string/strdup.c:45 > 0x4f96ca4c5d7 - - - addr2line -e /usr/lib/libc.so.100.3 0x975d7 dupgl /usr/src/lib/libc/locale/setlocale.c:46 > 0x4f7277c44d6 - - - addr2line -e /home/kyle/bin/got 0x1c4d6 main /home/kyle/src/got/got/got.c:227 > 0x4f7277c42bb - - - addr2line -e /home/kyle/bin/got 0x1c2bb _start ??:0 > 0x4f96ca6a7ea 16 1 16 addr2line -e /usr/lib/libc.so.100.3 0xb57ea _libc_strdup /usr/src/lib/libc/string/strdup.c:45 > 0x4f96ca4c5f5 - - - addr2line -e /usr/lib/libc.so.100.3 0x975f5 dupgl /usr/src/lib/libc/locale/setlocale.c:46 > 0x4f7277c44d6 - - - addr2line -e /home/kyle/bin/got 0x1c4d6 main /home/kyle/src/got/got/got.c:227 > 0x4f7277c42bb - - - addr2line -e /home/kyle/bin/got 0x1c2bb _start ??:0 > 0x4f96ca6a7ea 16 1 16 addr2line -e /usr/lib/libc.so.100.3 0xb57ea _libc_strdup /usr/src/lib/libc/string/strdup.c:45 > 0x4f96ca4c639 - - - addr2line -e /usr/lib/libc.so.100.3 0x97639 changegl /usr/src/lib/libc/locale/setlocale.c:0 > 0x4f7277c44d6 - - - addr2line -e /home/kyle/bin/got 0x1c4d6 main /home/kyle/src/got/got/got.c:227 > 0x4f7277c42bb - - - addr2line -e /home/kyle/bin/got 0x1c2bb _start ??:0 > > ******** End dump got ******* > > > I am lacking time to track this down and create a patch, > so I am sending this here if anyone can beat me to it. Here is a patch for the alloc_meta leak: 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 @@ -1087,6 +1087,18 @@ append_id(struct got_object_id *id, void *data, void * } static const struct got_error * +free_meta(struct got_object_id *id, void *data, void *arg) +{ + struct got_pack_meta *meta = data; + if (meta){ + clear_meta(meta); + free(meta); + } + + return NULL; +} + +static const struct got_error * queue_commit_or_tag_id(struct got_object_id *id, intptr_t color, struct got_object_id_queue *ids, struct got_repository *repo) { @@ -1954,6 +1966,7 @@ got_pack_create(struct got_object_id *packhash, int pa done: free_nmeta(deltify.meta, deltify.nmeta); free_nmeta(reuse.meta, reuse.nmeta); + got_object_idset_for_each(idset, free_meta, NULL); got_object_idset_free(idset); got_repo_unpin_pack(repo); return err; Here is the kdump after the patch is applied: ******** Start dump got ******* M=8 I=1 F=1 U=1 J=2 R=0 X=0 C=0x871151fc cache=0 G=4096 Leak report: f sum # avg 0x37b2718e530 64 1 64 addr2line -e /usr/lib/libc.so.100.3 0x89530 0x37b27177a7a 112 7 16 addr2line -e /usr/lib/libc.so.100.3 0x72a7a 0x37b135b7491 192 3 64 addr2line -e /usr/lib/libutil.so.18.0 0x10491 0x37b135b74a6 224 3 74 addr2line -e /usr/lib/libutil.so.18.0 0x104a6 0x3790170ab6c 32 1 32 addr2line -e /home/kyle/bin/got 0xb4b6c 0x3790170abca 7168 1 7168 addr2line -e /home/kyle/bin/got 0xb4bca 0x3790170ac20 512 1 512 addr2line -e /home/kyle/bin/got 0xb4c20 ******** End dump got ******* The functions that alloc got_pack_meta only insert them into `idset`, not `idset_exclude`, which is passed into `load_object_ids`, so we can to free them in `got_pack_create`. This patch has passed regressions on my end. Any thoughts/comments/suggestions?