From: Kyle Ackerman Subject: Re: ID metadata leak To: gameoftrees@openbsd.org Date: Fri, 8 Nov 2024 10:35:34 -0600 ping > On Sep 23, 2024, at 9:30 PM, Kyle Ackerman wrote: > > 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?