From eaa58d28dc7da9b2ef0d77f4c8e85aab55b71935 Mon Sep 17 00:00:00 2001 From: nicm Date: Mon, 16 Dec 2019 15:48:50 +0000 Subject: [PATCH 1/3] Instead of using large buffers in imsgs, add the data or path onto the end. --- client.c | 48 ++++++++++++++++++++++++++-------------- file.c | 59 ++++++++++++++++++++++++++++++------------------- server-client.c | 6 ++--- tmux.h | 12 +++------- 4 files changed, 73 insertions(+), 52 deletions(-) diff --git a/client.c b/client.c index 4bebb977..38fe52cd 100644 --- a/client.c +++ b/client.c @@ -486,14 +486,19 @@ static void client_write_open(void *data, size_t datalen) { struct msg_write_open *msg = data; + const char *path; struct msg_write_ready reply; struct client_file find, *cf; const int flags = O_NONBLOCK|O_WRONLY|O_CREAT; int error = 0; - if (datalen != sizeof *msg) + if (datalen < sizeof *msg) fatalx("bad MSG_WRITE_OPEN size"); - log_debug("open write file %d %s", msg->stream, msg->path); + if (datalen == sizeof *msg) + path = "-"; + else + path = (const char *)(msg + 1); + log_debug("open write file %d %s", msg->stream, path); find.stream = msg->stream; if ((cf = RB_FIND(client_files, &client_files, &find)) == NULL) { @@ -510,7 +515,7 @@ client_write_open(void *data, size_t datalen) cf->fd = -1; if (msg->fd == -1) - cf->fd = open(msg->path, msg->flags|flags, 0644); + cf->fd = open(path, msg->flags|flags, 0644); else { if (msg->fd != STDOUT_FILENO && msg->fd != STDERR_FILENO) errno = EBADF; @@ -542,16 +547,17 @@ client_write_data(void *data, size_t datalen) { struct msg_write_data *msg = data; struct client_file find, *cf; + size_t size = datalen - sizeof *msg; - if (datalen != sizeof *msg) + if (datalen < sizeof *msg) fatalx("bad MSG_WRITE size"); find.stream = msg->stream; if ((cf = RB_FIND(client_files, &client_files, &find)) == NULL) fatalx("unknown stream number"); - log_debug("write %zu to file %d", msg->size, cf->stream); + log_debug("write %zu to file %d", size, cf->stream); if (cf->event != NULL) - bufferevent_write(cf->event, msg->data, msg->size); + bufferevent_write(cf->event, msg + 1, size); } /* Close client file. */ @@ -585,26 +591,29 @@ client_read_callback(__unused struct bufferevent *bev, void *arg) struct client_file *cf = arg; void *bdata; size_t bsize; - struct msg_read_data msg; + struct msg_read_data *msg; + size_t msglen; + msg = xmalloc(sizeof *msg); for (;;) { bdata = EVBUFFER_DATA(cf->event->input); bsize = EVBUFFER_LENGTH(cf->event->input); if (bsize == 0) break; - if (bsize > sizeof msg.data) - bsize = sizeof msg.data; + if (bsize > MAX_IMSGSIZE - IMSG_HEADER_SIZE) + bsize = MAX_IMSGSIZE - IMSG_HEADER_SIZE; log_debug("read %zu from file %d", bsize, cf->stream); - memcpy(msg.data, bdata, bsize); - msg.size = bsize; - - msg.stream = cf->stream; - proc_send(client_peer, MSG_READ, -1, &msg, sizeof msg); + msglen = (sizeof *msg) + bsize; + msg = xrealloc(msg, msglen); + msg->stream = cf->stream; + memcpy(msg + 1, bdata, bsize); + proc_send(client_peer, MSG_READ, -1, msg, msglen); evbuffer_drain(cf->event->input, bsize); } + free(msg); } /* File read error callback. */ @@ -632,14 +641,19 @@ static void client_read_open(void *data, size_t datalen) { struct msg_read_open *msg = data; + const char *path; struct msg_read_done reply; struct client_file find, *cf; const int flags = O_NONBLOCK|O_RDONLY; int error = 0; - if (datalen != sizeof *msg) + if (datalen < sizeof *msg) fatalx("bad MSG_READ_OPEN size"); - log_debug("open read file %d %s", msg->stream, msg->path); + if (datalen == sizeof *msg) + path = "-"; + else + path = (const char *)(msg + 1); + log_debug("open read file %d %s", msg->stream, path); find.stream = msg->stream; if ((cf = RB_FIND(client_files, &client_files, &find)) == NULL) { @@ -656,7 +670,7 @@ client_read_open(void *data, size_t datalen) cf->fd = -1; if (msg->fd == -1) - cf->fd = open(msg->path, flags); + cf->fd = open(path, flags); else { if (msg->fd != STDIN_FILENO) errno = EBADF; diff --git a/file.c b/file.c index f0d622bc..7e1f1879 100644 --- a/file.c +++ b/file.c @@ -17,9 +17,11 @@ */ #include +#include #include #include +#include #include #include #include @@ -147,7 +149,6 @@ file_vprint(struct client *c, const char *fmt, va_list ap) msg.stream = 1; msg.fd = STDOUT_FILENO; msg.flags = 0; - strlcpy(msg.path, "-", sizeof msg.path); proc_send(c->peer, MSG_WRITE_OPEN, -1, &msg, sizeof msg); } else { evbuffer_add_vprintf(cf->buffer, fmt, ap); @@ -174,7 +175,6 @@ file_print_buffer(struct client *c, void *data, size_t size) msg.stream = 1; msg.fd = STDOUT_FILENO; msg.flags = 0; - strlcpy(msg.path, "-", sizeof msg.path); proc_send(c->peer, MSG_WRITE_OPEN, -1, &msg, sizeof msg); } else { evbuffer_add(cf->buffer, data, size); @@ -204,7 +204,6 @@ file_error(struct client *c, const char *fmt, ...) msg.stream = 2; msg.fd = STDERR_FILENO; msg.flags = 0; - strlcpy(msg.path, "-", sizeof msg.path); proc_send(c->peer, MSG_WRITE_OPEN, -1, &msg, sizeof msg); } else { evbuffer_add_vprintf(cf->buffer, fmt, ap); @@ -220,7 +219,8 @@ file_write(struct client *c, const char *path, int flags, const void *bdata, { struct client_file *cf; FILE *f; - struct msg_write_open msg; + struct msg_write_open *msg; + size_t msglen; int fd = -1; const char *mode; @@ -261,17 +261,22 @@ file_write(struct client *c, const char *path, int flags, const void *bdata, skip: evbuffer_add(cf->buffer, bdata, bsize); - msg.stream = cf->stream; - msg.fd = fd; - msg.flags = flags; - if (strlcpy(msg.path, cf->path, sizeof msg.path) >= sizeof msg.path) { + msglen = strlen(cf->path) + 1 + sizeof *msg; + if (msglen > MAX_IMSGSIZE - IMSG_HEADER_SIZE) { cf->error = E2BIG; goto done; } - if (proc_send(c->peer, MSG_WRITE_OPEN, -1, &msg, sizeof msg) != 0) { + msg = xmalloc(msglen); + msg->stream = cf->stream; + msg->fd = fd; + msg->flags = flags; + memcpy(msg + 1, cf->path, msglen - sizeof *msg); + if (proc_send(c->peer, MSG_WRITE_OPEN, -1, msg, msglen) != 0) { + free(msg); cf->error = EINVAL; goto done; } + free(msg); return; done: @@ -283,10 +288,10 @@ file_read(struct client *c, const char *path, client_file_cb cb, void *cbdata) { struct client_file *cf; FILE *f; - struct msg_read_open msg; + struct msg_read_open *msg; + size_t msglen, size; int fd = -1; char buffer[BUFSIZ]; - size_t size; if (strcmp(path, "-") == 0) { cf = file_create(c, file_next_stream++, cb, cbdata); @@ -327,16 +332,21 @@ file_read(struct client *c, const char *path, client_file_cb cb, void *cbdata) } skip: - msg.stream = cf->stream; - msg.fd = fd; - if (strlcpy(msg.path, cf->path, sizeof msg.path) >= sizeof msg.path) { + msglen = strlen(cf->path) + 1 + sizeof *msg; + if (msglen > MAX_IMSGSIZE - IMSG_HEADER_SIZE) { cf->error = E2BIG; goto done; } - if (proc_send(c->peer, MSG_READ_OPEN, -1, &msg, sizeof msg) != 0) { + msg = xmalloc(msglen); + msg->stream = cf->stream; + msg->fd = fd; + memcpy(msg + 1, cf->path, msglen - sizeof *msg); + if (proc_send(c->peer, MSG_READ_OPEN, -1, msg, msglen) != 0) { + free(msg); cf->error = EINVAL; goto done; } + free(msg); return; done: @@ -358,20 +368,22 @@ void file_push(struct client_file *cf) { struct client *c = cf->c; - struct msg_write_data msg; + struct msg_write_data *msg; + size_t msglen, sent, left; struct msg_write_close close; - size_t sent, left; + msg = xmalloc(sizeof *msg); left = EVBUFFER_LENGTH(cf->buffer); while (left != 0) { sent = left; - if (sent > sizeof msg.data) - sent = sizeof msg.data; - memcpy(msg.data, EVBUFFER_DATA(cf->buffer), sent); - msg.size = sent; + if (sent > MAX_IMSGSIZE - IMSG_HEADER_SIZE) + sent = MAX_IMSGSIZE - IMSG_HEADER_SIZE; - msg.stream = cf->stream; - if (proc_send(c->peer, MSG_WRITE, -1, &msg, sizeof msg) != 0) + msglen = (sizeof *msg) + sent; + msg = xrealloc(msg, msglen); + msg->stream = cf->stream; + memcpy(msg + 1, EVBUFFER_DATA(cf->buffer), sent); + if (proc_send(c->peer, MSG_WRITE, -1, msg, msglen) != 0) break; evbuffer_drain(cf->buffer, sent); @@ -387,4 +399,5 @@ file_push(struct client_file *cf) proc_send(c->peer, MSG_WRITE_CLOSE, -1, &close, sizeof close); file_fire_done(cf); } + free(msg); } diff --git a/server-client.c b/server-client.c index e6e4d8a9..fdedc35f 100644 --- a/server-client.c +++ b/server-client.c @@ -2024,10 +2024,10 @@ server_client_dispatch_read_data(struct client *c, struct imsg *imsg) struct msg_read_data *msg = imsg->data; size_t msglen = imsg->hdr.len - IMSG_HEADER_SIZE; struct client_file find, *cf; - void *bdata = msg->data; - size_t bsize = msg->size; + void *bdata = msg + 1; + size_t bsize = msglen - sizeof *msg; - if (msglen != sizeof *msg) + if (msglen < sizeof *msg) fatalx("bad MSG_READ_DATA size"); find.stream = msg->stream; if ((cf = RB_FIND(client_files, &c->files, &find)) == NULL) diff --git a/tmux.h b/tmux.h index 60e55475..84b2056c 100644 --- a/tmux.h +++ b/tmux.h @@ -507,13 +507,10 @@ struct msg_command { struct msg_read_open { int stream; int fd; - char path[PATH_MAX]; -}; +}; /* followed by path */ struct msg_read_data { int stream; - size_t size; - char data[BUFSIZ]; }; struct msg_read_done { @@ -524,15 +521,12 @@ struct msg_read_done { struct msg_write_open { int stream; int fd; - char path[PATH_MAX]; int flags; -}; +}; /* followed by path */ struct msg_write_data { int stream; - size_t size; - char data[BUFSIZ]; -}; +}; /* followed by data */ struct msg_write_ready { int stream; From b4520aaf2cb56cd14519e2df9d99ea6efc8ddd03 Mon Sep 17 00:00:00 2001 From: nicm Date: Mon, 16 Dec 2019 16:09:28 +0000 Subject: [PATCH 2/3] Need to include message size in the maximum buffer calculation. --- client.c | 4 ++-- file.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client.c b/client.c index 38fe52cd..a36c6471 100644 --- a/client.c +++ b/client.c @@ -601,8 +601,8 @@ client_read_callback(__unused struct bufferevent *bev, void *arg) if (bsize == 0) break; - if (bsize > MAX_IMSGSIZE - IMSG_HEADER_SIZE) - bsize = MAX_IMSGSIZE - IMSG_HEADER_SIZE; + if (bsize > MAX_IMSGSIZE - IMSG_HEADER_SIZE - sizeof *msg) + bsize = MAX_IMSGSIZE - IMSG_HEADER_SIZE - sizeof *msg; log_debug("read %zu from file %d", bsize, cf->stream); msglen = (sizeof *msg) + bsize; diff --git a/file.c b/file.c index 7e1f1879..9a3e79de 100644 --- a/file.c +++ b/file.c @@ -376,8 +376,8 @@ file_push(struct client_file *cf) left = EVBUFFER_LENGTH(cf->buffer); while (left != 0) { sent = left; - if (sent > MAX_IMSGSIZE - IMSG_HEADER_SIZE) - sent = MAX_IMSGSIZE - IMSG_HEADER_SIZE; + if (sent > MAX_IMSGSIZE - IMSG_HEADER_SIZE - sizeof *msg) + sent = MAX_IMSGSIZE - IMSG_HEADER_SIZE - sizeof *msg; msglen = (sizeof *msg) + sent; msg = xrealloc(msg, msglen); From 1bdd4828bd0a13d6fb81c903078ad99ff2429b5d Mon Sep 17 00:00:00 2001 From: nicm Date: Mon, 16 Dec 2019 16:39:03 +0000 Subject: [PATCH 3/3] If /dev/fd/X is a symlink and realpath() expands symlinks, /dev/fd/X ends up pointing to the wrong place before it is passed to the client. The path is only used internally so there is no real need for realpath(), remove it and move the get_path function to file.c where all the callers are. --- file.c | 16 ++++++++++++++-- server-client.c | 16 ---------------- tmux.h | 1 - 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/file.c b/file.c index 9a3e79de..439d3464 100644 --- a/file.c +++ b/file.c @@ -33,6 +33,18 @@ static int file_next_stream = 3; RB_GENERATE(client_files, client_file, entry, file_cmp); +static char * +file_get_path(struct client *c, const char *file) +{ + char *path; + + if (*file == '/') + path = xstrdup(file); + else + xasprintf(&path, "%s/%s", server_client_get_cwd(c, NULL), file); + return (path); +} + int file_cmp(struct client_file *cf1, struct client_file *cf2) { @@ -237,7 +249,7 @@ file_write(struct client *c, const char *path, int flags, const void *bdata, } cf = file_create(c, file_next_stream++, cb, cbdata); - cf->path = server_client_get_path(c, path); + cf->path = file_get_path(c, path); if (c == NULL || c->flags & CLIENT_ATTACHED) { if (flags & O_APPEND) @@ -306,7 +318,7 @@ file_read(struct client *c, const char *path, client_file_cb cb, void *cbdata) } cf = file_create(c, file_next_stream++, cb, cbdata); - cf->path = server_client_get_path(c, path); + cf->path = file_get_path(c, path); if (c == NULL || c->flags & CLIENT_ATTACHED) { f = fopen(cf->path, "rb"); diff --git a/server-client.c b/server-client.c index fdedc35f..ee7b4c70 100644 --- a/server-client.c +++ b/server-client.c @@ -2111,19 +2111,3 @@ server_client_get_cwd(struct client *c, struct session *s) return (home); return ("/"); } - -/* Resolve an absolute path or relative to client working directory. */ -char * -server_client_get_path(struct client *c, const char *file) -{ - char *path, resolved[PATH_MAX]; - - if (*file == '/') - path = xstrdup(file); - else - xasprintf(&path, "%s/%s", server_client_get_cwd(c, NULL), file); - if (realpath(path, resolved) == NULL) - return (path); - free(path); - return (xstrdup(resolved)); -} diff --git a/tmux.h b/tmux.h index 84b2056c..8b23bfae 100644 --- a/tmux.h +++ b/tmux.h @@ -2228,7 +2228,6 @@ void server_client_push_stdout(struct client *); void server_client_push_stderr(struct client *); void printflike(2, 3) server_client_add_message(struct client *, const char *, ...); -char *server_client_get_path(struct client *, const char *); const char *server_client_get_cwd(struct client *, struct session *); /* server-fn.c */