On Wed, Nov 15, 2023 at 07:46:57PM +0100, Omar Polo wrote:
> updated diff, this includes the fix to the daemon() call too.

Thanks for working on this! I read through and didn't see anything
horrible. Ok by me. We'll deal with any fallout that comes up later.
I will hold off a new release for a few days to give developers some
time to test it.

> +int
> +main_compose_sockets(struct gotwebd *env, uint32_t type, int fd,
> +    const void *data, uint16_t len)
> +{
> +	size_t	 i;
> +	int	 ret, d;
> +
> +	for (i = 0; i < env->nserver; ++i) {
> +		d = -1;
> +		if (fd != -1 && (d = dup(fd)) == -1)
> +			return (-1);

This function does not close fd on error, but closes fd on success.
It's not a big deal since all callers end up in fatal() if this
function fails, and this code is only used while loading the
configuration. But it looks odd when some return paths care
about closing the file and some don't.

> +
> +		ret = imsg_compose_event(&env->iev_server[i], type, 0, -1,
> +		    d, data, len);
> +		if (ret == -1)
> +			return (-1);
> +
> +		/* prevent fd exhaustion */
> +		if (d != -1) {
> +			do {
> +				ret = imsg_flush(&env->iev_server[i].ibuf);
> +			} while (ret == -1 && errno == EAGAIN);
> +			if (ret == -1)
> +				return (-1);
> +			imsg_event_add(&env->iev_server[i]);
> +		}
>  	}
>  
> -	return (0);
> +	if (fd != -1)
> +		close(fd);
> +
> +	return 0;
>  }


> +	if (p[0] != 3) {
> +		if (dup2(p[0], 3) == -1)

This magic number 3 was derived from PROC_GOTWEBD_SOCK_FILENO, right?
Consider adding a GOTWEBD_SOCK_FILENO constant for it.

> +			fatal("dup2");
> +	} else if (fcntl(p[0], F_SETFD, 0) == -1)
> +		fatal("fcntl");
> +
> +	/* obnoxious cast */
> +	execvp(argv0, (char * const *)argv);

I recently ran into this quirk elsewhere and it's indeed obnoxious :)

> -	for (proc = 0; proc < nitems(procs); proc++)
> -		procs[proc].p_chroot = env->httpd_chroot;
> +		sockets(env, 3);

Again, a magic number 3.

> +#define IMSG_SIZE_CHECK(imsg, p) do {					\
> +	if (IMSG_DATA_SIZE(imsg) < sizeof(*p))				\

Could this check even be != sizeof(*p)?
As far as I can see all messages used by gotwebd are of a fixed size.