From: Omar Polo <op@omarpolo.com>
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: Thu, 01 Sep 2022 17:44:17 +0200

On 2022/08/25 17:41:46 +0200, Omar Polo <op@omarpolo.com> wrote:
> I'm still seeing gotwebd increasing its memory usage over time so I'm
> still searching for leaks.
> 
> [...]
> 
> this still doesn't explain why gotwebd reached 600M of RES in hours,
> so there's probably more.

one big leak was in gotweb_render_index where we fail to free the data
allocated by scandir(3)

with this is, gotwebd memory consumption didn't raised after a few
thousands requests (I'm running ftp in a loop against it)! \o/

(I suspect there are still some leaks in the error paths inside the
main loop -- the one with the `render' label in it -- will come back
to this later if i don't forget and nobody beats me to it.)

diff /home/op/w/got
commit - f8faf9f103c9a4869c82a3fe55658f0a065fb1c0
path + /home/op/w/got
blob - d7b904108bc299cb7ad3753fbb33a3660024d371
file + gotwebd/gotweb.c
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -940,7 +940,7 @@ gotweb_render_index(struct request *c)
 	struct querystring *qs = t->qs;
 	struct repo_dir *repo_dir = NULL;
 	DIR *d;
-	struct dirent **sd_dent;
+	struct dirent **sd_dent = NULL;
 	const char *index_page_str;
 	char *c_path = NULL;
 	struct stat st;
@@ -957,6 +957,7 @@ gotweb_render_index(struct request *c)
 
 	d_cnt = scandir(srv->repos_path, &sd_dent, NULL, alphasort);
 	if (d_cnt == -1) {
+		sd_dent = NULL;
 		error = got_error_from_errno2("scandir", srv->repos_path);
 		goto done;
 	}
@@ -1119,6 +1120,11 @@ render:
 	if (error)
 		goto done;
 done:
+	if (sd_dent) {
+		for (d_i = 0; d_i < d_cnt; d_i++)
+			free(sd_dent[d_i]);
+		free(sd_dent);
+	}
 	if (d != NULL && closedir(d) == EOF && error == NULL)
 		error = got_error_from_errno("closedir");
 	return error;