From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: gotwebd: fix null-deref and ENOMEM question
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Wed, 31 Aug 2022 07:48:43 -0600

On Wed, Aug 31, 2022 at 12:02:26PM +0200, Omar Polo wrote:
> 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.
> > 
> > I've seen that we miss two free on the string returned by
> > gotweb_get_time_str, and while here I'm also dropping a redundant
> > ternary operator (age cannot be NULL if gotweb_get_time_str returned
> > successfully.)
> > 
> > this still doesn't explain why gotwebd reached 600M of RES in hours,
> > so there's probably more.
> >
> > ok?
> 
> anybody got a chance to look at this? :)

Sorry, I thought I ok'd this.

> 
> diff f0680473a7db1e5941bffdc2ab5f80ddec209122 c7d3b38b8596968a207e16ee25ce23773cb6a696
> commit - f0680473a7db1e5941bffdc2ab5f80ddec209122
> commit + c7d3b38b8596968a207e16ee25ce23773cb6a696
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -1172,7 +1172,7 @@ gotweb_render_blame(struct request *c)
>  	    "</div>\n"		/* #blame_header_wrapper */
>  	    "<div class='dotted_line'></div>\n"
>  	    "<div id='blame'>\n",
> -	    age ? age : "",
> +	    age,
>  	    msg);
>  	if (r == -1)
>  		goto done;
> @@ -1184,6 +1184,7 @@ gotweb_render_blame(struct request *c)
>  	fcgi_printf(c, "</div>\n"	/* #blame */
>  	    "</div>\n");		/* #blame_content */
>  done:
> +	free(age);
>  	free(msg);
>  	return error;
>  }
> @@ -1244,7 +1245,7 @@ gotweb_render_briefs(struct request *c)
>  		    "<div class='briefs_log'>"
>  		    "<a href='?index_page=%s&path=%s&action=diff&commit=%s"
>  		    "&headref=%s'>%s</a>",
> -		    age ? age : "",
> +		    age,
>  		    author,
>  		    index_page_str, repo_dir->name, rc->commit_id, qs->headref,
>  		    msg);
> @@ -1353,7 +1354,7 @@ gotweb_render_commits(struct request *c)
>  		    "<div class='commit'>\n%s</div>\n",
>  		    rc->commit_id,
>  		    author,
> -		    age ? age : "",
> +		    age,
>  		    msg);
>  		if (r == -1)
>  			goto done;
> @@ -1527,7 +1528,7 @@ gotweb_render_tree(struct request *c)
>  	    "<div class='dotted_line'></div>\n"
>  	    "<div id='tree'>\n",
>  	    rc->tree_id,
> -	    age ? age : "",
> +	    age,
>  	    msg);
>  	if (r == -1)
>  		goto done;
> @@ -1539,6 +1540,7 @@ gotweb_render_tree(struct request *c)
>  	fcgi_printf(c, "</div>\n"); /* #tree */
>  	fcgi_printf(c, "</div>\n"); /* #tree_content */
>  done:
> +	free(age);
>  	free(msg);
>  	return error;
>  }
> @@ -1594,7 +1596,7 @@ gotweb_render_diff(struct request *c)
>  	    rc->commit_id,
>  	    rc->tree_id,
>  	    author,
> -	    age ? age : "",
> +	    age,
>  	    msg);
>  	if (r == -1)
>  		goto done;
> @@ -1740,7 +1742,7 @@ gotweb_render_tag(struct request *c)
>  	    rt->commit_id,
>  	    tagname,
>  	    author,
> -	    age ? age : "",
> +	    age,
>  	    msg,
>  	    rt->tag_commit);
>  
> @@ -1833,7 +1835,7 @@ gotweb_render_tags(struct request *c)
>  		    "</div>\n"	/* .navs */
>  		    "</div>\n"	/* .navs_wrapper */
>  		    "<div class='dotted_line'></div>\n",
> -		    age ? age : "",
> +		    age,
>  		    tagname,
>  		    index_page_str, repo_dir->name, rt->commit_id,
>  		    msg ? msg : "",
> 

-- 

Tracey Emery