From: Tobias Heider <tobias.heider@stusta.de>
Subject: Re: iked: use imsg_get_fd()
To: tech@openbsd.org
Date: Tue, 16 Jan 2024 20:42:44 +0100

On Tue, Jan 16, 2024 at 02:06:02PM +0100, Claudio Jeker wrote:
> As usual iked is a bit more complicated when it comes to imsg handling.
> 
> Most conversions from imsg->fd to imsg_get_fd(imsg) are simple. Sometimes
> an extra variable is needed or code is shuffled a bit.
> 
> proc_forward_imsg() is never used with an imsg that contains a file
> descriptor. So just pass -1 there. This code could be replaced with
> imsg_forward().
> 
> I added an XXX for some code that will no longer be required once
> imsg_free() closes any unclaimed fd.
> 
> -- 
> :wq Claudio

ok

> 
> Index: config.c
> ===================================================================
> RCS file: /cvs/src/sbin/iked/config.c,v
> diff -u -p -r1.94 config.c
> --- config.c	15 Jan 2024 15:29:00 -0000	1.94
> +++ config.c	16 Jan 2024 12:55:20 -0000
> @@ -611,17 +611,17 @@ config_getsocket(struct iked *env, struc
>  {
>  	struct iked_socket	*sock, **sock0 = NULL, **sock1 = NULL;
>  
> -	log_debug("%s: received socket fd %d", __func__, imsg->fd);
> -
>  	if ((sock = calloc(1, sizeof(*sock))) == NULL)
>  		fatal("config_getsocket: calloc");
>  
>  	IMSG_SIZE_CHECK(imsg, &sock->sock_addr);
>  
>  	memcpy(&sock->sock_addr, imsg->data, sizeof(sock->sock_addr));
> -	sock->sock_fd = imsg->fd;
> +	sock->sock_fd = imsg_get_fd(imsg);
>  	sock->sock_env = env;
>  
> +	log_debug("%s: received socket fd %d", __func__, sock->sock_fd);
> +
>  	switch (sock->sock_addr.ss_family) {
>  	case AF_INET:
>  		sock0 = &env->sc_sock4[0];
> @@ -665,8 +665,10 @@ config_setpfkey(struct iked *env)
>  int
>  config_getpfkey(struct iked *env, struct imsg *imsg)
>  {
> -	log_debug("%s: received pfkey fd %d", __func__, imsg->fd);
> -	pfkey_init(env, imsg->fd);
> +	int fd = imsg_get_fd(imsg);
> +
> +	log_debug("%s: received pfkey fd %d", __func__, fd);
> +	pfkey_init(env, fd);
>  	return (0);
>  }
>  
> Index: ocsp.c
> ===================================================================
> RCS file: /cvs/src/sbin/iked/ocsp.c,v
> diff -u -p -r1.24 ocsp.c
> --- ocsp.c	3 Dec 2022 22:34:35 -0000	1.24
> +++ ocsp.c	16 Jan 2024 12:54:51 -0000
> @@ -364,9 +364,7 @@ ocsp_receive_fd(struct iked *env, struct
>  	uint8_t			*ptr;
>  	char			*path = NULL;
>  	size_t			 len;
> -	int			 ret = -1;
> -
> -	log_debug("%s: received socket fd %d", __func__, imsg->fd);
> +	int			 fd, ret = -1;
>  
>  	IMSG_SIZE_CHECK(imsg, &sh);
>  
> @@ -385,30 +383,32 @@ ocsp_receive_fd(struct iked *env, struct
>  	}
>  	if (ioe == NULL) {
>  		log_debug("%s: no pending request found", __func__);
> -		if (imsg->fd != -1)
> -			close(imsg->fd);
> +		if ((fd = imsg_get_fd(imsg)) != -1)	/* XXX */
> +			close(fd);
>  		return (-1);
>  	}
>  	TAILQ_REMOVE(&env->sc_ocsp, ioe, ioe_entry);
>  	ocsp = ioe->ioe_ocsp;
>  	free(ioe);
>  
> -	if (imsg->fd == -1)
> +	if ((fd = imsg_get_fd(imsg)) == -1)
>  		goto done;
>  
>  	if ((sock = calloc(1, sizeof(*sock))) == NULL)
>  		fatal("ocsp_receive_fd: calloc sock");
>  
>  	/* note that sock_addr is not set */
> -	sock->sock_fd = imsg->fd;
> +	sock->sock_fd = fd;
>  	sock->sock_env = env;
>  	ocsp->ocsp_sock = sock;
>  
> +	log_debug("%s: received socket fd %d", __func__, sock->sock_fd);
> +
>  	/* fetch 'path' and 'fd' from imsg */
>  	if ((path = get_string(ptr, len)) == NULL)
>  		goto done;
>  
> -	BIO_set_fd(ocsp->ocsp_cbio, imsg->fd, BIO_NOCLOSE);
> +	BIO_set_fd(ocsp->ocsp_cbio, sock->sock_fd, BIO_NOCLOSE);
>  
>  	if ((ocsp->ocsp_req_ctx = OCSP_sendreq_new(ocsp->ocsp_cbio,
>  	    path, NULL, -1)) == NULL)
> Index: proc.c
> ===================================================================
> RCS file: /cvs/src/sbin/iked/proc.c,v
> diff -u -p -r1.39 proc.c
> --- proc.c	28 Jun 2023 12:31:19 -0000	1.39
> +++ proc.c	13 Dec 2023 11:31:07 -0000
> @@ -667,7 +667,7 @@ proc_dispatch(int fd, short event, void 
>  		case IMSG_CTL_PROCFD:
>  			IMSG_SIZE_CHECK(&imsg, &pf);
>  			memcpy(&pf, imsg.data, sizeof(pf));
> -			proc_accept(ps, imsg.fd, pf.pf_procid,
> +			proc_accept(ps, imsg_get_fd(&imsg), pf.pf_procid,
>  			    pf.pf_instance);
>  			break;
>  		default:
> @@ -798,7 +798,7 @@ proc_forward_imsg(struct privsep *ps, st
>      enum privsep_procid id, int n)
>  {
>  	return (proc_compose_imsg(ps, id, n, imsg->hdr.type,
> -	    imsg->hdr.peerid, imsg->fd, imsg->data, IMSG_DATA_SIZE(imsg)));
> +	    imsg->hdr.peerid, -1, imsg->data, IMSG_DATA_SIZE(imsg)));
>  }
>  
>  struct imsgbuf *
>