Re: relayd: check imsg_compose_event return values

Kirill A. Korinsky <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]> 
On Sun, 26 Jul 2026 11:14:47 +0200,
Rafael Sadowski <[email protected]> wrote:
> 
> Also remove double imsg_compose_event declaration. OK?
> 

reads OK kirill@


> commit 05b0b40200dd6d68728f133742e0e7bed8b5f28e
> Author: Rafael Sadowski <[email protected]>
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.