From 6a44eaeb097cb837a9d2a7f6b2fda2cc774a93fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 24 Oct 2015 20:22:24 +0200 Subject: [PATCH] libqrexec-utils: bring back buffered write helpers It is required to prevent deadlocks in single-threaded select-based IO programs (namely: qrexec). POSIX API doesn't support checking how much can be written to pipe/socket without blocking, so to prevent blocking application must use O_NONBLOCK mode, and somehow deal with non-written data (buffer it). QubesOS/qubes-issues#1347 --- qrexec-lib/Makefile | 2 +- qrexec-lib/libqrexec-utils.h | 8 ++++++++ qrexec-lib/write-stdin.c | 35 ++++++++--------------------------- 3 files changed, 17 insertions(+), 28 deletions(-) diff --git a/qrexec-lib/Makefile b/qrexec-lib/Makefile index 0bc5189..6bfc3e1 100644 --- a/qrexec-lib/Makefile +++ b/qrexec-lib/Makefile @@ -12,7 +12,7 @@ endif all: libqrexec-utils.so.$(SO_VER) libqubes-rpc-filecopy.so.$(SO_VER) -libqrexec-utils.so.$(SO_VER): unix-server.o ioall.o buffer.o exec.o txrx-vchan.o +libqrexec-utils.so.$(SO_VER): unix-server.o ioall.o buffer.o exec.o txrx-vchan.o write-stdin.o $(CC) $(LDFLAGS) -Wl,-soname,$@ -o $@ $^ $(VCHANLIBS) libqubes-rpc-filecopy.so.$(SO_VER): ioall.o copy-file.o crc32.o unpack.o $(CC) $(LDFLAGS) -Wl,-soname,$@ -o $@ $^ diff --git a/qrexec-lib/libqrexec-utils.h b/qrexec-lib/libqrexec-utils.h index 8e7680c..e0c0788 100644 --- a/qrexec-lib/libqrexec-utils.h +++ b/qrexec-lib/libqrexec-utils.h @@ -28,6 +28,11 @@ struct buffer { int buflen; }; +/* return codes for buffered writes */ +#define WRITE_STDIN_OK 0 /* all written */ +#define WRITE_STDIN_BUFFERED 1 /* something still in the buffer */ +#define WRITE_STDIN_ERROR 2 /* write error, errno set */ + typedef void (do_exec_t)(const char *); void register_exec_func(do_exec_t *func); @@ -38,6 +43,9 @@ void buffer_remove(struct buffer *b, int len); int buffer_len(struct buffer *b); void *buffer_data(struct buffer *b); +int flush_client_data(int fd, struct buffer *buffer); +int write_stdin(int fd, const char *data, int len, struct buffer *buffer); +int fork_and_flush_stdin(int fd, struct buffer *buffer); void do_fork_exec(const char *cmdline, int *pid, int *stdin_fd, int *stdout_fd, int *stderr_fd); diff --git a/qrexec-lib/write-stdin.c b/qrexec-lib/write-stdin.c index 4aed9dd..ebf32bf 100644 --- a/qrexec-lib/write-stdin.c +++ b/qrexec-lib/write-stdin.c @@ -29,16 +29,18 @@ #include "libqrexec-utils.h" /* -There is buffered data in "buffer" for client id "client_id", and select() -reports that "fd" is writable. Write as much as possible to fd, if all sent, -notify the peer that this client's pipe is no longer full. +There is buffered data in "buffer" for client and select() +reports that "fd" is writable. Write as much as possible to fd. */ -int flush_client_data(libvchan_t *vchan, int fd, int client_id, struct buffer *buffer) +int flush_client_data(int fd, struct buffer *buffer) { int ret; int len; for (;;) { len = buffer_len(buffer); + if (!len) { + return WRITE_STDIN_OK; + } if (len > MAX_DATA_CHUNK) len = MAX_DATA_CHUNK; ret = write(fd, buffer_data(buffer), len); @@ -52,27 +54,15 @@ int flush_client_data(libvchan_t *vchan, int fd, int client_id, struct buffer *b // it will be wrong if we change MAX_DATA_CHUNK to something large // as pipes writes are atomic only to PIPE_MAX limit buffer_remove(buffer, ret); - len = buffer_len(buffer); - if (!len) { - struct server_header s_hdr; - s_hdr.type = MSG_XON; - s_hdr.client_id = client_id; - s_hdr.len = 0; - if (libvchan_send(vchan, (char*)&s_hdr, sizeof s_hdr) < 0) - return WRITE_STDIN_ERROR; - return WRITE_STDIN_OK; - } } } /* Write "len" bytes from "data" to "fd". If not all written, buffer the rest -to "buffer", and notify the peer that the client "client_id" pipe is full via -MSG_XOFF message. +to "buffer". */ -int write_stdin(libvchan_t *vchan, int fd, int client_id, const char *data, int len, - struct buffer *buffer) +int write_stdin(int fd, const char *data, int len, struct buffer *buffer) { int ret; int written = 0; @@ -88,26 +78,17 @@ int write_stdin(libvchan_t *vchan, int fd, int client_id, const char *data, int exit(1); } if (ret == -1) { - struct server_header s_hdr; - if (errno != EAGAIN) return WRITE_STDIN_ERROR; buffer_append(buffer, data + written, len - written); - s_hdr.type = MSG_XOFF; - s_hdr.client_id = client_id; - s_hdr.len = 0; - if (libvchan_send(vchan, (char*)&s_hdr, sizeof s_hdr) < 0) - return WRITE_STDIN_ERROR; - return WRITE_STDIN_BUFFERED; } written += ret; } return WRITE_STDIN_OK; - } /*