From: Omar Polo <op@omarpolo.com>
Subject: gotwebd: leaks in blame page
To: gameoftrees@openbsd.org
Date: Fri, 02 Sep 2022 17:20:09 +0200

commit #2 and #4 are real leaks, #3 only on the error path.  #1 is
just a redundant check I guess.

however, the blame page still leaks like crazy.  There's some serious
leak between got_gotweb_blame_cb and got_output_file_blame that I'm
just not seeing.

In a controlled environment (localhost) after 1K requests to a blame
page (always the same to rule out the cache) gotwebd' RES is 24M.  If
I comment the call to got_blame then it's "just" ~5M.  So, it leaks
both outside got_output_file_blame but the more serious leak is
inside.

-----------------------------------------------
commit d805ade9606de1350051e811407afc6aefe5c486
from: Omar Polo <op@omarpolo.com>
date: Fri Sep  2 14:13:15 2022 UTC
 
 gotwebd: drop useless check
 
 commit_id is not NULL if got_repo_match_object_id returned
 successfully
 
diff e5e662e42c45f0d30f5f97fb0e2ad5f3c4f8b488 d805ade9606de1350051e811407afc6aefe5c486
commit - e5e662e42c45f0d30f5f97fb0e2ad5f3c4f8b488
commit + d805ade9606de1350051e811407afc6aefe5c486
blob - 14d7cbf7261ca9171a51aa4286a2d0870ffcf937
blob + 3fdee51058625f396f08e673e79e87ba2d0843db
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -1228,11 +1228,6 @@ got_output_file_blame(struct request *c)
 	if (error)
 		goto done;
 
-	if (commit_id == NULL) {
-		error = got_error(GOT_ERR_NO_OBJ);
-		goto done;
-	}
-
 	error = got_object_get_type(&obj_type, repo, obj_id);
 	if (error)
 		goto done;

-----------------------------------------------
commit 0267d4954a2da82ddc8df42f9a951410616a69f5
from: Omar Polo <op@omarpolo.com>
date: Fri Sep  2 14:51:27 2022 UTC
 
 gotwebd: free eline in got_gotweb_blame_cb
 
diff d805ade9606de1350051e811407afc6aefe5c486 0267d4954a2da82ddc8df42f9a951410616a69f5
commit - d805ade9606de1350051e811407afc6aefe5c486
commit + 0267d4954a2da82ddc8df42f9a951410616a69f5
blob - 3fdee51058625f396f08e673e79e87ba2d0843db
blob + 76c6d44ac38e69c04070e891dd9273426ebc3fc3
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -1176,6 +1176,9 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno,
 
 		a->lineno_cur++;
 		bline = &a->lines[a->lineno_cur - 1];
+
+		free(eline);
+		eline = NULL;
 	}
 done:
 	free(line);

-----------------------------------------------
commit c95eb8a880b5f36e38453937ea27771601d94cac
from: Omar Polo <op@omarpolo.com>
date: Fri Sep  2 14:52:00 2022 UTC
 
 gotwebd: got_output_file_blame: free lines on error path
 
diff 0267d4954a2da82ddc8df42f9a951410616a69f5 c95eb8a880b5f36e38453937ea27771601d94cac
commit - 0267d4954a2da82ddc8df42f9a951410616a69f5
commit + c95eb8a880b5f36e38453937ea27771601d94cac
blob - 76c6d44ac38e69c04070e891dd9273426ebc3fc3
blob + 7ffadf4bd520c4bcb95709f980fb217d68e7f98c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -1296,7 +1296,8 @@ got_output_file_blame(struct request *c)
 	    GOT_DIFF_ALGORITHM_MYERS, got_gotweb_blame_cb, &bca, NULL, NULL,
 	    fd3, fd4, f1, f2);
 
-	if (blob) {
+done:
+	if (bca.lines) {
 		free(bca.line_offsets);
 		for (i = 0; i < bca.nlines; i++) {
 			struct blame_line *bline = &bca.lines[i];
@@ -1304,7 +1305,6 @@ got_output_file_blame(struct request *c)
 			free(bline->committer);
 		}
 	}
-done:
 	free(bca.lines);
 	if (fd2 != -1 && close(fd2) == -1 && error == NULL)
 		error = got_error_from_errno("close");

-----------------------------------------------
commit aeea8f91fd537018277c7b38da7a3b17ae32af12 (leaks)
from: Omar Polo <op@omarpolo.com>
date: Fri Sep  2 15:01:04 2022 UTC
 
 gotwebd: free obj_id and reflist in got_output_file_blame
 
diff c95eb8a880b5f36e38453937ea27771601d94cac aeea8f91fd537018277c7b38da7a3b17ae32af12
commit - c95eb8a880b5f36e38453937ea27771601d94cac
commit + aeea8f91fd537018277c7b38da7a3b17ae32af12
blob - 7ffadf4bd520c4bcb95709f980fb217d68e7f98c
blob + 76816f00e1861046d23bfcd7dc01706f222e7f5c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -1336,7 +1336,9 @@ done:
 		got_object_blob_close(blob);
 	free(in_repo_path);
 	free(commit_id);
+	free(obj_id);
 	free(path);
+	got_ref_list_free(&refs);
 	return error;
 }