"GOT", but the "O" is a cute, smiling sun Index | Thread

From:
Tracey Emery <tracey@traceyemery.net>
Subject:
Re: gotd: remove "sendfd" pledge promise from repo_read
To:
gameoftrees@openbsd.org
Date:
Fri, 28 Oct 2022 07:42:21 -0600

Download raw body.

On Fri, Oct 28, 2022 at 03:36:57PM +0200, Stefan Sperling wrote:
> This patch requires the corresponding patch for repo_write to be
> applied first.
> 

ok. same extra tab below

> diff 1b564763ebbf2659ed1af8474ba471640384301f c5119b16ae24d36f28af86bcbd2221a56af86a61
> commit - 1b564763ebbf2659ed1af8474ba471640384301f
> commit + c5119b16ae24d36f28af86bcbd2221a56af86a61
> blob - 0d1b70abc84b7ffd073e85521770b144812ebcfe
> blob + e3a20027002c07e25791ee436b0838e10613c439
> --- gotd/gotd.c
> +++ gotd/gotd.c
> @@ -682,6 +682,7 @@ send_packfile(struct gotd_client *client)
>  
>  	ipipe.client_id = client->id;
>  
> +	/* Send pack pipe end 0 to repo_read. */
>  	if (gotd_imsg_compose_event(&client->repo_read->iev,
>  	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[0],
>  	        &ipipe, sizeof(ipipe)) == -1) {
> @@ -690,9 +691,9 @@ send_packfile(struct gotd_client *client)
>  		return err;
>  	}
>  
> -	if (gotd_imsg_compose_event(&client->repo_read->iev,
> -	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[1],
> -	        &ipipe, sizeof(ipipe)) == -1)
> +	/* Send pack pipe end 1 to gotsh(1) (expects just an fd, no data). */
> +	if (gotd_imsg_compose_event(&client->iev,
> +	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[1], NULL, 0) == -1)
>  		err = got_error_from_errno("imsg compose PACKFILE_PIPE");
>  
>  	return err;
> @@ -2034,7 +2035,7 @@ main(int argc, char **argv)
>  		break;
>  	case PROC_REPO_READ:
>  #ifndef PROFILE
> -		if (pledge("stdio rpath sendfd recvfd", NULL) == -1)
> +		if (pledge("stdio rpath recvfd", NULL) == -1)
>  			err(1, "pledge");
>  #endif
>  		repo_read_main(title, pack_fds, temp_fds);
> blob - b1796ad22b340fe20d6d27d7e25500c103a0e37f
> blob + 3acfa77b04f537a7820959d7704e095c2998cde3
> --- gotd/repo_read.c
> +++ gotd/repo_read.c
> @@ -68,7 +68,7 @@ struct repo_read_client {
>  	int				 fd;
>  	int				 delta_cache_fd;
>  	int				 report_progress;
> -	int				 pack_pipe[2];
> +	int				 pack_pipe;
>  	struct gotd_object_id_array	 want_ids;
>  	struct gotd_object_id_array	 have_ids;
>  };
> @@ -91,8 +91,7 @@ add_client(struct repo_read_client *client, uint32_t c
>  	client->id = client_id;
>  	client->fd = fd;
>  	client->delta_cache_fd = -1;
> -	client->pack_pipe[0] = -1;
> -	client->pack_pipe[1] = -1;
> +	client->pack_pipe = -1;
>  	slot = client_hash(client->id) % nitems(repo_read_clients);
>  	STAILQ_INSERT_HEAD(&repo_read_clients[slot], client, entry);
>  }
> @@ -641,14 +640,10 @@ receive_pack_pipe(struct repo_read_client **client, st
>  	*client = find_client(ireq.client_id);
>  	if (*client == NULL)
>  		return got_error(GOT_ERR_CLIENT_ID);
> -	if ((*client)->pack_pipe[1] != -1)
> +	if ((*client)->pack_pipe != -1)
>  		return got_error(GOT_ERR_PRIVSEP_MSG);
>  
> -	if ((*client)->pack_pipe[0] == -1)
> -		(*client)->pack_pipe[0] = imsg->fd;
> -	else
> -		(*client)->pack_pipe[1] = imsg->fd;
> -
> +	(*client)->pack_pipe = imsg->fd;
>  	return NULL;
>  }
>  
> @@ -669,25 +664,11 @@ send_packfile(struct repo_read_client *client, struct 
>  
>  	got_ratelimit_init(&rl, 2, 0);
>  
> -	if (client->delta_cache_fd == -1 ||
> -	    client->pack_pipe[0] == -1 ||
> -	    client->pack_pipe[1] == -1)
> +	if (client->delta_cache_fd == -1 || client->pack_pipe == -1)
>  		return got_error(GOT_ERR_PRIVSEP_NO_FD);
>  
>  	imsg_init(&ibuf, client->fd);
>  
> -	/* Send pack file pipe to gotsh(1). */
> -	if (imsg_compose(&ibuf, GOTD_IMSG_PACKFILE_PIPE, PROC_REPO_READ,
> -	    repo_read.pid, client->pack_pipe[1], NULL, 0) == -1) {
> -		err = got_error_from_errno("imsg_compose ACK");
> -		if (err)	
same here
> -			goto done;
> -	}
> -	client->pack_pipe[1] = -1;
> -	err = gotd_imsg_flush(&ibuf);
> -	if (err)
> -		goto done;
> -
>  	delta_cache = fdopen(client->delta_cache_fd, "w+");
>  	if (delta_cache == NULL) {
>  		err = got_error_from_errno("fdopen");
> @@ -699,7 +680,7 @@ send_packfile(struct repo_read_client *client, struct 
>  	pa.ibuf = &ibuf;
>  	pa.report_progress = client->report_progress;
>  
> -	err = got_pack_create(packsha1, client->pack_pipe[0], delta_cache,
> +	err = got_pack_create(packsha1, client->pack_pipe, delta_cache,
>  	    client->have_ids.ids, client->have_ids.nids,
>  	    client->want_ids.ids, client->want_ids.nids,
>  	    repo_read.repo, 0, 1, pack_progress, &pa, &rl,
> @@ -729,7 +710,7 @@ recv_disconnect(struct imsg *imsg)
>  	const struct got_error *err = NULL;
>  	struct gotd_imsg_disconnect idisconnect;
>  	size_t datalen;
> -	int client_fd, delta_cache_fd, pipe[2];
> +	int client_fd, delta_cache_fd, pack_pipe;
>  	struct repo_read_client *client = NULL;
>  	uint64_t slot;
>  
> @@ -751,17 +732,14 @@ recv_disconnect(struct imsg *imsg)
>  	free_object_ids(&client->want_ids);
>  	client_fd = client->fd;
>  	delta_cache_fd = client->delta_cache_fd;
> -	pipe[0] = client->pack_pipe[0];
> -	pipe[1] = client->pack_pipe[1];
> +	pack_pipe = client->pack_pipe;
>  	free(client);
>  	if (close(client_fd) == -1)
>  		err = got_error_from_errno("close");
>  	if (delta_cache_fd != -1 && close(delta_cache_fd) == -1 && err == NULL)
>  		return got_error_from_errno("close");
> -	if (pipe[0] != -1 && close(pipe[0]) == -1 && err == NULL)
> +	if (pack_pipe != -1 && close(pack_pipe) == -1 && err == NULL)
>  		return got_error_from_errno("close");
> -	if (pipe[1] != -1 && close(pipe[1]) == -1 && err == NULL)
> -		return got_error_from_errno("close");
>  	return err;
>  }
>  
> @@ -830,7 +808,7 @@ repo_read_dispatch(int fd, short event, void *arg)
>  				    repo_read.title, err->msg);
>  				break;
>  			}
> -			if (client->pack_pipe[1] == -1)
> +			if (client->pack_pipe == -1)
>  				break;
>  			err = send_packfile(client, &imsg, iev);
>  			if (err)
> blob - 07ef39056b1ae8681d10c81839b68eb97c9a1146
> blob + 4f35feba02a39ad0317cd252dc1020e7194747e1
> --- lib/serve.c
> +++ lib/serve.c
> @@ -777,20 +777,29 @@ recv_done(int *packfd, int outfd, struct imsgbuf *ibuf
>  	if (err)
>  		return err;
>  
> -	err = gotd_imsg_poll_recv(&imsg, ibuf, 0);
> -	if (err)
> -		return err;
> +	while (*packfd == -1 && err == NULL) {
> +		err = gotd_imsg_poll_recv(&imsg, ibuf, 0);
> +		if (err)
> +			break;
>  
> -	if (imsg.hdr.type == GOTD_IMSG_ERROR)
> -		err = gotd_imsg_recv_error(NULL, &imsg);
> -	else if (imsg.hdr.type != GOTD_IMSG_PACKFILE_PIPE)
> -		err = got_error(GOT_ERR_PRIVSEP_MSG);
> -	else if (imsg.fd == -1)
> -		err = got_error(GOT_ERR_PRIVSEP_NO_FD);
> -	if (err == NULL)
> -		*packfd = imsg.fd;
> +		switch (imsg.hdr.type) {
> +		case GOTD_IMSG_ERROR:
> +			err = gotd_imsg_recv_error(NULL, &imsg);
> +			break;
> +		case GOTD_IMSG_PACKFILE_PIPE:
> +			if (imsg.fd != -1)
> +				*packfd = imsg.fd;
> +			else
> +				err = got_error(GOT_ERR_PRIVSEP_NO_FD);
> +			break;
> +		default:
> +			err = got_error(GOT_ERR_PRIVSEP_MSG);
> +			break;
> +		}
>  
> -	imsg_free(&imsg);
> +		imsg_free(&imsg);
> +	}
> +
>  	return err;
>  }
>  
> 

-- 

Tracey Emery