Download raw body.
[4/7] relayd: read imsg payloads via the new imsg/ibuf getters
[4/7] relayd: read imsg payloads via the new imsg/ibuf getters
On Sun, Jun 07, 2026 at 09:02:37AM +0200, Rafael Sadowski wrote:
> This is a series of commits for an imsg APIv2 rework in relayd. These
> were committed and tested individually. They can be reviewed in their
> entirety here:
>
> gothub: https://rsadowski.gothub.org/?action=summary&headref=imsg_v2&path=relayd.git
> PR: https://codeberg.org/rsadowski/relayd/pulls/1/files
>
> commit a8940b10e7a293b19907c9dbbca397a06d6a01ca
> Author: Rafael Sadowski <rafael@sizeofvoid.org>
> Date: Sat Jun 6 20:03:29 2026 +0200
>
> relayd: read imsg payloads via the new imsg/ibuf getters
>
> Convert the config_get* handlers from IMSG_SIZE_CHECK() + memcpy() to
> the new imsg API. Fixed-size payloads use imsg_get_data(). Functions
> with a fixed header followed by variable-length data use imsg_get_ibuf()
> + ibuf_get() and read the remainder from the same ibuf cursor, since
> imsg_get_data() requires the payload to match the requested size
> exactly.
Some comments below.
> diff --git a/config.c b/config.c
> index ccf0ef4..7ca37d5 100644
> --- a/config.c
> +++ b/config.c
> @@ -250,9 +250,10 @@ int
> config_getreset(struct relayd *env, struct imsg *imsg)
> {
> u_int mode;
> -
> - IMSG_SIZE_CHECK(imsg, &mode);
> - memcpy(&mode, imsg->data, sizeof(mode));
> + if (imsg_get_data(imsg, &mode, sizeof(mode)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + return (-1);
Before this was a fatal error, it no longer is.
This is a common idiom. I think doing there error handling like you did is
actually an improvement.
> + }
>
> config_purge(env, mode);
>
> @@ -267,11 +268,12 @@ config_getcfg(struct relayd *env, struct imsg *imsg)
> struct host *h, *ph;
> u_int what;
>
> - if (IMSG_DATA_SIZE(imsg) != sizeof(struct relayd_config))
> - return (0); /* ignore */
You no longer do this ignore (but I wonder if that is really a good idea
to ignore the length error here).
> -
> /* Update runtime flags */
> - memcpy(&env->sc_conf, imsg->data, sizeof(env->sc_conf));
> + if (imsg_get_data(imsg, &env->sc_conf,
> + sizeof(struct relayd_config)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + return (-1);
> + }
>
> what = ps->ps_what[privsep_process];
>
> @@ -341,21 +343,26 @@ config_settable(struct relayd *env, struct table *tb)
> int
> config_gettable(struct relayd *env, struct imsg *imsg)
> {
> + struct ibuf ibuf;
> struct table *tb;
> size_t sb;
> - u_int8_t *p = imsg->data;
> - size_t s;
>
> - if ((tb = calloc(1, sizeof(*tb))) == NULL)
> + if (imsg_get_ibuf(imsg, &ibuf) == -1) {
> + log_warn("%s: imsg_get_ibuf", __func__);
> return (-1);
> + }
>
> - IMSG_SIZE_CHECK(imsg, &tb->conf);
> - memcpy(&tb->conf, p, sizeof(tb->conf));
> - s = sizeof(tb->conf);
> + if ((tb = calloc(1, sizeof(*tb))) == NULL)
> + return (-1);
>
> - sb = IMSG_DATA_SIZE(imsg) - s;
> - if (sb > 0) {
> - if ((tb->sendbuf = get_string(p + s, sb)) == NULL) {
> + if (ibuf_get(&ibuf, &tb->conf, sizeof(tb->conf)) == -1) {
> + log_warn("%s: ibuf_get", __func__);
> + free(tb);
> + return (-1);
> + }
> + if ((sb = ibuf_size(&ibuf)) > 0) {
> + if ((tb->sendbuf = get_string(ibuf_data(&ibuf), sb)) == NULL) {
Oh hell, another wart. get_string() should really take an ibuf as
argument. I would try to stay away from ibuf_data() as much as I can.
> + log_warn("%s: get_string", __func__);
> free(tb);
> return (-1);
> }
> @@ -363,6 +370,7 @@ config_gettable(struct relayd *env, struct imsg *imsg)
> if (tb->conf.check == CHECK_BINSEND_EXPECT) {
> tb->sendbinbuf = string2binary(tb->sendbuf);
> if (tb->sendbinbuf == NULL) {
> + free(tb->sendbuf);
> free(tb);
> return (-1);
> }
> @@ -389,8 +397,11 @@ config_gethost(struct relayd *env, struct imsg *imsg)
> if ((host = calloc(1, sizeof(*host))) == NULL)
> return (-1);
>
> - IMSG_SIZE_CHECK(imsg, &host->conf);
> - memcpy(&host->conf, imsg->data, sizeof(host->conf));
> + if (imsg_get_data(imsg, &host->conf, sizeof(host->conf)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + free(host);
> + return (-1);
> + }
>
> if (host_find(env, host->conf.id) != NULL) {
> log_debug("%s: host %d already exists",
> @@ -457,8 +468,11 @@ config_getrdr(struct relayd *env, struct imsg *imsg)
> if ((rdr = calloc(1, sizeof(*rdr))) == NULL)
> return (-1);
>
> - IMSG_SIZE_CHECK(imsg, &rdr->conf);
> - memcpy(&rdr->conf, imsg->data, sizeof(rdr->conf));
> + if (imsg_get_data(imsg, &rdr->conf, sizeof(rdr->conf)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + free(rdr);
> + return (-1);
> + }
>
> if ((rdr->table = table_find(env, rdr->conf.table_id)) == NULL) {
> log_debug("%s: table not found", __func__);
> @@ -488,11 +502,14 @@ config_getvirt(struct relayd *env, struct imsg *imsg)
> struct rdr *rdr;
> struct address *virt;
>
> - IMSG_SIZE_CHECK(imsg, virt);
> -
> if ((virt = calloc(1, sizeof(*virt))) == NULL)
> return (-1);
> - memcpy(virt, imsg->data, sizeof(*virt));
> +
> + if (imsg_get_data(imsg, virt, sizeof(*virt)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + free(virt);
> + return (-1);
> + }
>
> if ((rdr = rdr_find(env, virt->rdrid)) == NULL) {
> log_debug("%s: rdr not found", __func__);
> @@ -544,8 +561,11 @@ config_getrt(struct relayd *env, struct imsg *imsg)
> if ((rt = calloc(1, sizeof(*rt))) == NULL)
> return (-1);
>
> - IMSG_SIZE_CHECK(imsg, &rt->rt_conf);
> - memcpy(&rt->rt_conf, imsg->data, sizeof(rt->rt_conf));
> + if (imsg_get_data(imsg, &rt->rt_conf, sizeof(rt->rt_conf)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + free(rt);
> + return (-1);
> + }
>
> if ((rt->rt_gwtable = table_find(env, rt->rt_conf.gwtable)) == NULL) {
> log_debug("%s: table not found", __func__);
> @@ -574,8 +594,11 @@ config_getroute(struct relayd *env, struct imsg *imsg)
> if ((nr = calloc(1, sizeof(*nr))) == NULL)
> return (-1);
>
> - IMSG_SIZE_CHECK(imsg, &nr->nr_conf);
> - memcpy(&nr->nr_conf, imsg->data, sizeof(nr->nr_conf));
> + if (imsg_get_data(imsg, &nr->nr_conf, sizeof(nr->nr_conf)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + free(nr);
> + return (-1);
> + }
>
> if (route_find(env, nr->nr_conf.id) != NULL) {
> log_debug("%s: route %d already exists",
> @@ -690,22 +713,28 @@ config_setrule(struct relayd *env, struct protocol *proto)
> int
> config_getproto(struct relayd *env, struct imsg *imsg)
> {
> + struct ibuf ibuf;
> struct protocol *proto;
> - size_t styl;
> size_t s;
> - u_int8_t *p = imsg->data;
>
> - if ((proto = calloc(1, sizeof(*proto))) == NULL)
> + if (imsg_get_ibuf(imsg, &ibuf) == -1) {
> + log_warn("%s: imsg_get_ibuf", __func__);
> return (-1);
> + }
>
> - IMSG_SIZE_CHECK(imsg, proto);
> - memcpy(proto, p, sizeof(*proto));
> - s = sizeof(*proto);
> + if ((proto = calloc(1, sizeof(*proto))) == NULL)
> + return (-1);
>
> - styl = IMSG_DATA_SIZE(imsg) - s;
> - proto->style = NULL;
> - if (styl > 0) {
> - if ((proto->style = get_string(p + s, styl - 1)) == NULL) {
> + if (ibuf_get(&ibuf, proto, sizeof(*proto)) == -1) {
> + log_warn("%s: ibuf_get", __func__);
> + free(proto);
> + return (-1);
> + }
> + if ((s = ibuf_size(&ibuf)) > 0) {
> + proto->style = NULL;
> + if ((proto->style = get_string(ibuf_data(&ibuf), s - 1))
> + == NULL) {
> + log_warn("%s: get_string", __func__);
> free(proto);
> return (-1);
> }
> @@ -729,40 +758,48 @@ config_getproto(struct relayd *env, struct imsg *imsg)
> int
> config_getrule(struct relayd *env, struct imsg *imsg)
> {
> + struct ibuf ibuf;
> struct protocol *proto;
> struct relay_rule *rule;
> - size_t s, i;
> - u_int8_t *p = imsg->data;
> + size_t i;
> ssize_t len;
>
> + if (imsg_get_ibuf(imsg, &ibuf) == -1) {
> + log_warn("%s: imsg_get_ibuf", __func__);
> + return (-1);
> + }
> +
> if ((rule = calloc(1, sizeof(*rule))) == NULL)
> return (-1);
>
> - IMSG_SIZE_CHECK(imsg, rule);
> - memcpy(rule, p, sizeof(*rule));
> - s = sizeof(*rule);
> - len = IMSG_DATA_SIZE(imsg) - s;
> + if (ibuf_get(&ibuf, rule, sizeof(*rule)) == -1) {
> + log_warn("%s: ibuf_get", __func__);
> + free(rule);
> + return (-1);
> + }
>
> if ((proto = proto_find(env, rule->rule_protoid)) == NULL) {
> free(rule);
> return (-1);
> }
>
> +
> #define GETKV(_n, _f) { \
> - if (rule->rule_ctl.kvlen[_n]._f >= 0) { \
> + len = rule->rule_ctl.kvlen[_n]._f; \
> + if (len >= 0) { \
> /* Also accept "empty" 0-length strings */ \
> - if ((len < rule->rule_ctl.kvlen[_n]._f) || \
> + if (ibuf_size(&ibuf) < (size_t)len || \
> (rule->rule_kv[_n].kv_##_f = \
> - get_string(p + s, \
> - rule->rule_ctl.kvlen[_n]._f)) == NULL) { \
> + get_string(ibuf_data(&ibuf), len)) == NULL) { \
> + free(rule); \
> + return (-1); \
> + } \
> + if (len > 0 && ibuf_skip(&ibuf, len) == -1) { \
> free(rule); \
> return (-1); \
> } \
> - s += rule->rule_ctl.kvlen[_n]._f; \
> - len -= rule->rule_ctl.kvlen[_n]._f; \
> - \
> DPRINTF("%s: %s %s (len %ld, option %d): %s", __func__, \
> - #_n, #_f, rule->rule_ctl.kvlen[_n]._f, \
> + #_n, #_f, len, \
> rule->rule_kv[_n].kv_option, \
> rule->rule_kv[_n].kv_##_f); \
> } \
> @@ -1027,17 +1064,24 @@ config_setrelay(struct relayd *env, struct relay *rlay)
> int
> config_getrelay(struct relayd *env, struct imsg *imsg)
> {
> + struct ibuf ibuf;
> struct privsep *ps = env->sc_ps;
> struct relay *rlay;
> - u_int8_t *p = imsg->data;
> - size_t s;
> + size_t s;
> +
> + if (imsg_get_ibuf(imsg, &ibuf) == -1) {
> + log_warn("%s: imsg_get_ibuf", __func__);
> + return (-1);
> + }
>
> if ((rlay = calloc(1, sizeof(*rlay))) == NULL)
> return (-1);
>
> - IMSG_SIZE_CHECK(imsg, &rlay->rl_conf);
> - memcpy(&rlay->rl_conf, p, sizeof(rlay->rl_conf));
> - s = sizeof(rlay->rl_conf);
> + if (ibuf_get(&ibuf, &rlay->rl_conf, sizeof(rlay->rl_conf)) == -1) {
> + log_warn("%s: ibuf_get", __func__);
> + free(rlay);
> + return (-1);
> + }
>
> rlay->rl_s = imsg_get_fd(imsg);
> rlay->rl_tls_ca_fd = -1;
> @@ -1054,17 +1098,15 @@ config_getrelay(struct relayd *env, struct imsg *imsg)
> }
> }
>
> - if ((off_t)(IMSG_DATA_SIZE(imsg) - s) <
> - (rlay->rl_conf.tls_cakey_len)) {
> + s = ibuf_size(&ibuf);
> + if (s != (size_t)rlay->rl_conf.tls_cakey_len) {
> log_debug("%s: invalid message length", __func__);
> goto fail;
> }
> -
> - if (rlay->rl_conf.tls_cakey_len) {
> - if ((rlay->rl_tls_cakey = get_data(p + s,
> - rlay->rl_conf.tls_cakey_len)) == NULL)
> + if (s > 0) {
> + if ((rlay->rl_tls_cakey = get_data(ibuf_data(&ibuf), s))
> + == NULL)
Again, I think get_data() needs to either use the ibuf or burn in hell.
> goto fail;
> - s += rlay->rl_conf.tls_cakey_len;
> }
>
> TAILQ_INIT(&rlay->rl_tables);
> @@ -1092,10 +1134,14 @@ config_getrelaytable(struct relayd *env, struct imsg *imsg)
> struct ctl_relaytable crt;
> struct relay *rlay;
> struct table *table;
> - u_int8_t *p = imsg->data;
>
> - IMSG_SIZE_CHECK(imsg, &crt);
> - memcpy(&crt, p, sizeof(crt));
> + if (imsg_get_data(imsg, &crt, sizeof(crt)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + return (-1);
> + }
> +
> + if ((rlt = calloc(1, sizeof(*rlt))) == NULL)
> + goto fail;
>
> if ((rlay = relay_find(env, crt.relayid)) == NULL) {
> log_debug("%s: unknown relay", __func__);
> @@ -1107,9 +1153,6 @@ config_getrelaytable(struct relayd *env, struct imsg *imsg)
> goto fail;
> }
>
> - if ((rlt = calloc(1, sizeof(*rlt))) == NULL)
> - goto fail;
> -
> rlt->rlt_table = table;
> rlt->rlt_mode = crt.mode;
> rlt->rlt_flags = crt.flags;
> @@ -1133,10 +1176,11 @@ config_getrelayfd(struct relayd *env, struct imsg *imsg)
> struct ctl_relayfd crfd;
> struct relay *rlay = NULL;
> struct relay_cert *cert;
> - u_int8_t *p = imsg->data;
>
> - IMSG_SIZE_CHECK(imsg, &crfd);
> - memcpy(&crfd, p, sizeof(crfd));
> + if (imsg_get_data(imsg, &crfd, sizeof(crfd)) == -1) {
> + log_warn("%s: imsg_get_data", __func__);
> + return (-1);
> + }
>
> switch (crfd.type) {
> case RELAY_FD_CERT:
>
Diff looks good otherwise.
--
:wq Claudio
[4/7] relayd: read imsg payloads via the new imsg/ibuf getters
[4/7] relayd: read imsg payloads via the new imsg/ibuf getters