From 252f41818e4fa683e9ba4914bce0672754b47123 Mon Sep 17 00:00:00 2001 From: Nicholas Marriott Date: Wed, 27 Nov 2024 10:30:52 +0000 Subject: [PATCH] Update imsg and remove workaround. --- compat/imsg-buffer.c | 38 +++++++++++--------------------- compat/imsg.c | 52 ++++++++++++++++++++++++++++++-------------- compat/imsg.h | 8 +++---- proc.c | 24 ++------------------ server-client.c | 4 ++-- tmux.h | 1 - 6 files changed, 57 insertions(+), 70 deletions(-) diff --git a/compat/imsg-buffer.c b/compat/imsg-buffer.c index 74485bbd..f5f36a69 100644 --- a/compat/imsg-buffer.c +++ b/compat/imsg-buffer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: imsg-buffer.c,v 1.30 2024/11/22 07:20:50 tb Exp $ */ +/* $OpenBSD: imsg-buffer.c,v 1.31 2024/11/26 13:57:31 claudio Exp $ */ /* * Copyright (c) 2023 Claudio Jeker @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -51,7 +52,7 @@ struct msgbuf { uint32_t queued; char *rbuf; struct ibuf *rpmsg; - ssize_t (*readhdr)(struct ibuf *, void *); + struct ibuf *(*readhdr)(struct ibuf *, void *, int *); void *rarg; size_t roff; size_t hdrsize; @@ -587,8 +588,8 @@ msgbuf_new(void) } struct msgbuf * -msgbuf_new_reader(size_t hdrsz, ssize_t (*readhdr)(struct ibuf *, void *), - void *arg) +msgbuf_new_reader(size_t hdrsz, + struct ibuf *(*readhdr)(struct ibuf *, void *, int *), void *arg) { struct msgbuf *msgbuf; char *buf; @@ -771,33 +772,16 @@ ibuf_read_process(struct msgbuf *msgbuf, int fd) ibuf_from_buffer(&rbuf, msgbuf->rbuf, msgbuf->roff); - /* fds must be passed at start of message of at least hdrsize bytes */ - if (msgbuf->rpmsg != NULL && fd != -1) { - close(fd); - fd = -1; - } - do { if (msgbuf->rpmsg == NULL) { - if (ibuf_size(&rbuf) < msgbuf->hdrsize) { - if (fd != -1) { - close(fd); - fd = -1; - } + if (ibuf_size(&rbuf) < msgbuf->hdrsize) break; - } /* get size from header */ ibuf_from_buffer(&msg, ibuf_data(&rbuf), msgbuf->hdrsize); - sz = msgbuf->readhdr(&msg, msgbuf->rarg); - if (sz == -1) + if ((msgbuf->rpmsg = msgbuf->readhdr(&msg, + msgbuf->rarg, &fd)) == NULL) goto fail; - if ((msgbuf->rpmsg = ibuf_open(sz)) == NULL) - goto fail; - if (fd != -1) { - ibuf_fd_set(msgbuf->rpmsg, fd); - fd = -1; - } } if (ibuf_left(msgbuf->rpmsg) <= ibuf_size(&rbuf)) @@ -820,10 +804,14 @@ ibuf_read_process(struct msgbuf *msgbuf, int fd) memmove(msgbuf->rbuf, ibuf_data(&rbuf), ibuf_size(&rbuf)); msgbuf->roff = ibuf_size(&rbuf); + if (fd != -1) + close(fd); return (1); fail: - /* XXX cleanup */ + /* XXX how to properly clean up is unclear */ + if (fd != -1) + close(fd); return (-1); } diff --git a/compat/imsg.c b/compat/imsg.c index b4e59869..5b1d2648 100644 --- a/compat/imsg.c +++ b/compat/imsg.c @@ -1,4 +1,4 @@ -/* $OpenBSD: imsg.c,v 1.36 2024/11/21 13:03:21 claudio Exp $ */ +/* $OpenBSD: imsg.c,v 1.37 2024/11/26 13:57:31 claudio Exp $ */ /* * Copyright (c) 2023 Claudio Jeker @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -30,8 +31,9 @@ #include "imsg.h" #define IMSG_ALLOW_FDPASS 0x01 +#define IMSG_FD_MARK 0x80000000U -static ssize_t imsg_parse_hdr(struct ibuf *, void *); +static struct ibuf *imsg_parse_hdr(struct ibuf *, void *, int *); int imsgbuf_init(struct imsgbuf *imsgbuf, int fd) @@ -53,12 +55,15 @@ imsgbuf_allow_fdpass(struct imsgbuf *imsgbuf) imsgbuf->flags |= IMSG_ALLOW_FDPASS; } -void +int imsgbuf_set_maxsize(struct imsgbuf *imsgbuf, uint32_t maxsize) { - if (maxsize < IMSG_HEADER_SIZE) - return; + if (maxsize < IMSG_HEADER_SIZE || maxsize & IMSG_FD_MARK) { + errno = EINVAL; + return (-1); + } imsgbuf->maxsize = maxsize; + return (0); } int @@ -119,6 +124,7 @@ imsg_get(struct imsgbuf *imsgbuf, struct imsg *imsg) else m.data = NULL; m.buf = buf; + m.hdr.len &= ~IMSG_FD_MARK; *imsg = m; return (ibuf_size(buf) + IMSG_HEADER_SIZE); @@ -267,7 +273,7 @@ int imsg_forward(struct imsgbuf *imsgbuf, struct imsg *msg) { struct ibuf *wbuf; - size_t len = 0; + size_t len; ibuf_rewind(msg->buf); ibuf_skip(msg->buf, sizeof(msg->hdr)); @@ -277,7 +283,7 @@ imsg_forward(struct imsgbuf *imsgbuf, struct imsg *msg) msg->hdr.pid, len)) == NULL) return (-1); - if (msg->buf != NULL) { + if (len != 0) { if (ibuf_add_ibuf(wbuf, msg->buf) == -1) { ibuf_free(wbuf); return (-1); @@ -329,9 +335,12 @@ void imsg_close(struct imsgbuf *imsgbuf, struct ibuf *msg) { struct imsg_hdr *hdr; + uint32_t len; - hdr = (struct imsg_hdr *)msg->buf; - hdr->len = ibuf_size(msg); + len = ibuf_size(msg); + if (ibuf_fd_avail(msg)) + len |= IMSG_FD_MARK; + (void)ibuf_set_h32(msg, offsetof(struct imsg_hdr, len), len); ibuf_close(imsgbuf->w, msg); } @@ -341,18 +350,29 @@ imsg_free(struct imsg *imsg) ibuf_free(imsg->buf); } -static ssize_t -imsg_parse_hdr(struct ibuf *buf, void *arg) +static struct ibuf * +imsg_parse_hdr(struct ibuf *buf, void *arg, int *fd) { struct imsgbuf *imsgbuf = arg; struct imsg_hdr hdr; + struct ibuf *b; + uint32_t len; if (ibuf_get(buf, &hdr, sizeof(hdr)) == -1) - return -1; - if (hdr.len < IMSG_HEADER_SIZE || - hdr.len > imsgbuf->maxsize) { + return (NULL); + + len = hdr.len & ~IMSG_FD_MARK; + + if (len < IMSG_HEADER_SIZE || len > imsgbuf->maxsize) { errno = ERANGE; - return (-1); + return (NULL); } - return hdr.len; + if ((b = ibuf_open(len)) == NULL) + return (NULL); + if (hdr.len & IMSG_FD_MARK) { + ibuf_fd_set(b, *fd); + *fd = -1; + } + + return b; } diff --git a/compat/imsg.h b/compat/imsg.h index 8d78beb3..462bfc97 100644 --- a/compat/imsg.h +++ b/compat/imsg.h @@ -1,4 +1,4 @@ -/* $OpenBSD: imsg.h,v 1.18 2024/11/21 13:03:21 claudio Exp $ */ +/* $OpenBSD: imsg.h,v 1.19 2024/11/26 13:57:31 claudio Exp $ */ /* * Copyright (c) 2023 Claudio Jeker @@ -110,8 +110,8 @@ int ibuf_fd_avail(struct ibuf *); int ibuf_fd_get(struct ibuf *); void ibuf_fd_set(struct ibuf *, int); struct msgbuf *msgbuf_new(void); -struct msgbuf *msgbuf_new_reader(size_t, ssize_t (*)(struct ibuf *, void *), - 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 *); @@ -124,7 +124,7 @@ struct ibuf *msgbuf_get(struct msgbuf *); /* imsg.c */ int imsgbuf_init(struct imsgbuf *, int); void imsgbuf_allow_fdpass(struct imsgbuf *imsgbuf); -void imsgbuf_set_maxsize(struct imsgbuf *, uint32_t); +int imsgbuf_set_maxsize(struct imsgbuf *, uint32_t); int imsgbuf_read(struct imsgbuf *); int imsgbuf_write(struct imsgbuf *); int imsgbuf_flush(struct imsgbuf *); diff --git a/proc.c b/proc.c index 587da26f..01281d02 100644 --- a/proc.c +++ b/proc.c @@ -55,7 +55,6 @@ struct tmuxpeer { struct tmuxproc *parent; struct imsgbuf ibuf; - int lastfd; struct event event; uid_t uid; @@ -72,7 +71,7 @@ static int peer_check_version(struct tmuxpeer *, struct imsg *); static void proc_update_event(struct tmuxpeer *); static void -proc_event_cb(int fd, short events, void *arg) +proc_event_cb(__unused int fd, short events, void *arg) { struct tmuxpeer *peer = arg; ssize_t n; @@ -90,14 +89,7 @@ proc_event_cb(int fd, short events, void *arg) } if (n == 0) break; - fd = imsg_get_fd(&imsg); - log_debug("peer %p message %d fd %d", peer, - imsg.hdr.type, fd); - if (fd != -1) { - if (peer->lastfd != -1) - close(peer->lastfd); - peer->lastfd = fd; - } + log_debug("peer %p message %d", peer, imsg.hdr.type); if (peer_check_version(peer, &imsg) != 0) { imsg_free(&imsg); @@ -313,7 +305,6 @@ proc_add_peer(struct tmuxproc *tp, int fd, peer = xcalloc(1, sizeof *peer); peer->parent = tp; - peer->lastfd = -1; peer->dispatchcb = dispatchcb; peer->arg = arg; @@ -342,8 +333,6 @@ proc_remove_peer(struct tmuxpeer *peer) event_del(&peer->event); imsgbuf_clear(&peer->ibuf); - if (peer->lastfd != -1) - close(peer->lastfd); close(peer->ibuf.fd); free(peer); } @@ -395,12 +384,3 @@ proc_get_peer_uid(struct tmuxpeer *peer) { return (peer->uid); } - -int -proc_get_last_fd(struct tmuxpeer *peer) -{ - int fd = peer->lastfd; - - peer->lastfd = -1; - return (fd); -} diff --git a/server-client.c b/server-client.c index c7218827..10298c83 100644 --- a/server-client.c +++ b/server-client.c @@ -3559,13 +3559,13 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) case MSG_IDENTIFY_STDIN: if (datalen != 0) fatalx("bad MSG_IDENTIFY_STDIN size"); - c->fd = proc_get_last_fd(c->peer); + c->fd = imsg_get_fd(imsg); log_debug("client %p IDENTIFY_STDIN %d", c, c->fd); break; case MSG_IDENTIFY_STDOUT: if (datalen != 0) fatalx("bad MSG_IDENTIFY_STDOUT size"); - c->out_fd = proc_get_last_fd(c->peer); + c->out_fd = imsg_get_fd(imsg); log_debug("client %p IDENTIFY_STDOUT %d", c, c->out_fd); break; case MSG_IDENTIFY_ENVIRON: diff --git a/tmux.h b/tmux.h index 2b837a3c..8cffe09c 100644 --- a/tmux.h +++ b/tmux.h @@ -2225,7 +2225,6 @@ void proc_flush_peer(struct tmuxpeer *); void proc_toggle_log(struct tmuxproc *); pid_t proc_fork_and_daemon(int *); uid_t proc_get_peer_uid(struct tmuxpeer *); -int proc_get_last_fd(struct tmuxpeer *); /* cfg.c */ extern int cfg_finished;