Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: relayd: check imsg_compose_event return values
To:
Rafael Sadowski <rafael@sizeofvoid.org>
Cc:
tech@openbsd.org
Date:
Wed, 12 Aug 2026 00:26:13 +0200

Download raw body.

Thread
On Sun, 26 Jul 2026 11:14:47 +0200,
Rafael Sadowski <rafael@sizeofvoid.org> wrote:
> 
> Also remove double imsg_compose_event declaration. OK?
> 

reads OK kirill@


> commit 05b0b40200dd6d68728f133742e0e7bed8b5f28e
> Author: Rafael Sadowski <rafael@sizeofvoid.org>
> Date:   Sun Jul 26 09:49:41 2026 +0200
> 
>     relayd: check imsg_compose_event return values
> 
> diff --git a/control.c b/control.c
> index 3757bd0..54e9227 100644
> --- a/control.c
> +++ b/control.c
> @@ -283,78 +283,102 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
>  				fatalx("%s: imsg_get_data", __func__);
> 
> -			if (disable_rdr(c, &id))
> -				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> -			else {
> +			if (disable_rdr(c, &id)) {
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
> +			} else {
>  				control_imsg_forward(&imsg);
> -				imsg_compose_event(&c->iev, IMSG_CTL_OK,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_OK,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  			}
>  			break;
>  		case IMSG_CTL_RDR_ENABLE:
>  			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
>  				fatalx("%s: imsg_get_data", __func__);
> 
> -			if (enable_rdr(c, &id))
> -				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> -			else {
> +			if (enable_rdr(c, &id)) {
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
> +			} else {
>  				control_imsg_forward(&imsg);
> -				imsg_compose_event(&c->iev, IMSG_CTL_OK,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_OK,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  			}
>  			break;
>  		case IMSG_CTL_TABLE_DISABLE:
>  			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
>  				fatalx("%s: imsg_get_data", __func__);
> 
> -			if (disable_table(c, &id))
> -				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> -			else {
> +			if (disable_table(c, &id)) {
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
> +			} else {
>  				control_imsg_forward(&imsg);
> -				imsg_compose_event(&c->iev, IMSG_CTL_OK,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_OK,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  			}
>  			break;
>  		case IMSG_CTL_TABLE_ENABLE:
>  			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
>  				fatalx("%s: imsg_get_data", __func__);
> 
> -			if (enable_table(c, &id))
> -				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> -			else {
> +			if (enable_table(c, &id)) {
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
> +			} else {
>  				control_imsg_forward(&imsg);
> -				imsg_compose_event(&c->iev, IMSG_CTL_OK,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_OK,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  			}
>  			break;
>  		case IMSG_CTL_HOST_DISABLE:
>  			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
>  				fatalx("%s: imsg_get_data", __func__);
> 
> -			if (disable_host(c, &id, NULL))
> -				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> -			else {
> +			if (disable_host(c, &id, NULL)) {
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
> +			} else {
>  				control_imsg_forward(&imsg);
> -				imsg_compose_event(&c->iev, IMSG_CTL_OK,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_OK,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  			}
>  			break;
>  		case IMSG_CTL_HOST_ENABLE:
>  			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
>  				fatalx("%s: imsg_get_data", __func__);
> 
> -			if (enable_host(c, &id, NULL))
> -				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> -			else {
> +			if (enable_host(c, &id, NULL)) {
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
> +			} else {
>  				control_imsg_forward(&imsg);
> -				imsg_compose_event(&c->iev, IMSG_CTL_OK,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_OK,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  			}
>  			break;
>  		case IMSG_CTL_SHUTDOWN:
> @@ -365,16 +389,19 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  			if (proc_compose(env->sc_ps, PROC_HCE, IMSG_CTL_POLL,
>  			    NULL, 0) == -1)
>  				log_warn("%s: proc_compose", __func__);
> -			imsg_compose_event(&c->iev, IMSG_CTL_OK,
> -			    0, ps->ps_instance + 1, -1, NULL, 0);
> +			if (imsg_compose_event(&c->iev, IMSG_CTL_OK,
> +			    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +				log_warn("%s: imsg_compose_event", __func__);
>  			break;
>  		case IMSG_CTL_NOTIFY:
>  			if (c->flags & CTL_CONN_NOTIFY) {
>  				log_debug("%s: "
>  				    "client requested notify more than once",
>  				    __func__);
> -				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> -				    0, ps->ps_instance + 1, -1, NULL, 0);
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
> +				    0, ps->ps_instance + 1, -1, NULL, 0) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  				break;
>  			}
>  			c->flags |= CTL_CONN_NOTIFY;
> diff --git a/pfe.c b/pfe.c
> index d527e3c..e62ed05 100644
> --- a/pfe.c
> +++ b/pfe.c
> @@ -303,9 +303,9 @@ pfe_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
>  			    __func__, con.se_cid);
>  			return (0);
>  		}
> -		imsg_compose_event(&c->iev,
> -		    IMSG_CTL_SESSION, 0, 0, -1,
> -		    &con, sizeof(con));
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_SESSION, 0, 0, -1,
> +		    &con, sizeof(con)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
>  		break;
>  	case IMSG_CTL_END:
>  		if (imsg_get_data(imsg, &cid, sizeof(cid)) == -1)
> @@ -320,8 +320,9 @@ pfe_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
>  			return (0);
>  		} else if (--c->waiting == 0) {
>  			/* Last ack for a previous request */
> -			imsg_compose_event(&c->iev, IMSG_CTL_END,
> -			    0, 0, -1, NULL, 0);
> +			if (imsg_compose_event(&c->iev, IMSG_CTL_END, 0, 0, -1,
> +			    NULL, 0) == -1)
> +				log_warn("%s: imsg_compose_event", __func__);
>  		}
>  		break;
>  	case IMSG_SESS_PUBLISH:
> @@ -376,49 +377,64 @@ show(struct ctl_conn *c)
>  	if (env->sc_rdrs == NULL)
>  		goto relays;
>  	TAILQ_FOREACH(rdr, env->sc_rdrs, entry) {
> -		imsg_compose_event(&c->iev, IMSG_CTL_RDR, 0, 0, -1,
> -		    rdr, sizeof(*rdr));
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_RDR, 0, 0, -1,
> +		    rdr, sizeof(*rdr)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
>  		if (rdr->conf.flags & F_DISABLE)
>  			continue;
> 
> -		imsg_compose_event(&c->iev, IMSG_CTL_RDR_STATS, 0, 0, -1,
> -		    &rdr->stats, sizeof(rdr->stats));
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_RDR_STATS, 0, 0, -1,
> +		    &rdr->stats, sizeof(rdr->stats)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
> +
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_TABLE, 0, 0, -1,
> +		    rdr->table, sizeof(*rdr->table)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
> 
> -		imsg_compose_event(&c->iev, IMSG_CTL_TABLE, 0, 0, -1,
> -		    rdr->table, sizeof(*rdr->table));
>  		if (!(rdr->table->conf.flags & F_DISABLE))
>  			TAILQ_FOREACH(host, &rdr->table->hosts, entry)
> -				imsg_compose_event(&c->iev, IMSG_CTL_HOST,
> -				    0, 0, -1, host, sizeof(*host));
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_HOST,
> +				    0, 0, -1, host, sizeof(*host)) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
> 
>  		if (rdr->backup->conf.id == EMPTY_TABLE)
>  			continue;
> -		imsg_compose_event(&c->iev, IMSG_CTL_TABLE, 0, 0, -1,
> -		    rdr->backup, sizeof(*rdr->backup));
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_TABLE, 0, 0, -1,
> +		    rdr->backup, sizeof(*rdr->backup)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
>  		if (!(rdr->backup->conf.flags & F_DISABLE))
>  			TAILQ_FOREACH(host, &rdr->backup->hosts, entry)
> -				imsg_compose_event(&c->iev, IMSG_CTL_HOST,
> -				    0, 0, -1, host, sizeof(*host));
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_HOST,
> +				    0, 0, -1, host, sizeof(*host)) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  	}
>  relays:
>  	if (env->sc_relays == NULL)
>  		goto routers;
>  	TAILQ_FOREACH(rlay, env->sc_relays, rl_entry) {
>  		rlay->rl_stats[env->sc_conf.prefork_relay].id = EMPTY_ID;
> -		imsg_compose_event(&c->iev, IMSG_CTL_RELAY, 0, 0, -1,
> -		    rlay, sizeof(*rlay));
> -		imsg_compose_event(&c->iev, IMSG_CTL_RELAY_STATS, 0, 0, -1,
> -		    &rlay->rl_stats, sizeof(rlay->rl_stats));
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_RELAY, 0, 0, -1,
> +		    rlay, sizeof(*rlay)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
> +
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_RELAY_STATS, 0, 0, -1,
> +		    &rlay->rl_stats, sizeof(rlay->rl_stats)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
> 
>  		TAILQ_FOREACH(rlt, &rlay->rl_tables, rlt_entry) {
> -			imsg_compose_event(&c->iev, IMSG_CTL_TABLE, 0, 0, -1,
> -			    rlt->rlt_table, sizeof(*rlt->rlt_table));
> +			if (imsg_compose_event(&c->iev, IMSG_CTL_TABLE, 0, 0,
> +			    -1, rlt->rlt_table, sizeof(*rlt->rlt_table)) == -1)
> +				log_warn("%s: imsg_compose_event", __func__);
>  			if (!(rlt->rlt_table->conf.flags & F_DISABLE))
>  				TAILQ_FOREACH(host,
>  				    &rlt->rlt_table->hosts, entry)
> -					imsg_compose_event(&c->iev,
> +					if (imsg_compose_event(&c->iev,
>  					    IMSG_CTL_HOST, 0, 0, -1,
> -					    host, sizeof(*host));
> +					    host, sizeof(*host)) == -1)
> +						log_warn("%s: imsg_compose_event",
> +						    __func__);
>  		}
>  	}
> 
> @@ -426,24 +442,31 @@ routers:
>  	if (env->sc_rts == NULL)
>  		goto end;
>  	TAILQ_FOREACH(rt, env->sc_rts, rt_entry) {
> -		imsg_compose_event(&c->iev, IMSG_CTL_ROUTER, 0, 0, -1,
> -		    rt, sizeof(*rt));
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_ROUTER, 0, 0, -1,
> +		    rt, sizeof(*rt)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
>  		if (rt->rt_conf.flags & F_DISABLE)
>  			continue;
> 
>  		TAILQ_FOREACH(nr, &rt->rt_netroutes, nr_entry)
> -			imsg_compose_event(&c->iev, IMSG_CTL_NETROUTE,
> -			    0, 0, -1, nr, sizeof(*nr));
> -		imsg_compose_event(&c->iev, IMSG_CTL_TABLE, 0, 0, -1,
> -		    rt->rt_gwtable, sizeof(*rt->rt_gwtable));
> +			if (imsg_compose_event(&c->iev, IMSG_CTL_NETROUTE,
> +			    0, 0, -1, nr, sizeof(*nr)) == -1)
> +				log_warn("%s: imsg_compose_event", __func__);
> +		if (imsg_compose_event(&c->iev, IMSG_CTL_TABLE, 0, 0, -1,
> +		    rt->rt_gwtable, sizeof(*rt->rt_gwtable)) == -1)
> +			log_warn("%s: imsg_compose_event", __func__);
> +
>  		if (!(rt->rt_gwtable->conf.flags & F_DISABLE))
>  			TAILQ_FOREACH(host, &rt->rt_gwtable->hosts, entry)
> -				imsg_compose_event(&c->iev, IMSG_CTL_HOST,
> -				    0, 0, -1, host, sizeof(*host));
> +				if (imsg_compose_event(&c->iev, IMSG_CTL_HOST,
> +				    0, 0, -1, host, sizeof(*host)) == -1)
> +					log_warn("%s: imsg_compose_event",
> +					    __func__);
>  	}
> 
>  end:
> -	imsg_compose_event(&c->iev, IMSG_CTL_END, 0, 0, -1, NULL, 0);
> +	if (imsg_compose_event(&c->iev, IMSG_CTL_END, 0, 0, -1, NULL, 0) == -1)
> +		log_warn("%s: imsg_compose_event", __func__);
>  }
> 
>  void
> diff --git a/relayd.h b/relayd.h
> index 2aa6eab..4716091 100644
> --- a/relayd.h
> +++ b/relayd.h
> @@ -1332,9 +1332,6 @@ int			 map6to4(struct sockaddr_storage *);
>  int			 map4to6(struct sockaddr_storage *,
>      struct sockaddr_storage *);
>  void			 imsg_event_add(struct imsgev *);
> -int			 imsg_compose_event(struct imsgev *, u_int16_t,
> -    u_int32_t,
> -    pid_t, int, void *, u_int16_t);
>  void			 socket_rlimit(int);
>  void			*get_data(struct ibuf *, size_t);
>  int			 sockaddr_cmp(struct sockaddr *, struct sockaddr *,
> @@ -1342,8 +1339,7 @@ int			 sockaddr_cmp(struct sockaddr *, struct sockaddr *,
>  struct in6_addr		*prefixlen2mask6(u_int8_t, u_int32_t *);
>  u_int32_t		 prefixlen2mask(u_int8_t);
>  int			 accept_reserve(int, struct sockaddr *, socklen_t *,
> -    int,
> -    volatile int *);
> +    int, volatile int *);
>  struct kv		*kv_add(struct kvtree *, char *, char *, int);
>  int			 kv_set(struct kv *, char *, ...)
>  	__attribute__((__format__(printf, 2, 3)));
> @@ -1430,8 +1426,7 @@ void			 imsg_event_add(struct imsgev *);
>  int			 imsg_compose_event(struct imsgev *, uint16_t, uint32_t,
>      pid_t, int, void *, uint16_t);
>  int			 imsg_composev_event(struct imsgev *, uint16_t,
> -    uint32_t,
> -    pid_t, int, const struct iovec *, int);
> +    uint32_t, pid_t, int, const struct iovec *, int);
> 
>  /* config.c */
>  int	 config_init(struct relayd *);
> 

-- 
wbr, Kirill