"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Kyle Ackerman <kackerman0102@gmail.com>
Subject:
Re: got_pack_meta->dtab leak
To:
gameoftrees@openbsd.org
Date:
Sun, 20 Oct 2024 16:13:22 -0500

Download raw body.

Thread
ping 

> On Sep 23, 2024, at 10:07 PM, Kyle Ackerman <kackerman0102@gmail.com> wrote:
> 
> 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?