commit c8b8dc5c502e3b69cc28f29c0a9570a8378488c6 from: Omar Polo date: Fri Mar 28 16:58:08 2025 UTC update imsg commit - cfe4e15a08dd6e6a8bec9b9fdaa12358e52ee149 commit + c8b8dc5c502e3b69cc28f29c0a9570a8378488c6 blob - 51f90f8b2cf295a14f4590dbc498e143fd0e5a35 blob + 41e4f38a4f1d53afb30ce03de93feefd7d4934fb --- compat/imsg-buffer.c +++ compat/imsg-buffer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: imsg-buffer.c,v 1.18 2023/12/12 15:47:41 claudio Exp $ */ +/* $OpenBSD: imsg-buffer.c,v 1.31 2024/11/26 13:57:31 claudio Exp $ */ /* * Copyright (c) 2023 Claudio Jeker @@ -32,25 +32,37 @@ #include "imsg.h" -static int ibuf_realloc(struct ibuf *, size_t); -static void ibuf_enqueue(struct msgbuf *, struct ibuf *); -static void ibuf_dequeue(struct msgbuf *, struct ibuf *); +struct msgbuf { + TAILQ_HEAD(, ibuf) bufs; + TAILQ_HEAD(, ibuf) rbufs; + uint32_t queued; + char *rbuf; + struct ibuf *rpmsg; + struct ibuf *(*readhdr)(struct ibuf *, void *, int *); + void *rarg; + size_t roff; + size_t hdrsize; +}; + +static void msgbuf_read_enqueue(struct msgbuf *, struct ibuf *); +static void msgbuf_enqueue(struct msgbuf *, struct ibuf *); +static void msgbuf_dequeue(struct msgbuf *, struct ibuf *); static void msgbuf_drain(struct msgbuf *, size_t); +#define IBUF_FD_MARK_ON_STACK -2 + struct ibuf * ibuf_open(size_t len) { struct ibuf *buf; - if (len == 0) { - errno = EINVAL; - return (NULL); - } if ((buf = calloc(1, sizeof(struct ibuf))) == NULL) return (NULL); - if ((buf->buf = calloc(len, 1)) == NULL) { - free(buf); - return (NULL); + if (len > 0) { + if ((buf->buf = calloc(len, 1)) == NULL) { + free(buf); + return (NULL); + } } buf->size = buf->max = len; buf->fd = -1; @@ -83,39 +95,36 @@ ibuf_dynamic(size_t len, size_t max) return (buf); } -static int -ibuf_realloc(struct ibuf *buf, size_t len) -{ - unsigned char *b; - - /* on static buffers max is eq size and so the following fails */ - if (len > SIZE_MAX - buf->wpos || buf->wpos + len > buf->max) { - errno = ERANGE; - return (-1); - } - - b = recallocarray(buf->buf, buf->size, buf->wpos + len, 1); - if (b == NULL) - return (-1); - buf->buf = b; - buf->size = buf->wpos + len; - - return (0); -} - void * ibuf_reserve(struct ibuf *buf, size_t len) { void *b; - if (len > SIZE_MAX - buf->wpos || buf->max == 0) { + if (len > SIZE_MAX - buf->wpos) { errno = ERANGE; return (NULL); } + if (buf->fd == IBUF_FD_MARK_ON_STACK) { + /* can not grow stack buffers */ + errno = EINVAL; + return (NULL); + } - if (buf->wpos + len > buf->size) - if (ibuf_realloc(buf, len) == -1) + if (buf->wpos + len > buf->size) { + unsigned char *nb; + + /* check if buffer is allowed to grow */ + if (buf->wpos + len > buf->max) { + errno = ERANGE; return (NULL); + } + nb = realloc(buf->buf, buf->wpos + len); + if (nb == NULL) + return (NULL); + memset(nb + buf->size, 0, buf->wpos + len - buf->size); + buf->buf = nb; + buf->size = buf->wpos + len; + } b = buf->buf + buf->wpos; buf->wpos += len; @@ -140,14 +149,7 @@ ibuf_add_ibuf(struct ibuf *buf, const struct ibuf *fro return ibuf_add(buf, ibuf_data(from), ibuf_size(from)); } -/* remove after tree is converted */ int -ibuf_add_buf(struct ibuf *buf, const struct ibuf *from) -{ - return ibuf_add_ibuf(buf, from); -} - -int ibuf_add_n8(struct ibuf *buf, uint64_t value) { uint8_t v; @@ -354,7 +356,8 @@ ibuf_size(const struct ibuf *buf) size_t ibuf_left(const struct ibuf *buf) { - if (buf->max == 0) + /* on stack buffers have no space left */ + if (buf->fd == IBUF_FD_MARK_ON_STACK) return (0); return (buf->max - buf->wpos); } @@ -366,8 +369,8 @@ ibuf_truncate(struct ibuf *buf, size_t len) buf->wpos = buf->rpos + len; return (0); } - if (buf->max == 0) { - /* only allow to truncate down */ + if (buf->fd == IBUF_FD_MARK_ON_STACK) { + /* only allow to truncate down for stack buffers */ errno = ERANGE; return (-1); } @@ -383,7 +386,7 @@ ibuf_rewind(struct ibuf *buf) void ibuf_close(struct msgbuf *msgbuf, struct ibuf *buf) { - ibuf_enqueue(msgbuf, buf); + msgbuf_enqueue(msgbuf, buf); } void @@ -392,7 +395,7 @@ ibuf_from_buffer(struct ibuf *buf, void *data, size_t memset(buf, 0, sizeof(*buf)); buf->buf = data; buf->size = buf->wpos = len; - buf->fd = -1; + buf->fd = IBUF_FD_MARK_ON_STACK; } void @@ -425,9 +428,27 @@ ibuf_get_ibuf(struct ibuf *buf, size_t len, struct ibu ibuf_from_buffer(new, ibuf_data(buf), len); buf->rpos += len; return (0); +} + +int +ibuf_get_h16(struct ibuf *buf, uint16_t *value) +{ + return ibuf_get(buf, value, sizeof(*value)); } int +ibuf_get_h32(struct ibuf *buf, uint32_t *value) +{ + return ibuf_get(buf, value, sizeof(*value)); +} + +int +ibuf_get_h64(struct ibuf *buf, uint64_t *value) +{ + return ibuf_get(buf, value, sizeof(*value)); +} + +int ibuf_get_n8(struct ibuf *buf, uint8_t *value) { return ibuf_get(buf, value, sizeof(*value)); @@ -463,22 +484,21 @@ ibuf_get_n64(struct ibuf *buf, uint64_t *value) return (rv); } -int -ibuf_get_h16(struct ibuf *buf, uint16_t *value) +char * +ibuf_get_string(struct ibuf *buf, size_t len) { - return ibuf_get(buf, value, sizeof(*value)); -} + char *str; -int -ibuf_get_h32(struct ibuf *buf, uint32_t *value) -{ - return ibuf_get(buf, value, sizeof(*value)); -} + if (ibuf_size(buf) < len) { + errno = EBADMSG; + return (NULL); + } -int -ibuf_get_h64(struct ibuf *buf, uint64_t *value) -{ - return ibuf_get(buf, value, sizeof(*value)); + str = strndup(ibuf_data(buf), len); + if (str == NULL) + return (NULL); + buf->rpos += len; + return (str); } int @@ -498,9 +518,10 @@ ibuf_free(struct ibuf *buf) { if (buf == NULL) return; - if (buf->max == 0) /* if buf lives on the stack */ - abort(); /* abort before causing more harm */ - if (buf->fd != -1) + /* if buf lives on the stack abort before causing more harm */ + if (buf->fd == IBUF_FD_MARK_ON_STACK) + abort(); + if (buf->fd >= 0) close(buf->fd); freezero(buf->buf, buf->size); free(buf); @@ -509,7 +530,7 @@ ibuf_free(struct ibuf *buf) int ibuf_fd_avail(struct ibuf *buf) { - return (buf->fd != -1); + return (buf->fd >= 0); } int @@ -517,6 +538,9 @@ ibuf_fd_get(struct ibuf *buf) { int fd; + /* negative fds are internal use and equivalent to -1 */ + if (buf->fd < 0) + return (-1); fd = buf->fd; buf->fd = -1; return (fd); @@ -525,73 +549,73 @@ ibuf_fd_get(struct ibuf *buf) void ibuf_fd_set(struct ibuf *buf, int fd) { - if (buf->max == 0) /* if buf lives on the stack */ - abort(); /* abort before causing more harm */ - if (buf->fd != -1) + /* if buf lives on the stack abort before causing more harm */ + if (buf->fd == IBUF_FD_MARK_ON_STACK) + abort(); + if (buf->fd >= 0) close(buf->fd); - buf->fd = fd; + buf->fd = -1; + if (fd >= 0) + buf->fd = fd; } -int -ibuf_write(struct msgbuf *msgbuf) +struct msgbuf * +msgbuf_new(void) { - struct iovec iov[IOV_MAX]; - struct ibuf *buf; - unsigned int i = 0; - ssize_t n; + struct msgbuf *msgbuf; - memset(&iov, 0, sizeof(iov)); - TAILQ_FOREACH(buf, &msgbuf->bufs, entry) { - if (i >= IOV_MAX) - break; - iov[i].iov_base = ibuf_data(buf); - iov[i].iov_len = ibuf_size(buf); - i++; - } + if ((msgbuf = calloc(1, sizeof(*msgbuf))) == NULL) + return (NULL); + msgbuf->queued = 0; + TAILQ_INIT(&msgbuf->bufs); + TAILQ_INIT(&msgbuf->rbufs); -again: - if ((n = writev(msgbuf->fd, iov, i)) == -1) { - if (errno == EINTR) - goto again; - if (errno == ENOBUFS) - errno = EAGAIN; - return (-1); + return msgbuf; +} + +struct msgbuf * +msgbuf_new_reader(size_t hdrsz, + struct ibuf *(*readhdr)(struct ibuf *, void *, int *), void *arg) +{ + struct msgbuf *msgbuf; + char *buf; + + if (hdrsz == 0 || hdrsz > IBUF_READ_SIZE / 2) { + errno = EINVAL; + return (NULL); } - if (n == 0) { /* connection closed */ - errno = 0; - return (0); + if ((buf = malloc(IBUF_READ_SIZE)) == NULL) + return (NULL); + + msgbuf = msgbuf_new(); + if (msgbuf == NULL) { + free(buf); + return (NULL); } - msgbuf_drain(msgbuf, n); + msgbuf->rbuf = buf; + msgbuf->hdrsize = hdrsz; + msgbuf->readhdr = readhdr; + msgbuf->rarg = arg; - return (1); + return (msgbuf); } void -msgbuf_init(struct msgbuf *msgbuf) +msgbuf_free(struct msgbuf *msgbuf) { - msgbuf->queued = 0; - msgbuf->fd = -1; - TAILQ_INIT(&msgbuf->bufs); + if (msgbuf == NULL) + return; + msgbuf_clear(msgbuf); + free(msgbuf->rbuf); + free(msgbuf); } -static void -msgbuf_drain(struct msgbuf *msgbuf, size_t n) +uint32_t +msgbuf_queuelen(struct msgbuf *msgbuf) { - struct ibuf *buf, *next; - - for (buf = TAILQ_FIRST(&msgbuf->bufs); buf != NULL && n > 0; - buf = next) { - next = TAILQ_NEXT(buf, entry); - if (n >= ibuf_size(buf)) { - n -= ibuf_size(buf); - ibuf_dequeue(msgbuf, buf); - } else { - buf->rpos += n; - n = 0; - } - } + return (msgbuf->queued); } void @@ -599,14 +623,68 @@ msgbuf_clear(struct msgbuf *msgbuf) { struct ibuf *buf; + /* write side */ while ((buf = TAILQ_FIRST(&msgbuf->bufs)) != NULL) - ibuf_dequeue(msgbuf, buf); + msgbuf_dequeue(msgbuf, buf); + msgbuf->queued = 0; + + /* read side */ + while ((buf = TAILQ_FIRST(&msgbuf->rbufs)) != NULL) { + TAILQ_REMOVE(&msgbuf->rbufs, buf, entry); + ibuf_free(buf); + } + msgbuf->roff = 0; + ibuf_free(msgbuf->rpmsg); + msgbuf->rpmsg = NULL; +} + +struct ibuf * +msgbuf_get(struct msgbuf *msgbuf) +{ + struct ibuf *buf; + + if ((buf = TAILQ_FIRST(&msgbuf->rbufs)) != NULL) + TAILQ_REMOVE(&msgbuf->rbufs, buf, entry); + return buf; } int -msgbuf_write(struct msgbuf *msgbuf) +ibuf_write(int fd, struct msgbuf *msgbuf) { struct iovec iov[IOV_MAX]; + struct ibuf *buf; + unsigned int i = 0; + ssize_t n; + + memset(&iov, 0, sizeof(iov)); + TAILQ_FOREACH(buf, &msgbuf->bufs, entry) { + if (i >= IOV_MAX) + break; + iov[i].iov_base = ibuf_data(buf); + iov[i].iov_len = ibuf_size(buf); + i++; + } + if (i == 0) + return (0); /* nothing queued */ + + again: + if ((n = writev(fd, iov, i)) == -1) { + if (errno == EINTR) + goto again; + if (errno == EAGAIN || errno == ENOBUFS) + /* lets retry later again */ + return (0); + return (-1); + } + + msgbuf_drain(msgbuf, n); + return (0); +} + +int +msgbuf_write(int fd, struct msgbuf *msgbuf) +{ + struct iovec iov[IOV_MAX]; struct ibuf *buf, *buf0 = NULL; unsigned int i = 0; ssize_t n; @@ -632,6 +710,9 @@ msgbuf_write(struct msgbuf *msgbuf) buf0 = buf; } + if (i == 0) + return (0); /* nothing queued */ + msg.msg_iov = iov; msg.msg_iovlen = i; @@ -645,20 +726,16 @@ msgbuf_write(struct msgbuf *msgbuf) *(int *)CMSG_DATA(cmsg) = buf0->fd; } -again: - if ((n = sendmsg(msgbuf->fd, &msg, 0)) == -1) { + again: + if ((n = sendmsg(fd, &msg, 0)) == -1) { if (errno == EINTR) goto again; - if (errno == ENOBUFS) - errno = EAGAIN; + if (errno == EAGAIN || errno == ENOBUFS) + /* lets retry later again */ + return (0); return (-1); } - if (n == 0) { /* connection closed */ - errno = 0; - return (0); - } - /* * assumption: fd got sent if sendmsg sent anything * this works because fds are passed one at a time @@ -670,28 +747,209 @@ again: msgbuf_drain(msgbuf, n); + return (0); +} + +static int +ibuf_read_process(struct msgbuf *msgbuf, int fd) +{ + struct ibuf rbuf, msg; + ssize_t sz; + + ibuf_from_buffer(&rbuf, msgbuf->rbuf, msgbuf->roff); + + do { + if (msgbuf->rpmsg == NULL) { + if (ibuf_size(&rbuf) < msgbuf->hdrsize) + break; + /* get size from header */ + ibuf_from_buffer(&msg, ibuf_data(&rbuf), + msgbuf->hdrsize); + if ((msgbuf->rpmsg = msgbuf->readhdr(&msg, + msgbuf->rarg, &fd)) == NULL) + goto fail; + } + + if (ibuf_left(msgbuf->rpmsg) <= ibuf_size(&rbuf)) + sz = ibuf_left(msgbuf->rpmsg); + else + sz = ibuf_size(&rbuf); + + /* neither call below can fail */ + if (ibuf_get_ibuf(&rbuf, sz, &msg) == -1 || + ibuf_add_ibuf(msgbuf->rpmsg, &msg) == -1) + goto fail; + + if (ibuf_left(msgbuf->rpmsg) == 0) { + msgbuf_read_enqueue(msgbuf, msgbuf->rpmsg); + msgbuf->rpmsg = NULL; + } + } while (ibuf_size(&rbuf) > 0); + + if (ibuf_size(&rbuf) > 0) + memmove(msgbuf->rbuf, ibuf_data(&rbuf), ibuf_size(&rbuf)); + msgbuf->roff = ibuf_size(&rbuf); + + if (fd != -1) + close(fd); return (1); + + fail: + /* XXX how to properly clean up is unclear */ + if (fd != -1) + close(fd); + return (-1); } -uint32_t -msgbuf_queuelen(struct msgbuf *msgbuf) +int +ibuf_read(int fd, struct msgbuf *msgbuf) { - return (msgbuf->queued); + struct iovec iov; + ssize_t n; + + if (msgbuf->rbuf == NULL) { + errno = EINVAL; + return (-1); + } + + iov.iov_base = msgbuf->rbuf + msgbuf->roff; + iov.iov_len = IBUF_READ_SIZE - msgbuf->roff; + + again: + if ((n = readv(fd, &iov, 1)) == -1) { + if (errno == EINTR) + goto again; + if (errno == EAGAIN) + /* lets retry later again */ + return (1); + return (-1); + } + if (n == 0) /* connection closed */ + return (0); + + msgbuf->roff += n; + /* new data arrived, try to process it */ + return (ibuf_read_process(msgbuf, -1)); } +int +msgbuf_read(int fd, struct msgbuf *msgbuf) +{ + struct msghdr msg; + struct cmsghdr *cmsg; + union { + struct cmsghdr hdr; + char buf[CMSG_SPACE(sizeof(int) * 1)]; + } cmsgbuf; + struct iovec iov; + ssize_t n; + int fdpass = -1; + + if (msgbuf->rbuf == NULL) { + errno = EINVAL; + return (-1); + } + + memset(&msg, 0, sizeof(msg)); + memset(&cmsgbuf, 0, sizeof(cmsgbuf)); + + iov.iov_base = msgbuf->rbuf + msgbuf->roff; + iov.iov_len = IBUF_READ_SIZE - msgbuf->roff; + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = &cmsgbuf.buf; + msg.msg_controllen = sizeof(cmsgbuf.buf); + +again: + if ((n = recvmsg(fd, &msg, 0)) == -1) { + if (errno == EINTR) + goto again; + if (errno == EMSGSIZE) + /* + * Not enough fd slots: fd passing failed, retry + * to receive the message without fd. + * imsg_get_fd() will return -1 in that case. + */ + goto again; + if (errno == EAGAIN) + /* lets retry later again */ + return (1); + return (-1); + } + if (n == 0) /* connection closed */ + return (0); + + msgbuf->roff += n; + + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; + cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (cmsg->cmsg_level == SOL_SOCKET && + cmsg->cmsg_type == SCM_RIGHTS) { + int i, j, f; + + /* + * We only accept one file descriptor. Due to C + * padding rules, our control buffer might contain + * more than one fd, and we must close them. + */ + j = ((char *)cmsg + cmsg->cmsg_len - + (char *)CMSG_DATA(cmsg)) / sizeof(int); + for (i = 0; i < j; i++) { + f = ((int *)CMSG_DATA(cmsg))[i]; + if (i == 0) + fdpass = f; + else + close(f); + } + } + /* we do not handle other ctl data level */ + } + + /* new data arrived, try to process it */ + return (ibuf_read_process(msgbuf, fdpass)); +} + static void -ibuf_enqueue(struct msgbuf *msgbuf, struct ibuf *buf) +msgbuf_read_enqueue(struct msgbuf *msgbuf, struct ibuf *buf) { - if (buf->max == 0) /* if buf lives on the stack */ - abort(); /* abort before causing more harm */ + /* if buf lives on the stack abort before causing more harm */ + if (buf->fd == IBUF_FD_MARK_ON_STACK) + abort(); + TAILQ_INSERT_TAIL(&msgbuf->rbufs, buf, entry); +} + +static void +msgbuf_enqueue(struct msgbuf *msgbuf, struct ibuf *buf) +{ + /* if buf lives on the stack abort before causing more harm */ + if (buf->fd == IBUF_FD_MARK_ON_STACK) + abort(); TAILQ_INSERT_TAIL(&msgbuf->bufs, buf, entry); msgbuf->queued++; } static void -ibuf_dequeue(struct msgbuf *msgbuf, struct ibuf *buf) +msgbuf_dequeue(struct msgbuf *msgbuf, struct ibuf *buf) { TAILQ_REMOVE(&msgbuf->bufs, buf, entry); msgbuf->queued--; ibuf_free(buf); } + +static void +msgbuf_drain(struct msgbuf *msgbuf, size_t n) +{ + struct ibuf *buf, *next; + + for (buf = TAILQ_FIRST(&msgbuf->bufs); buf != NULL && n > 0; + buf = next) { + next = TAILQ_NEXT(buf, entry); + if (n >= ibuf_size(buf)) { + n -= ibuf_size(buf); + msgbuf_dequeue(msgbuf, buf); + } else { + buf->rpos += n; + n = 0; + } + } +} blob - c3eef13191d6fbcd0c647e47c1950afc53aa1619 blob + 43b5bd81cfb1257af33854e7a4e4a62c1a4d7cbf --- compat/imsg.c +++ compat/imsg.c @@ -1,4 +1,4 @@ -/* $OpenBSD: imsg.c,v 1.23 2023/12/12 15:47:41 claudio Exp $ */ +/* $OpenBSD: imsg.c,v 1.38 2024/11/29 04:35:13 tb Exp $ */ /* * Copyright (c) 2023 Claudio Jeker @@ -24,164 +24,117 @@ #include #include +#include #include #include #include #include "imsg.h" -struct imsg_fd { - TAILQ_ENTRY(imsg_fd) entry; - int fd; -}; +#define IMSG_ALLOW_FDPASS 0x01 +#define IMSG_FD_MARK 0x80000000U -int imsg_fd_overhead = 0; +static struct ibuf *imsg_parse_hdr(struct ibuf *, void *, int *); -static int imsg_dequeue_fd(struct imsgbuf *); - -void -imsg_init(struct imsgbuf *imsgbuf, int fd) +int +imsgbuf_init(struct imsgbuf *imsgbuf, int fd) { - msgbuf_init(&imsgbuf->w); - memset(&imsgbuf->r, 0, sizeof(imsgbuf->r)); - imsgbuf->fd = fd; - imsgbuf->w.fd = fd; + imsgbuf->w = msgbuf_new_reader(IMSG_HEADER_SIZE, imsg_parse_hdr, + imsgbuf); + if (imsgbuf->w == NULL) + return (-1); imsgbuf->pid = getpid(); - TAILQ_INIT(&imsgbuf->fds); + imsgbuf->maxsize = MAX_IMSGSIZE; + imsgbuf->fd = fd; + imsgbuf->flags = 0; + return (0); } -ssize_t -imsg_read(struct imsgbuf *imsgbuf) +void +imsgbuf_allow_fdpass(struct imsgbuf *imsgbuf) { - struct msghdr msg; - struct cmsghdr *cmsg; - union { - struct cmsghdr hdr; - char buf[CMSG_SPACE(sizeof(int) * 1)]; - } cmsgbuf; - struct iovec iov; - ssize_t n = -1; - int fd; - struct imsg_fd *ifd; + imsgbuf->flags |= IMSG_ALLOW_FDPASS; +} - memset(&msg, 0, sizeof(msg)); - memset(&cmsgbuf, 0, sizeof(cmsgbuf)); - - iov.iov_base = imsgbuf->r.buf + imsgbuf->r.wpos; - iov.iov_len = sizeof(imsgbuf->r.buf) - imsgbuf->r.wpos; - msg.msg_iov = &iov; - msg.msg_iovlen = 1; - msg.msg_control = &cmsgbuf.buf; - msg.msg_controllen = sizeof(cmsgbuf.buf); - - if ((ifd = calloc(1, sizeof(struct imsg_fd))) == NULL) +int +imsgbuf_set_maxsize(struct imsgbuf *imsgbuf, uint32_t maxsize) +{ + if (maxsize < IMSG_HEADER_SIZE || maxsize & IMSG_FD_MARK) { + errno = EINVAL; return (-1); - -again: - if (getdtablecount() + imsg_fd_overhead + - (int)((CMSG_SPACE(sizeof(int))-CMSG_SPACE(0))/sizeof(int)) - >= getdtablesize()) { - errno = EAGAIN; - free(ifd); - return (-1); } + imsgbuf->maxsize = maxsize; + return (0); +} - if ((n = recvmsg(imsgbuf->fd, &msg, 0)) == -1) { - if (errno == EINTR) - goto again; - goto fail; - } +int +imsgbuf_read(struct imsgbuf *imsgbuf) +{ + if (imsgbuf->flags & IMSG_ALLOW_FDPASS) + return msgbuf_read(imsgbuf->fd, imsgbuf->w); + else + return ibuf_read(imsgbuf->fd, imsgbuf->w); +} - imsgbuf->r.wpos += n; +int +imsgbuf_write(struct imsgbuf *imsgbuf) +{ + if (imsgbuf->flags & IMSG_ALLOW_FDPASS) + return msgbuf_write(imsgbuf->fd, imsgbuf->w); + else + return ibuf_write(imsgbuf->fd, imsgbuf->w); +} - for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; - cmsg = CMSG_NXTHDR(&msg, cmsg)) { - if (cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SCM_RIGHTS) { - int i; - int j; - - /* - * We only accept one file descriptor. Due to C - * padding rules, our control buffer might contain - * more than one fd, and we must close them. - */ - j = ((char *)cmsg + cmsg->cmsg_len - - (char *)CMSG_DATA(cmsg)) / sizeof(int); - for (i = 0; i < j; i++) { - fd = ((int *)CMSG_DATA(cmsg))[i]; - if (ifd != NULL) { - ifd->fd = fd; - TAILQ_INSERT_TAIL(&imsgbuf->fds, ifd, - entry); - ifd = NULL; - } else - close(fd); - } - } - /* we do not handle other ctl data level */ +int +imsgbuf_flush(struct imsgbuf *imsgbuf) +{ + while (imsgbuf_queuelen(imsgbuf) > 0) { + if (imsgbuf_write(imsgbuf) == -1) + return (-1); } + return (0); +} -fail: - free(ifd); - return (n); +void +imsgbuf_clear(struct imsgbuf *imsgbuf) +{ + msgbuf_free(imsgbuf->w); + imsgbuf->w = NULL; } +uint32_t +imsgbuf_queuelen(struct imsgbuf *imsgbuf) +{ + return msgbuf_queuelen(imsgbuf->w); +} + ssize_t imsg_get(struct imsgbuf *imsgbuf, struct imsg *imsg) { struct imsg m; - size_t av, left, datalen; + struct ibuf *buf; - av = imsgbuf->r.wpos; - - if (IMSG_HEADER_SIZE > av) + if ((buf = msgbuf_get(imsgbuf->w)) == NULL) return (0); - memcpy(&m.hdr, imsgbuf->r.buf, sizeof(m.hdr)); - if (m.hdr.len < IMSG_HEADER_SIZE || - m.hdr.len > MAX_IMSGSIZE) { - errno = ERANGE; + if (ibuf_get(buf, &m.hdr, sizeof(m.hdr)) == -1) return (-1); - } - if (m.hdr.len > av) - return (0); - m.fd = -1; - m.buf = NULL; - m.data = NULL; + if (ibuf_size(buf)) + m.data = ibuf_data(buf); + else + m.data = NULL; + m.buf = buf; + m.hdr.len &= ~IMSG_FD_MARK; - datalen = m.hdr.len - IMSG_HEADER_SIZE; - imsgbuf->r.rptr = imsgbuf->r.buf + IMSG_HEADER_SIZE; - if (datalen != 0) { - if ((m.buf = ibuf_open(datalen)) == NULL) - return (-1); - if (ibuf_add(m.buf, imsgbuf->r.rptr, datalen) == -1) { - /* this should never fail */ - ibuf_free(m.buf); - return (-1); - } - m.data = ibuf_data(m.buf); - } - - if (m.hdr.flags & IMSGF_HASFD) - m.fd = imsg_dequeue_fd(imsgbuf); - - if (m.hdr.len < av) { - left = av - m.hdr.len; - memmove(&imsgbuf->r.buf, imsgbuf->r.buf + m.hdr.len, left); - imsgbuf->r.wpos = left; - } else - imsgbuf->r.wpos = 0; - *imsg = m; - return (datalen + IMSG_HEADER_SIZE); + return (ibuf_size(buf) + IMSG_HEADER_SIZE); } int imsg_get_ibuf(struct imsg *imsg, struct ibuf *ibuf) { - if (imsg->buf == NULL) { + if (ibuf_size(imsg->buf) == 0) { errno = EBADMSG; return (-1); } @@ -195,7 +148,7 @@ imsg_get_data(struct imsg *imsg, void *data, size_t le errno = EINVAL; return (-1); } - if (imsg->buf == NULL || ibuf_size(imsg->buf) != len) { + if (ibuf_size(imsg->buf) != len) { errno = EBADMSG; return (-1); } @@ -205,10 +158,7 @@ imsg_get_data(struct imsg *imsg, void *data, size_t le int imsg_get_fd(struct imsg *imsg) { - int fd = imsg->fd; - - imsg->fd = -1; - return fd; + return ibuf_fd_get(imsg->buf); } uint32_t @@ -220,8 +170,6 @@ imsg_get_id(struct imsg *imsg) size_t imsg_get_len(struct imsg *imsg) { - if (imsg->buf == NULL) - return 0; return ibuf_size(imsg->buf); } @@ -291,14 +239,13 @@ imsg_compose_ibuf(struct imsgbuf *imsgbuf, uint32_t ty struct imsg_hdr hdr; int save_errno; - if (ibuf_size(buf) + IMSG_HEADER_SIZE > MAX_IMSGSIZE) { + if (ibuf_size(buf) + IMSG_HEADER_SIZE > imsgbuf->maxsize) { errno = ERANGE; goto fail; } hdr.type = type; hdr.len = ibuf_size(buf) + IMSG_HEADER_SIZE; - hdr.flags = 0; hdr.peerid = id; if ((hdr.pid = pid) == 0) hdr.pid = imsgbuf->pid; @@ -308,8 +255,8 @@ imsg_compose_ibuf(struct imsgbuf *imsgbuf, uint32_t ty if (imsg_add(hdrbuf, &hdr, sizeof(hdr)) == -1) goto fail; - ibuf_close(&imsgbuf->w, hdrbuf); - ibuf_close(&imsgbuf->w, buf); + ibuf_close(imsgbuf->w, hdrbuf); + ibuf_close(imsgbuf->w, buf); return (1); fail: @@ -327,24 +274,18 @@ int imsg_forward(struct imsgbuf *imsgbuf, struct imsg *msg) { struct ibuf *wbuf; - size_t len = 0; + size_t len; - if (msg->fd != -1) { - close(msg->fd); - msg->fd = -1; - } + ibuf_rewind(msg->buf); + ibuf_skip(msg->buf, sizeof(msg->hdr)); + len = ibuf_size(msg->buf); - if (msg->buf != NULL) { - ibuf_rewind(msg->buf); - len = ibuf_size(msg->buf); - } - if ((wbuf = imsg_create(imsgbuf, msg->hdr.type, msg->hdr.peerid, msg->hdr.pid, len)) == NULL) return (-1); - if (msg->buf != NULL) { - if (ibuf_add_buf(wbuf, msg->buf) == -1) { + if (len != 0) { + if (ibuf_add_ibuf(wbuf, msg->buf) == -1) { ibuf_free(wbuf); return (-1); } @@ -362,17 +303,16 @@ imsg_create(struct imsgbuf *imsgbuf, uint32_t type, ui struct imsg_hdr hdr; datalen += IMSG_HEADER_SIZE; - if (datalen > MAX_IMSGSIZE) { + if (datalen > imsgbuf->maxsize) { errno = ERANGE; return (NULL); } hdr.type = type; - hdr.flags = 0; hdr.peerid = id; if ((hdr.pid = pid) == 0) hdr.pid = imsgbuf->pid; - if ((wbuf = ibuf_dynamic(datalen, MAX_IMSGSIZE)) == NULL) { + if ((wbuf = ibuf_dynamic(datalen, imsgbuf->maxsize)) == NULL) { return (NULL); } if (imsg_add(wbuf, &hdr, sizeof(hdr)) == -1) @@ -395,16 +335,13 @@ imsg_add(struct ibuf *msg, const void *data, size_t da void imsg_close(struct imsgbuf *imsgbuf, struct ibuf *msg) { - struct imsg_hdr *hdr; + uint32_t len; - hdr = (struct imsg_hdr *)msg->buf; - - hdr->flags &= ~IMSGF_HASFD; + len = ibuf_size(msg); if (ibuf_fd_avail(msg)) - hdr->flags |= IMSGF_HASFD; - hdr->len = ibuf_size(msg); - - ibuf_close(&imsgbuf->w, msg); + len |= IMSG_FD_MARK; + (void)ibuf_set_h32(msg, offsetof(struct imsg_hdr, len), len); + ibuf_close(imsgbuf->w, msg); } void @@ -413,37 +350,29 @@ imsg_free(struct imsg *imsg) ibuf_free(imsg->buf); } -static int -imsg_dequeue_fd(struct imsgbuf *imsgbuf) +static struct ibuf * +imsg_parse_hdr(struct ibuf *buf, void *arg, int *fd) { - int fd; - struct imsg_fd *ifd; + struct imsgbuf *imsgbuf = arg; + struct imsg_hdr hdr; + struct ibuf *b; + uint32_t len; - if ((ifd = TAILQ_FIRST(&imsgbuf->fds)) == NULL) - return (-1); + if (ibuf_get(buf, &hdr, sizeof(hdr)) == -1) + return (NULL); - fd = ifd->fd; - TAILQ_REMOVE(&imsgbuf->fds, ifd, entry); - free(ifd); + len = hdr.len & ~IMSG_FD_MARK; - return (fd); -} + if (len < IMSG_HEADER_SIZE || len > imsgbuf->maxsize) { + errno = ERANGE; + return (NULL); + } + if ((b = ibuf_open(len)) == NULL) + return (NULL); + if (hdr.len & IMSG_FD_MARK) { + ibuf_fd_set(b, *fd); + *fd = -1; + } -int -imsg_flush(struct imsgbuf *imsgbuf) -{ - while (imsgbuf->w.queued) - if (msgbuf_write(&imsgbuf->w) <= 0) - return (-1); - return (0); + return b; } - -void -imsg_clear(struct imsgbuf *imsgbuf) -{ - int fd; - - msgbuf_clear(&imsgbuf->w); - while ((fd = imsg_dequeue_fd(imsgbuf)) != -1) - close(fd); -} blob - dd47b1889da577ce63769035f0e4bd0b1d8a7c32 blob + f70ff098cf3f4d650bf9896ceaca431a88f28bea --- compat/imsg.h +++ compat/imsg.h @@ -1,4 +1,4 @@ -/* $OpenBSD: imsg.h,v 1.8 2023/12/12 15:47:41 claudio Exp $ */ +/* $OpenBSD: imsg.h,v 1.19 2024/11/26 13:57:31 claudio Exp $ */ /* * Copyright (c) 2023 Claudio Jeker @@ -23,6 +23,7 @@ #define _IMSG_H_ #include +#include #define IBUF_READ_SIZE 65535 #define IMSG_HEADER_SIZE sizeof(struct imsg_hdr) @@ -38,40 +39,25 @@ struct ibuf { int fd; }; -struct msgbuf { - TAILQ_HEAD(, ibuf) bufs; - uint32_t queued; - int fd; -}; +struct msgbuf; -struct ibuf_read { - unsigned char buf[IBUF_READ_SIZE]; - unsigned char *rptr; - size_t wpos; -}; - -struct imsg_fd; struct imsgbuf { - TAILQ_HEAD(, imsg_fd) fds; - struct ibuf_read r; - struct msgbuf w; - int fd; + struct msgbuf *w; pid_t pid; + uint32_t maxsize; + int fd; + int flags; }; -#define IMSGF_HASFD 1 - struct imsg_hdr { uint32_t type; - uint16_t len; - uint16_t flags; + uint32_t len; uint32_t peerid; uint32_t pid; }; struct imsg { struct imsg_hdr hdr; - int fd; void *data; struct ibuf *buf; }; @@ -82,7 +68,6 @@ struct iovec; struct ibuf *ibuf_open(size_t); struct ibuf *ibuf_dynamic(size_t, size_t); int ibuf_add(struct ibuf *, const void *, size_t); -int ibuf_add_buf(struct ibuf *, const struct ibuf *); int ibuf_add_ibuf(struct ibuf *, const struct ibuf *); int ibuf_add_zero(struct ibuf *, size_t); int ibuf_add_n8(struct ibuf *, uint64_t); @@ -119,20 +104,33 @@ int ibuf_get_n64(struct ibuf *, uint64_t *); int ibuf_get_h16(struct ibuf *, uint16_t *); int ibuf_get_h32(struct ibuf *, uint32_t *); int ibuf_get_h64(struct ibuf *, uint64_t *); +char *ibuf_get_string(struct ibuf *, size_t); int ibuf_skip(struct ibuf *, size_t); void ibuf_free(struct ibuf *); int ibuf_fd_avail(struct ibuf *); int ibuf_fd_get(struct ibuf *); void ibuf_fd_set(struct ibuf *, int); -int ibuf_write(struct msgbuf *); -void msgbuf_init(struct msgbuf *); +struct msgbuf *msgbuf_new(void); +struct msgbuf *msgbuf_new_reader(size_t, + struct ibuf *(*)(struct ibuf *, void *, int *), void *); +void msgbuf_free(struct msgbuf *); void msgbuf_clear(struct msgbuf *); uint32_t msgbuf_queuelen(struct msgbuf *); -int msgbuf_write(struct msgbuf *); +int ibuf_write(int, struct msgbuf *); +int msgbuf_write(int, struct msgbuf *); +int ibuf_read(int, struct msgbuf *); +int msgbuf_read(int, struct msgbuf *); +struct ibuf *msgbuf_get(struct msgbuf *); /* imsg.c */ -void imsg_init(struct imsgbuf *, int); -ssize_t imsg_read(struct imsgbuf *); +int imsgbuf_init(struct imsgbuf *, int); +void imsgbuf_allow_fdpass(struct imsgbuf *imsgbuf); +int imsgbuf_set_maxsize(struct imsgbuf *, uint32_t); +int imsgbuf_read(struct imsgbuf *); +int imsgbuf_write(struct imsgbuf *); +int imsgbuf_flush(struct imsgbuf *); +void imsgbuf_clear(struct imsgbuf *); +uint32_t imsgbuf_queuelen(struct imsgbuf *); ssize_t imsg_get(struct imsgbuf *, struct imsg *); int imsg_get_ibuf(struct imsg *, struct ibuf *); int imsg_get_data(struct imsg *, void *, size_t); @@ -152,7 +150,5 @@ struct ibuf *imsg_create(struct imsgbuf *, uint32_t, u int imsg_add(struct ibuf *, const void *, size_t); void imsg_close(struct imsgbuf *, struct ibuf *); void imsg_free(struct imsg *); -int imsg_flush(struct imsgbuf *); -void imsg_clear(struct imsgbuf *); #endif blob - e1fb2593da4a23e14b1488dd774cc0d7cb259b44 blob + 2da1175b68c31e2b9f572288f297455ed0b351c8 --- configure.ac +++ configure.ac @@ -202,7 +202,7 @@ AS_IF([test "x$with_libimsg" = "xyes"], [ ]) ]) -AC_SEARCH_LIBS([imsg_init], [util], [ +AC_SEARCH_LIBS([imsgbuf_init], [util], [ AC_DEFINE([HAVE_IMSG], 1, [Define to 1 if you have imsg functions (-lutil or -limsg).]) ], [ blob - 65a705971ea2f9a254dbf0dcbf208401bd9e2d2d blob + d1d426e94792797943f3b3dc0400351c91133ce0 --- control.c +++ control.c @@ -155,7 +155,13 @@ control_accept(int listenfd, int event, void *bula) c = xcalloc(1, sizeof(struct ctl_conn)); - imsg_init(&c->iev.ibuf, connfd); + if (imsgbuf_init(&c->iev.ibuf, connfd) == -1) { + message("%s: ev_add: %s", __func__, strerror(errno)); + close(connfd); + free(c); + return; + } + c->iev.handler = control_dispatch_imsg; c->iev.events = EV_READ; if (ev_add(connfd, c->iev.events, c->iev.handler, &c->iev) == -1) { @@ -204,20 +210,20 @@ control_close(int fd) return; } - msgbuf_clear(&c->iev.ibuf.w); - TAILQ_REMOVE(&ctl_conns, c, entry); - ev_del(c->iev.ibuf.fd); close(c->iev.ibuf.fd); + imsgbuf_clear(&c->iev.ibuf); + TAILQ_REMOVE(&ctl_conns, c, entry); + + free(c); + /* Some file descriptors are available again. */ if (ev_timer_pending(control_state.timeout)) { ev_timer_cancel(control_state.timeout); control_state.timeout = 0; ev_add(control_state.fd, EV_READ, control_accept, NULL); } - - free(c); } void @@ -233,14 +239,13 @@ control_dispatch_imsg(int fd, int event, void *bula) } if (event & EV_READ) { - if (((n = imsg_read(&c->iev.ibuf)) == -1 && errno != EAGAIN) || - n == 0) { + if (imsgbuf_read(&c->iev.ibuf) == -1) { control_close(fd); return; } } if (event & EV_WRITE) { - if (msgbuf_write(&c->iev.ibuf.w) <= 0 && errno != EAGAIN) { + if (imsgbuf_write(&c->iev.ibuf) == -1) { control_close(fd); return; } blob - 1680c011fd08a77fc6d0009ed020c48efded8de1 blob + 1a27057eb0bff3416cb1b75a3360435db1e4a48c --- imsgev.c +++ imsgev.c @@ -23,7 +23,7 @@ void imsg_event_add(struct imsgev *iev) { iev->events = EV_READ; - if (iev->ibuf.w.queued) + if (imsgbuf_queuelen(&iev->ibuf)) iev->events |= EV_WRITE; ev_add(iev->ibuf.fd, iev->events, iev->handler, iev); blob - b4095c8a50f78adc731e74dcccb649ed864cd84b blob + 64c2e4414394dca4cfc2e4d7e1c6f3b709ba1ad4 --- net.c +++ net.c @@ -580,16 +580,16 @@ handle_dispatch_imsg(int fd, int event, void *d) int certok; if (event & EV_READ) { - if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN) - err(1, "imsg_read"); - if (n == 0) - err(1, "connection closed"); + if (imsgbuf_read(ibuf) == -1) { + if (errno == EPIPE) + errx(1, "connection closed"); + else + err(1, "imsg_read failure"); + } } if (event & EV_WRITE) { - if ((n = msgbuf_write(&ibuf->w)) == -1 && errno != EAGAIN) + if (imsgbuf_write(ibuf) == -1 && errno != EAGAIN) err(1, "msgbuf_write"); - if (n == 0) - err(1, "connection closed"); } for (;;) { @@ -704,7 +704,8 @@ net_main(void) /* Setup pipe and event handler to the main process */ iev_ui = xmalloc(sizeof(*iev_ui)); - imsg_init(&iev_ui->ibuf, 3); + if (imsgbuf_init(&iev_ui->ibuf, 3) == -1) + err(1, "imsgbuf_init failed"); iev_ui->handler = handle_dispatch_imsg; iev_ui->events = EV_READ; ev_add(iev_ui->ibuf.fd, iev_ui->events, iev_ui->handler, iev_ui); @@ -713,7 +714,7 @@ net_main(void) ev_loop(); - msgbuf_clear(&iev_ui->ibuf.w); + imsgbuf_clear(&iev_ui->ibuf); close(iev_ui->ibuf.fd); free(iev_ui); blob - d1be335dfcb0919299d8afa13e11cc06d75f77b9 blob + 3bb3d2b7cef512ba92a8ebc66c68f715f9e0d42b --- telescope.c +++ telescope.c @@ -457,16 +457,12 @@ handle_dispatch_imsg(int fd, int event, void *data) int code; if (event & EV_READ) { - if ((n = imsg_read(imsgbuf)) == -1 && errno != EAGAIN) + if (imsgbuf_read(imsgbuf) == -1) err(1, "imsg_read"); - if (n == 0) - err(1, "connection closed"); } if (event & EV_WRITE) { - if ((n = msgbuf_write(&imsgbuf->w)) == -1 && errno != EAGAIN) + if (imsgbuf_write(imsgbuf) == -1) err(1, "msgbuf_write"); - if (n == 0) - err(1, "connection closed"); } for (;;) { @@ -1052,10 +1048,11 @@ send_url(const char *url) if (connect(ctl_sock, (struct sockaddr *)&sun, sizeof(sun)) == -1) err(1, "connect: %s", ctlsock_path); - imsg_init(&ibuf, ctl_sock); + if (imsgbuf_init(&ibuf, ctl_sock) == -1) + err(1, "imsgbuf_init"); imsg_compose(&ibuf, IMSG_CTL_OPEN_URL, 0, 0, -1, url, strlen(url) + 1); - imsg_flush(&ibuf); + imsgbuf_flush(&ibuf); close(ctl_sock); } @@ -1192,7 +1189,8 @@ main(int argc, char * const *argv) if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe2net) == -1) err(1, "socketpair"); start_child(PROC_NET, argv0, pipe2net[1]); - imsg_init(&net_ibuf.ibuf, pipe2net[0]); + if (imsgbuf_init(&net_ibuf.ibuf, pipe2net[0]) == -1) + err(1, "imsgbuf_init"); iev_net = &net_ibuf; iev_net->handler = handle_dispatch_imsg; @@ -1232,7 +1230,7 @@ main(int argc, char * const *argv) } ui_send_net(IMSG_QUIT, 0, -1, NULL, 0); - imsg_flush(&iev_net->ibuf); + imsgbuf_flush(&iev_net->ibuf); /* wait for children to terminate */ do {