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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: gotwebd: fix null-deref and ENOMEM question
To:
Omar Polo <op@omarpolo.com>
Cc:
Tracey Emery <tracey@traceyemery.net>, gameoftrees@openbsd.org
Date:
Wed, 31 Aug 2022 14:04:09 +0200

Download raw body.

Thread
On Thu, Aug 25, 2022 at 05:41:46PM +0200, Omar Polo wrote:
> this still doesn't explain why gotwebd reached 600M of RES in hours,
> so there's probably more.

A big leak is that the code forgots to free the memory allocated
internally by bloom filters:

blob - fc898c75e876b90c401ee5436e0a2932924d9bbd
file + lib/repository.c
--- lib/repository.c
+++ lib/repository.c
@@ -872,6 +872,7 @@ got_repo_close(struct got_repository *repo)
 	    &repo->packidx_bloom_filters))) {
 		RB_REMOVE(got_packidx_bloom_filter_tree,
 		    &repo->packidx_bloom_filters, bf);
+		bloom_free(bf->bloom);
 		free(bf->bloom);
 		free(bf);
 	}

Ok for the above?

The patch below can be used to trigger this leak via 'got cat foo':

diff /home/stsp/src/got
commit - 3c8433402874be5d4f42156998646f05de8535c3
path + /home/stsp/src/got
blob - 24a866da4c5f2667eabcecfe5a0b19cde5a70e76
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -12820,7 +12820,6 @@ cmd_cat(int argc, char *argv[])
 	}
 
 	error = got_repo_open(&repo, repo_path, NULL, pack_fds);
-	free(repo_path);
 	if (error != NULL)
 		goto done;
 
@@ -12843,6 +12842,20 @@ cmd_cat(int argc, char *argv[])
 	if (error)
 		goto done;
 
+	while (1) {
+		got_object_commit_close(commit);
+		commit = NULL;
+		error = got_repo_close(repo);
+		if (error != NULL)
+			goto done;
+		error = got_repo_open(&repo, repo_path, NULL, pack_fds);
+		if (error != NULL)
+			goto done;
+		error = got_object_open_as_commit(&commit, repo, commit_id);
+		if (error)
+			goto done;
+	}
+
 	for (i = 0; i < argc; i++) {
 		if (force_path) {
 			error = got_object_id_by_path(&id, repo, commit,
@@ -12896,6 +12909,7 @@ done:
 	free(label);
 	free(id);
 	free(commit_id);
+	free(repo_path);
 	if (commit)
 		got_object_commit_close(commit);
 	if (worktree)