From: Rafael Sadowski Subject: relayd: check proc_compose return values To: tech@openbsd.org Date: Sat, 25 Jul 2026 21:05:33 +0200 OK? commit d7c2b3a44c48d2759fa510be891cefd49dece2ea Author: Rafael Sadowski Date: Sat Jul 25 20:46:57 2026 +0200 relayd: check proc_compose return values Fatal on config, setup, and startup imsg send failures. Inconsistent state means we can't recover. Warn on per-request responses and sync messages. A single dropped message shouldn't kill the whole process. diff --git a/agentx_control.c b/agentx_control.c index 7af1c0e..a6f5ef4 100644 --- a/agentx_control.c +++ b/agentx_control.c @@ -427,9 +427,11 @@ agentx_init(struct relayd *nenv) void agentx_nofd(struct agentx *usa, void *cookie, int close) { - if (!close) - proc_compose(env->sc_ps, PROC_PARENT, IMSG_AGENTXSOCK, NULL, 0); - else { + if (!close) { + if (proc_compose(env->sc_ps, PROC_PARENT, IMSG_AGENTXSOCK, + NULL, 0) == -1) + log_warn("%s: proc_compose", __func__); + } else { sa = NULL; agentx_init(env); event_del(&(env->sc_agentxev)); @@ -456,7 +458,9 @@ agentx_setsock(struct relayd *lenv, enum privsep_procid id) s = -1; } done: - proc_compose_imsg(lenv->sc_ps, id, -1, IMSG_AGENTXSOCK, -1, s, NULL, 0); + if (proc_compose_imsg(lenv->sc_ps, id, -1, IMSG_AGENTXSOCK, -1, s, + NULL, 0) == -1) + log_warn("%s: proc_compose_imsg", __func__); } void @@ -485,7 +489,9 @@ void agentx_sock(int fd, short event, void *arg) { if (event & EV_TIMEOUT) { - proc_compose(env->sc_ps, PROC_PARENT, IMSG_AGENTXSOCK, NULL, 0); + if (proc_compose(env->sc_ps, PROC_PARENT, IMSG_AGENTXSOCK, + NULL, 0) == -1) + log_warn("%s: proc_compose", __func__); return; } if (event & EV_WRITE) { diff --git a/config.c b/config.c index b273fd7..b4ec0aa 100644 --- a/config.c +++ b/config.c @@ -230,7 +230,9 @@ config_setreset(struct relayd *env, u_int reset) if ((reset & ps->ps_what[id]) == 0 || id == privsep_process) continue; - proc_compose(ps, id, IMSG_CTL_RESET, &reset, sizeof(reset)); + if (proc_compose(ps, id, IMSG_CTL_RESET, &reset, + sizeof(reset)) == -1) + fatal("%s: proc_compose", __func__); /* * XXX Make sure that the reset message is sent @@ -294,7 +296,9 @@ config_getcfg(struct relayd *env, struct imsg *imsg) } if (privsep_process != PROC_PARENT) - proc_compose(env->sc_ps, PROC_PARENT, IMSG_CFG_DONE, NULL, 0); + if (proc_compose(env->sc_ps, PROC_PARENT, IMSG_CFG_DONE, NULL, + 0) == -1) + fatal("%s: proc_compose", __func__); return (0); } @@ -327,11 +331,13 @@ config_settable(struct relayd *env, struct table *tb) iov[c++].iov_len = strlen(tb->sendbuf); } - proc_composev(ps, id, IMSG_CFG_TABLE, iov, c); + if (proc_composev(ps, id, IMSG_CFG_TABLE, iov, c) == -1) + fatal("%s: proc_composev", __func__); TAILQ_FOREACH(host, &tb->hosts, entry) { - proc_compose(ps, id, IMSG_CFG_HOST, - &host->conf, sizeof(host->conf)); + if (proc_compose(ps, id, IMSG_CFG_HOST, &host->conf, + sizeof(host->conf)) == -1) + fatal("%s: proc_compose", __func__); } } @@ -445,13 +451,15 @@ config_setrdr(struct relayd *env, struct rdr *rdr) DPRINTF("%s: sending rdr %s to %s", __func__, rdr->conf.name, ps->ps_title[id]); - proc_compose(ps, id, IMSG_CFG_RDR, - &rdr->conf, sizeof(rdr->conf)); + if (proc_compose(ps, id, IMSG_CFG_RDR, &rdr->conf, + sizeof(rdr->conf)) == -1) + fatal("%s: proc_compose", __func__); TAILQ_FOREACH(virt, &rdr->virts, entry) { virt->rdrid = rdr->conf.id; - proc_compose(ps, id, IMSG_CFG_VIRT, - virt, sizeof(*virt)); + if (proc_compose(ps, id, IMSG_CFG_VIRT, virt, + sizeof(*virt)) == -1) + fatal("%s: proc_compose", __func__); } } @@ -539,12 +547,14 @@ config_setrt(struct relayd *env, struct router *rt) DPRINTF("%s: sending router %s to %s tbl %d", __func__, rt->rt_conf.name, ps->ps_title[id], rt->rt_conf.gwtable); - proc_compose(ps, id, IMSG_CFG_ROUTER, - &rt->rt_conf, sizeof(rt->rt_conf)); + if (proc_compose(ps, id, IMSG_CFG_ROUTER, + &rt->rt_conf, sizeof(rt->rt_conf)) == -1) + fatal("%s: proc_compose", __func__); TAILQ_FOREACH(nr, &rt->rt_netroutes, nr_entry) { - proc_compose(ps, id, IMSG_CFG_ROUTE, - &nr->nr_conf, sizeof(nr->nr_conf)); + if (proc_compose(ps, id, IMSG_CFG_ROUTE, + &nr->nr_conf, sizeof(nr->nr_conf)) == -1) + fatal("%s: proc_compose", __func__); } } @@ -650,7 +660,8 @@ config_setproto(struct relayd *env, struct protocol *proto) iov[c++].iov_len = strlen(proto->style) + 1; } - proc_composev(ps, id, IMSG_CFG_PROTO, iov, c); + if (proc_composev(ps, id, IMSG_CFG_PROTO, iov, c) == -1) + fatal("%s: proc_composev", __func__); } return (0); @@ -701,7 +712,8 @@ config_setrule(struct relayd *env, struct protocol *proto) rule->rule_ctl.kvlen[i].value = -1; } - proc_composev(ps, id, IMSG_CFG_RULE, iov, c); + if (proc_composev(ps, id, IMSG_CFG_RULE, iov, c) == -1) + fatal("%s: proc_composev", __func__); } } @@ -1012,7 +1024,9 @@ config_setrelay(struct relayd *env, struct relay *rlay) iov[c].iov_base = &crt; iov[c++].iov_len = sizeof(crt); - proc_composev(ps, id, IMSG_CFG_RELAY_TABLE, iov, c); + if (proc_composev(ps, id, IMSG_CFG_RELAY_TABLE, iov, + c) == -1) + fatal("%s: proc_composev", __func__); } } diff --git a/control.c b/control.c index 1d4a3e6..3757bd0 100644 --- a/control.c +++ b/control.c @@ -362,8 +362,9 @@ control_dispatch_imsg(int fd, short event, void *arg) proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT); break; case IMSG_CTL_POLL: - proc_compose(env->sc_ps, PROC_HCE, - IMSG_CTL_POLL, NULL, 0); + 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); break; diff --git a/hce.c b/hce.c index 05c1555..af4e7b0 100644 --- a/hce.c +++ b/hce.c @@ -136,7 +136,8 @@ hce_launch_checks(int fd, short event, void *arg) /* * notify pfe checks are done and schedule next check */ - proc_compose(env->sc_ps, PROC_PFE, IMSG_SYNC, NULL, 0); + if (proc_compose(env->sc_ps, PROC_PFE, IMSG_SYNC, NULL, 0) == -1) + log_warn("%s: proc_compose", __func__); TAILQ_FOREACH(table, env->sc_tables, entry) { TAILQ_FOREACH(host, &table->hosts, entry) { if ((host->flags & F_CHECK_DONE) == 0) @@ -246,7 +247,9 @@ hce_notify_done(struct host *host, enum host_error he) if (msg) log_debug("%s: %s (%s)", __func__, host->conf.name, msg); - proc_compose(env->sc_ps, PROC_PFE, IMSG_HOST_STATUS, &st, sizeof(st)); + if (proc_compose(env->sc_ps, PROC_PFE, IMSG_HOST_STATUS, &st, + sizeof(st)) == -1) + log_warn("%s: proc_compose", __func__); if (host->up != host->last_up) logopt = RELAYD_OPT_LOGUPDATE; diff --git a/pfe.c b/pfe.c index c66ad06..d527e3c 100644 --- a/pfe.c +++ b/pfe.c @@ -151,8 +151,9 @@ pfe_dispatch_hce(int fd, struct privsep_proc *p, struct imsg *imsg) break; /* Forward to relay engine(s) */ - proc_compose(env->sc_ps, PROC_RELAY, - IMSG_HOST_STATUS, &st, sizeof(st)); + if (proc_compose(env->sc_ps, PROC_RELAY, IMSG_HOST_STATUS, &st, + sizeof(st)) == -1) + log_warn("%s: proc_compose", __func__); if ((table = table_find(env, host->conf.tableid)) == NULL) @@ -279,8 +280,9 @@ pfe_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg) fatalx("%s: invalid relay proc", __func__); if (natlook(env, &cnl) != 0) cnl.in = -1; - proc_compose_imsg(env->sc_ps, PROC_RELAY, cnl.proc, - IMSG_NATLOOK, -1, -1, &cnl, sizeof(cnl)); + if (proc_compose_imsg(env->sc_ps, PROC_RELAY, cnl.proc, + IMSG_NATLOOK, -1, -1, &cnl, sizeof(cnl)) == -1) + log_warn("%s: proc_compose_imsg", __func__); break; case IMSG_STATISTICS: if (imsg_get_data(imsg, &crs, sizeof(crs)) == -1) @@ -455,8 +457,9 @@ show_sessions(struct ctl_conn *c) /* * Request all the running sessions from the process */ - proc_compose_imsg(env->sc_ps, PROC_RELAY, proc, - IMSG_CTL_SESSION, -1, -1, &cid, sizeof(cid)); + if (proc_compose_imsg(env->sc_ps, PROC_RELAY, proc, + IMSG_CTL_SESSION, -1, -1, &cid, sizeof(cid)) == -1) + log_warn("%s: proc_compose_imsg", __func__); c->waiting++; } } @@ -544,12 +547,14 @@ disable_table(struct ctl_conn *c, struct ctl_id *id) table->up = 0; TAILQ_FOREACH(host, &table->hosts, entry) host->up = HOST_UNKNOWN; - proc_compose(env->sc_ps, PROC_HCE, IMSG_TABLE_DISABLE, - &table->conf.id, sizeof(table->conf.id)); + if (proc_compose(env->sc_ps, PROC_HCE, IMSG_TABLE_DISABLE, + &table->conf.id, sizeof(table->conf.id)) == -1) + log_warn("%s: proc_compose", __func__); /* Forward to relay engine(s) */ - proc_compose(env->sc_ps, PROC_RELAY, IMSG_TABLE_DISABLE, - &table->conf.id, sizeof(table->conf.id)); + if (proc_compose(env->sc_ps, PROC_RELAY, IMSG_TABLE_DISABLE, + &table->conf.id, sizeof(table->conf.id)) == -1) + log_warn("%s: proc_compose", __func__); log_debug("%s: table %d", __func__, table->conf.id); pfe_sync(); @@ -580,12 +585,14 @@ enable_table(struct ctl_conn *c, struct ctl_id *id) table->up = 0; TAILQ_FOREACH(host, &table->hosts, entry) host->up = HOST_UNKNOWN; - proc_compose(env->sc_ps, PROC_HCE, IMSG_TABLE_ENABLE, - &table->conf.id, sizeof(table->conf.id)); + if (proc_compose(env->sc_ps, PROC_HCE, IMSG_TABLE_ENABLE, + &table->conf.id, sizeof(table->conf.id)) == -1) + log_warn("%s: proc_compose", __func__); /* Forward to relay engine(s) */ - proc_compose(env->sc_ps, PROC_RELAY, IMSG_TABLE_ENABLE, - &table->conf.id, sizeof(table->conf.id)); + if (proc_compose(env->sc_ps, PROC_RELAY, IMSG_TABLE_ENABLE, + &table->conf.id, sizeof(table->conf.id)) == -1) + log_warn("%s: proc_compose", __func__); log_debug("%s: table %d", __func__, table->conf.id); pfe_sync(); @@ -627,12 +634,15 @@ disable_host(struct ctl_conn *c, struct ctl_id *id, struct host *host) host->check_cnt = 0; host->up_cnt = 0; - proc_compose(env->sc_ps, PROC_HCE, IMSG_HOST_DISABLE, - &host->conf.id, sizeof(host->conf.id)); + if (proc_compose(env->sc_ps, PROC_HCE, IMSG_HOST_DISABLE, + &host->conf.id, sizeof(host->conf.id)) == -1) + log_warn("%s: proc_compose", __func__); /* Forward to relay engine(s) */ - proc_compose(env->sc_ps, PROC_RELAY, IMSG_HOST_DISABLE, - &host->conf.id, sizeof(host->conf.id)); + if (proc_compose(env->sc_ps, PROC_RELAY, IMSG_HOST_DISABLE, + &host->conf.id, sizeof(host->conf.id)) == -1) + log_warn("%s: proc_compose", __func__); + log_debug("%s: host %d", __func__, host->conf.id); if (!host->conf.parentid) { @@ -680,12 +690,14 @@ enable_host(struct ctl_conn *c, struct ctl_id *id, struct host *host) host->flags &= ~(F_DEL); host->flags &= ~(F_ADD); - proc_compose(env->sc_ps, PROC_HCE, IMSG_HOST_ENABLE, - &host->conf.id, sizeof(host->conf.id)); + if (proc_compose(env->sc_ps, PROC_HCE, IMSG_HOST_ENABLE, + &host->conf.id, sizeof(host->conf.id)) == -1) + log_warn("%s: proc_compose", __func__); /* Forward to relay engine(s) */ - proc_compose(env->sc_ps, PROC_RELAY, IMSG_HOST_ENABLE, - &host->conf.id, sizeof(host->conf.id)); + if (proc_compose(env->sc_ps, PROC_RELAY, IMSG_HOST_ENABLE, + &host->conf.id, sizeof(host->conf.id)) == -1) + log_warn("%s: proc_compose", __func__); log_debug("%s: host %d", __func__, host->conf.id); @@ -808,8 +820,9 @@ pfe_sync(void) demote.level, table->conf.name, table->conf.demote_group); (void)strlcpy(demote.group, table->conf.demote_group, sizeof(demote.group)); - proc_compose(env->sc_ps, PROC_PARENT, IMSG_DEMOTE, - &demote, sizeof(demote)); + if (proc_compose(env->sc_ps, PROC_PARENT, IMSG_DEMOTE, + &demote, sizeof(demote)) == -1) + log_warn("%s: proc_compose", __func__); } } diff --git a/pfe_route.c b/pfe_route.c index 5e0044c..aad82e4 100644 --- a/pfe_route.c +++ b/pfe_route.c @@ -82,8 +82,10 @@ sync_routes(struct relayd *env, struct router *rt) memcpy(&crt.host, &host->conf, sizeof(host->conf)); memcpy(&crt.rt, &rt->rt_conf, sizeof(rt->rt_conf)); - proc_compose(env->sc_ps, PROC_PARENT, - IMSG_RTMSG, &crt, sizeof(crt)); + if (proc_compose(env->sc_ps, PROC_PARENT, IMSG_RTMSG, + &crt, sizeof(crt)) == -1) { + log_warn("%s: proc_compose", __func__); + } } } } diff --git a/relay.c b/relay.c index 58e57b7..8469f8d 100644 --- a/relay.c +++ b/relay.c @@ -359,14 +359,17 @@ relay_init(struct privsep *ps, struct privsep_proc *p, void *arg) void relay_session_publish(struct rsession *s) { - proc_compose(env->sc_ps, PROC_PFE, IMSG_SESS_PUBLISH, s, sizeof(*s)); + if (proc_compose(env->sc_ps, PROC_PFE, IMSG_SESS_PUBLISH, s, + sizeof(*s)) == -1) + log_warn("%s: proc_compose", __func__); } void relay_session_unpublish(struct rsession *s) { - proc_compose(env->sc_ps, PROC_PFE, IMSG_SESS_UNPUBLISH, - &s->se_id, sizeof(s->se_id)); + if (proc_compose(env->sc_ps, PROC_PFE, IMSG_SESS_UNPUBLISH, + &s->se_id, sizeof(s->se_id)) == -1) + log_warn("%s: proc_compose", __func__); } void @@ -417,8 +420,9 @@ relay_statistics(int fd, short events, void *arg) crs.id = rlay->rl_conf.id; crs.proc = ps->ps_instance; - proc_compose(env->sc_ps, PROC_PFE, IMSG_STATISTICS, - &crs, sizeof(crs)); + if (proc_compose(env->sc_ps, PROC_PFE, IMSG_STATISTICS, &crs, + sizeof(crs)) == -1) + log_warn("%s: proc_compose", __func__); for (con = SPLAY_ROOT(&rlay->rl_sessions); con != NULL; con = next_con) { @@ -1233,8 +1237,9 @@ relay_accept(int fd, short event, void *arg) memcpy(&cnl->src, &con->se_in.ss, sizeof(cnl->src)); memcpy(&cnl->dst, &con->se_sockname, sizeof(cnl->dst)); - proc_compose(env->sc_ps, PROC_PFE, IMSG_NATLOOK, - cnl, sizeof(*cnl)); + if (proc_compose(env->sc_ps, PROC_PFE, IMSG_NATLOOK, + cnl, sizeof(*cnl)) == -1) + log_warn("%s: proc_compose", __func__); /* Schedule timeout */ evtimer_set(&con->se_ev, relay_natlook, con); @@ -1492,8 +1497,9 @@ relay_bindanyreq(struct rsession *con, in_port_t port, int proto) bnd.bnd_port = port; bnd.bnd_proto = proto; bcopy(&con->se_in.ss, &bnd.bnd_ss, sizeof(bnd.bnd_ss)); - proc_compose(env->sc_ps, PROC_PARENT, IMSG_BINDANY, - &bnd, sizeof(bnd)); + if (proc_compose(env->sc_ps, PROC_PARENT, IMSG_BINDANY, + &bnd, sizeof(bnd)) == -1) + log_warn("%s: proc_compose", __func__); /* Schedule timeout */ evtimer_set(&con->se_ev, relay_bindany, con); @@ -1815,8 +1821,9 @@ relay_close(struct rsession *con, const char *msg, int err) if (con->se_cnl != NULL) { #if 0 - proc_compose_imsg(env->sc_ps, PROC_PFE, -1, IMSG_KILLSTATES, -1, - cnl, sizeof(*cnl)); + if (proc_compose_imsg(env->sc_ps, PROC_PFE, -1, + IMSG_KILLSTATES, -1, cnl, sizeof(*cnl)) == -1) + log_warn("%s: proc_compose_imsg", __func__); #endif free(con->se_cnl); } @@ -1965,12 +1972,14 @@ relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg) &rlay->rl_sessions) { memcpy(&se, con, sizeof(se)); se.se_cid = cid; - proc_compose(env->sc_ps, p->p_id, - IMSG_CTL_SESSION, &se, sizeof(se)); + if (proc_compose(env->sc_ps, p->p_id, + IMSG_CTL_SESSION, &se, sizeof(se)) == -1) + log_warn("%s: proc_compose", __func__); } } - proc_compose(env->sc_ps, p->p_id, IMSG_CTL_END, - &cid, sizeof(cid)); + if (proc_compose(env->sc_ps, p->p_id, IMSG_CTL_END, + &cid, sizeof(cid)) == -1) + log_warn("%s: proc_compose", __func__); break; default: return (-1); diff --git a/relay_udp.c b/relay_udp.c index 7226861..7403fac 100644 --- a/relay_udp.c +++ b/relay_udp.c @@ -316,8 +316,9 @@ relay_udp_server(int fd, short sig, void *arg) cnl->proto = IPPROTO_UDP; bcopy(&con->se_in.ss, &cnl->src, sizeof(cnl->src)); bcopy(&rlay->rl_conf.ss, &cnl->dst, sizeof(cnl->dst)); - proc_compose(env->sc_ps, PROC_PFE, - IMSG_NATLOOK, cnl, sizeof(*cnl)); + if (proc_compose(env->sc_ps, PROC_PFE, IMSG_NATLOOK, cnl, + sizeof(*cnl)) == -1) + log_warn("%s: proc_compose", __func__); /* Schedule timeout */ evtimer_set(&con->se_ev, relay_natlook, con); diff --git a/relayd.c b/relayd.c index b174fc8..5de0cc1 100644 --- a/relayd.c +++ b/relayd.c @@ -314,8 +314,9 @@ parent_configure(struct relayd *env) for (id = 0; id < PROC_MAX; id++) { if (id == privsep_process) continue; - proc_compose_imsg(env->sc_ps, id, -1, IMSG_CFG_DONE, -1, - -1, &env->sc_conf, sizeof(env->sc_conf)); + if (proc_compose_imsg(env->sc_ps, id, -1, IMSG_CFG_DONE, -1, + -1, &env->sc_conf, sizeof(env->sc_conf)) == -1) + fatal("%s: proc_compose_imsg", __func__); } ret = 0; @@ -373,7 +374,9 @@ parent_configure_done(struct relayd *env) if (id == privsep_process) continue; - proc_compose(env->sc_ps, id, IMSG_CTL_START, NULL, 0); + if (proc_compose(env->sc_ps, id, IMSG_CTL_START, NULL, + 0) == -1) + fatal("%s: proc_compose", __func__); } } } @@ -478,7 +481,9 @@ parent_dispatch_hce(int fd, struct privsep_proc *p, struct imsg *imsg) scr.name[sizeof(scr.name) - 1] = '\0'; scr.path[sizeof(scr.path) - 1] = '\0'; scr.retval = script_exec(env, &scr); - proc_compose(ps, PROC_HCE, IMSG_SCRIPT, &scr, sizeof(scr)); + if (proc_compose(ps, PROC_HCE, IMSG_SCRIPT, &scr, + sizeof(scr)) == -1) + log_warn("%s: proc_compose", __func__); break; case IMSG_CFG_DONE: parent_configure_done(env); @@ -518,8 +523,9 @@ parent_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg) /* NOTREACHED */ } s = bindany(&bnd); - proc_compose_imsg(ps, PROC_RELAY, bnd.bnd_proc, - IMSG_BINDANY, -1, s, &bnd.bnd_id, sizeof(bnd.bnd_id)); + if (proc_compose_imsg(ps, PROC_RELAY, bnd.bnd_proc, + IMSG_BINDANY, -1, s, &bnd.bnd_id, sizeof(bnd.bnd_id)) == -1) + log_warn("%s: proc_compose_imsg", __func__); break; case IMSG_CFG_DONE: parent_configure_done(env); @@ -1931,8 +1937,9 @@ parent_tls_ticket_rekey(int fd, short events, void *arg) key.tt_keyrev = arc4random(); arc4random_buf(key.tt_key, sizeof(key.tt_key)); - proc_compose_imsg(env->sc_ps, PROC_RELAY, -1, IMSG_TLSTICKET_REKEY, - -1, -1, &key, sizeof(key)); + if (proc_compose_imsg(env->sc_ps, PROC_RELAY, -1, IMSG_TLSTICKET_REKEY, + -1, -1, &key, sizeof(key)) == -1) + log_warn("%s: proc_compose_imsg", __func__); evtimer_set(&rekeyev, parent_tls_ticket_rekey, env); timerclear(&tv);