From: Kyle Ackerman Subject: got_blame memory leak To: gameoftrees@openbsd.org Date: Sun, 24 Mar 2024 17:21:35 -0500 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?