Download raw body.
httpd: convert imsg handling to imsg_get_ and remove IMSG_SIZE_CHECK macro
This is a fist diff on a longer imsg journey in httpd. The goal is to
remove IMSG_SIZE_CHECK().
commit e08b6a50c4d986afa2b3eda5fb7661fcf7ed90f3
Author: Rafael Sadowski <rafael@sizeofvoid.org>
Date: Sun Aug 30 08:50:26 2026 +0200
httpd: convert imsg handling to imsg_get_ and remove IMSG_SIZE_CHECK macro
diff --git a/config.c b/config.c
index ecaa59a..4001d86 100644
--- a/config.c
+++ b/config.c
@@ -130,8 +130,8 @@ config_getreset(struct httpd *env, struct imsg *imsg)
{
unsigned int mode;
- IMSG_SIZE_CHECK(imsg, &mode);
- memcpy(&mode, imsg->data, sizeof(mode));
+ if (imsg_get_data(imsg, &mode, sizeof(mode)) == -1)
+ fatalx("%s: imsg_get_data", __func__);
config_purge(env, mode);
@@ -621,8 +621,11 @@ config_getserver_config(struct httpd *env, struct server *srv,
if ((srv_conf = calloc(1, sizeof(*srv_conf))) == NULL)
return (-1);
- IMSG_SIZE_CHECK(imsg, srv_conf);
- memcpy(srv_conf, p, sizeof(*srv_conf));
+ if (imsg_get_data(imsg, srv_conf, sizeof(*srv_conf)) == -1) {
+ free(srv_conf);
+ fatalx("%s: imsg_get_data", __func__);
+ }
+
s = sizeof(*srv_conf);
/* Reset these variables to avoid free'ing invalid pointers */
@@ -804,8 +807,9 @@ config_getserver(struct httpd *env, struct imsg *imsg)
size_t s;
int fd;
- IMSG_SIZE_CHECK(imsg, &srv_conf);
- memcpy(&srv_conf, p, sizeof(srv_conf));
+ if (imsg_get_data(imsg, &srv_conf, sizeof(srv_conf)) == -1)
+ fatalx("%s: imsg_get_data", __func__);
+
s = sizeof(srv_conf);
/* Reset these variables to avoid free'ing invalid pointers */
@@ -928,22 +932,27 @@ config_gettls(struct httpd *env, struct server_config *srv_conf,
int
config_getserver_tls(struct httpd *env, struct imsg *imsg)
{
+ struct ibuf ibuf;
struct server_config *srv_conf = NULL;
struct tls_config tls_conf;
- uint8_t *p = imsg->data;
+ uint8_t *data;
size_t len;
- IMSG_SIZE_CHECK(imsg, &tls_conf);
- memcpy(&tls_conf, p, sizeof(tls_conf));
+ if (imsg_get_ibuf(imsg, &ibuf) == -1) {
+ log_warn("%s: imsg_get_ibuf", __func__);
+ return (-1);
+ }
- len = tls_conf.tls_chunk_len;
+ if (ibuf_get(&ibuf, &tls_conf, sizeof(tls_conf)) == -1)
+ fatalx("%s: ibuf_get", __func__);
- if ((IMSG_DATA_SIZE(imsg) - sizeof(tls_conf)) < len) {
+ if (ibuf_size(&ibuf) != tls_conf.tls_chunk_len) {
log_debug("%s: invalid message length", __func__);
goto fail;
}
- p += sizeof(tls_conf);
+ data = ibuf_data(&ibuf);
+ len = tls_conf.tls_chunk_len;
if ((srv_conf = serverconfig_byid(tls_conf.id)) == NULL) {
log_debug("%s: server not found", __func__);
@@ -952,32 +961,32 @@ config_getserver_tls(struct httpd *env, struct imsg *imsg)
switch (tls_conf.tls_type) {
case TLS_CFG_CA:
- if (config_gettls(env, srv_conf, &tls_conf, "ca", p, len,
+ if (config_gettls(env, srv_conf, &tls_conf, "ca", data, len,
&srv_conf->tls_ca, &srv_conf->tls_ca_len) != 0)
goto fail;
break;
case TLS_CFG_CERT:
- if (config_gettls(env, srv_conf, &tls_conf, "cert", p, len,
+ if (config_gettls(env, srv_conf, &tls_conf, "cert", data, len,
&srv_conf->tls_cert, &srv_conf->tls_cert_len) != 0)
goto fail;
break;
case TLS_CFG_CRL:
- if (config_gettls(env, srv_conf, &tls_conf, "crl", p, len,
+ if (config_gettls(env, srv_conf, &tls_conf, "crl", data, len,
&srv_conf->tls_crl, &srv_conf->tls_crl_len) != 0)
goto fail;
break;
case TLS_CFG_KEY:
- if (config_gettls(env, srv_conf, &tls_conf, "key", p, len,
+ if (config_gettls(env, srv_conf, &tls_conf, "key", data, len,
&srv_conf->tls_key, &srv_conf->tls_key_len) != 0)
goto fail;
break;
case TLS_CFG_OCSP_STAPLE:
if (config_gettls(env, srv_conf, &tls_conf, "ocsp staple",
- p, len, &srv_conf->tls_ocsp_staple,
+ data, len, &srv_conf->tls_ocsp_staple,
&srv_conf->tls_ocsp_staple_len) != 0)
goto fail;
break;
@@ -1029,10 +1038,9 @@ config_getmedia(struct httpd *env, struct imsg *imsg)
struct privsep *ps = env->sc_ps;
#endif
struct media_type media;
- uint8_t *p = imsg->data;
- IMSG_SIZE_CHECK(imsg, &media);
- memcpy(&media, p, sizeof(media));
+ if (imsg_get_data(imsg, &media, sizeof(media)) == -1)
+ fatalx("%s: imsg_get_data", __func__);
if (media_add(env->sc_mediatypes, &media) == NULL) {
log_debug("%s: failed to add media \"%s\"",
@@ -1085,10 +1093,9 @@ config_getauth(struct httpd *env, struct imsg *imsg)
struct privsep *ps = env->sc_ps;
#endif
struct auth auth;
- uint8_t *p = imsg->data;
- IMSG_SIZE_CHECK(imsg, &auth);
- memcpy(&auth, p, sizeof(auth));
+ if (imsg_get_data(imsg, &auth, sizeof(auth)) == -1)
+ fatalx("%s: imsg_get_data", __func__);
if (auth_add(env->sc_auth, &auth) == NULL) {
log_debug("%s: failed to add auth \"%s[%u]\"",
diff --git a/httpd.c b/httpd.c
index 6d8d031..4b2303b 100644
--- a/httpd.c
+++ b/httpd.c
@@ -430,8 +430,8 @@ parent_dispatch_logger(int fd, struct privsep_proc *p, struct imsg *imsg)
switch (imsg->hdr.type) {
case IMSG_CTL_RESET:
- IMSG_SIZE_CHECK(imsg, &v);
- memcpy(&v, imsg->data, sizeof(v));
+ if (imsg_get_data(imsg, &v, sizeof(v)) == -1)
+ fatalx("%s: imsg_get_data", __func__);
parent_reload(env, v, NULL);
break;
case IMSG_CTL_RELOAD:
diff --git a/httpd.h b/httpd.h
index 71377cf..99631ee 100644
--- a/httpd.h
+++ b/httpd.h
@@ -164,10 +164,6 @@ struct imsgev {
short events;
};
-#define IMSG_SIZE_CHECK(imsg, p) do { \
- if (IMSG_DATA_SIZE(imsg) < sizeof(*p)) \
- fatalx("bad length imsg received"); \
-} while (0)
#define IMSG_DATA_SIZE(imsg) ((imsg)->hdr.len - IMSG_HEADER_SIZE)
#define MAX_IMSG_DATA_SIZE (MAX_IMSGSIZE - IMSG_HEADER_SIZE)
diff --git a/logger.c b/logger.c
index c2eae70..049a5b1 100644
--- a/logger.c
+++ b/logger.c
@@ -140,8 +140,8 @@ logger_open_fd(struct imsg *imsg)
struct log_file *log;
uint32_t id;
- IMSG_SIZE_CHECK(imsg, &id);
- memcpy(&id, imsg->data, sizeof(id));
+ if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
+ fatalx("%s: imsg_get_data", __func__);
TAILQ_FOREACH(log, &log_files, log_entry) {
if (log->log_id == id) {
@@ -158,19 +158,37 @@ logger_open_fd(struct imsg *imsg)
int
logger_open_priv(struct imsg *imsg)
{
+ struct ibuf ibuf;
char path[PATH_MAX];
char name[PATH_MAX], *p;
+ char *filename = NULL;
uint32_t id;
size_t len;
- int fd;
+ int fd, ret;
+
+ if (imsg_get_ibuf(imsg, &ibuf) == -1) {
+ log_warn("%s: imsg_get_ibuf", __func__);
+ return (-1);
+ }
/* called from the privileged process */
- IMSG_SIZE_CHECK(imsg, &id);
- memcpy(&id, imsg->data, sizeof(id));
- p = (char *)imsg->data + sizeof(id);
+ if (ibuf_get(&ibuf, &id, sizeof(id)) == -1)
+ fatalx("%s: ibuf_get", __func__);
- if ((size_t)snprintf(name, sizeof(name), "/%s", p) >= sizeof(name))
+ if ((len = ibuf_size(&ibuf)) == 0) {
+ log_debug("%s: invalid message length", __func__);
return (-1);
+ }
+
+ if ((filename = ibuf_get_string(&ibuf, len)) == NULL) {
+ log_warn("%s: ibuf_get_string", __func__);
+ return (-1);
+ }
+ ret = snprintf(name, sizeof(name), "/%s", filename);
+ free(filename);
+ if (ret < 0 || (size_t)ret >= sizeof(name))
+ return (-1);
+
if ((len = strlcpy(path, httpd_env->sc_logdir, sizeof(path))) >=
sizeof(path))
return (-1);
@@ -243,13 +261,20 @@ logger_start(void)
int
logger_log(struct imsg *imsg)
{
+ struct ibuf ibuf;
char *logline;
uint32_t id;
struct server_config *srv_conf;
struct log_file *log;
- IMSG_SIZE_CHECK(imsg, &id);
- memcpy(&id, imsg->data, sizeof(id));
+ if (imsg_get_ibuf(imsg, &ibuf) == -1) {
+ log_warn("%s: imsg_get_ibuf", __func__);
+ return (-1);
+ }
+
+ /* called from the privileged process */
+ if (ibuf_get(&ibuf, &id, sizeof(id)) == -1)
+ fatalx("%s: ibuf_get", __func__);
if ((srv_conf = serverconfig_byid(id)) == NULL)
fatalx("invalid logging requestr");
@@ -265,7 +290,7 @@ logger_log(struct imsg *imsg)
}
/* XXX get_string() would sanitize the string, but add a malloc */
- logline = (char *)imsg->data + sizeof(id);
+ logline = ibuf_data(&ibuf);
/* For debug output */
log_debug("%s", logline);
diff --git a/server.c b/server.c
index bc96722..f3cbb7c 100644
--- a/server.c
+++ b/server.c
@@ -1408,8 +1408,8 @@ server_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg)
return (-1);
break;
case IMSG_TLSTICKET_REKEY:
- IMSG_SIZE_CHECK(imsg, (&key));
- memcpy(&key, imsg->data, sizeof(key));
+ if (imsg_get_data(imsg, &key, sizeof(key)) == -1)
+ fatalx("%s: imsg_get_data", __func__);
/* apply to the right server */
if ((srv = server_byid(key.tt_id)) == NULL) {
log_debug("%s: invalid sever id", __func__);
httpd: convert imsg handling to imsg_get_ and remove IMSG_SIZE_CHECK macro