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

From:
Kyle Ackerman <kackerman0102@gmail.com>
Subject:
got_blame memory leak
To:
gameoftrees@openbsd.org
Date:
Sun, 24 Mar 2024 17:21:35 -0500

Download raw body.

Thread
There is a small memory leak in {got,tog}.

Here is the kdump of got blame
******** Start dump got *******
M=8 I=1 F=0 U=0 J=1 R=0 X=0 C=0 cache=64 G=0
Leak report:
                 f     sum      #    avg
     0xe0084f637e0      64      1     64 addr2line -e /usr/lib/libc.so.99.0 0xac7e0
     0xe0084f38181     112      7     16 addr2line -e /usr/lib/libc.so.99.0 0x81181
     0xe0084f75333   65536      1  65536 addr2line -e /usr/lib/libc.so.99.0 0xbe333
     0xdfe690c6307      32      1     32 addr2line -e /home/kyle/bin/got 0x79307
     0xe0084f4730b   18280      1  18280 addr2line -e /usr/lib/libc.so.99.0 0x9030b
     0xe0084f46c95   18280      1  18280 addr2line -e /usr/lib/libc.so.99.0 0x8fc95

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

Here is the kdump after the diff below is applied:
******** Start dump got *******
M=8 I=1 F=0 U=0 J=1 R=0 X=0 C=0 cache=64 G=0
Leak report:
                 f     sum      #    avg
     0x2409888c30b   18280      1  18280 addr2line -e /usr/lib/libc.so.99.0 0x9030b
     0x2409888bc95   18280      1  18280 addr2line -e /usr/lib/libc.so.99.0 0x8fc95
     0x240988a87e0      64      1     64 addr2line -e /usr/lib/libc.so.99.0 0xac7e0
     0x2409887d181     112      7     16 addr2line -e /usr/lib/libc.so.99.0 0x81181
     0x240988ba333   65536      1  65536 addr2line -e /usr/lib/libc.so.99.0 0xbe333

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


The leak is present in got_privsep_recv_traversed_commits. There is
an edge case where it receives consecutive imsgs. The first behaves as
normal and we got_object_id_dup the last commit id for
changed_commit_id. The following imsg(s) then still allocates the last
commit id, leaking the one(s) prior allocated.


Here is a diff that checks and frees changed_commit_id.
diff /home/kyle/src/got
commit - 39910b637a9a53cc48b0c63766da691dec0af593
path + /home/kyle/src/got
blob - 857c09bbb3a5d9425176ed93727a912b2242aac6
file + lib/privsep.c
--- lib/privsep.c
+++ lib/privsep.c
@@ -2730,6 +2730,9 @@ got_privsep_recv_traversed_commits(struct got_commit_o
 
 				/* The last commit may contain a change. */
 				if (i == icommits->ncommits - 1) {
+					/* In case we recieved a prior imsg */
+					if (*changed_commit_id)
+						free(*changed_commit_id);
 					*changed_commit_id =
 					    got_object_id_dup(&qid->id);
 					if (*changed_commit_id == NULL) {

All regression tests have passed on my end. Any comments/suggestions/oks?