From 6b1ef63d79393906fe8f090235c118aac9104f9a Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 14:37:39 +0000 Subject: [PATCH 1/9] transfused: improve read_message robustness Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index bc722101b..7474412f0 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -184,21 +184,24 @@ uint64_t message_id(uint64_t *message) int read_message(char *descr, parameters *params, int fd, char *buf, size_t max_read){ - int read_count; + ssize_t read_count; + size_t upto = 0; size_t nbyte; uint32_t len; - /* TODO: socket read conditions e.g.EAGAIN */ - read_count = read(fd, buf, 4); - if (read_count != 4) { - if (read_count < 0) + do { + read_count = read(fd, buf + upto, sizeof(uint32_t) - upto); + if (read_count == -1) { + if (errno == EAGAIN || errno == EINTR) + continue; die(1, params, "", "read %s: error reading: ", descr); + } if (read_count == 0) die(1, params, NULL, "read %s: EOF reading length", descr); - die(1, params, NULL, - "read %s: short read length %d", descr, read_count); - } + upto += read_count; + } while (upto < sizeof(uint32_t)); + len = *((uint32_t *) buf); if (len > max_read) die(1, params, NULL, @@ -209,10 +212,12 @@ int read_message(char *descr, parameters *params, int fd, buf += 4; do { - /* TODO: socket read conditions e.g.EAGAIN */ read_count = read(fd, buf, nbyte); - if (read_count < 0) + if (read_count < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; die(1, params, "", "read %s: error reading: ", descr); + } if (read_count == 0) die(1, params, NULL, "read %s: EOF reading", descr); nbyte -= read_count; From 78328cf026f12b18c8f713476030e4ce1e344eca Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 14:38:07 +0000 Subject: [PATCH 2/9] transfused: improve write_exactly error handling Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index 7474412f0..2f01a739c 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -303,10 +303,12 @@ void write_exactly(char *descr, int fd, void *p, size_t nbyte) char *buf = p; do { - /* TODO: socket write conditions e.g.EAGAIN */ write_count = write(fd, buf, nbyte); - if (write_count < 0) + if (write_count < 0) { + if (errno == EINTR || errno == EAGAIN) + continue; die(1, NULL, "", "%s: error writing: ", descr); + } if (write_count == 0) die(1, NULL, "", "%s: 0 write: ", descr); From 6cf50dd803792eebc8d0dd24c189044d949ac11e Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 14:40:33 +0000 Subject: [PATCH 3/9] transfused: replace brittle event pid writing with write_exactly Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index 2f01a739c..33df672d8 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -744,23 +744,14 @@ void write_pid(connection_t *connection) { pid_t pid = gettid(); char *pid_s; - int pid_s_len, write_count; + int pid_s_len; if (asprintf(&pid_s, "%lld", (long long)pid) == -1) die(1, connection->params, "Couldn't allocate pid string", ""); pid_s_len = strlen(pid_s); - /* TODO: check for socket write conditions e.g.EAGAIN */ - write_count = write(connection->sock, pid_s, pid_s_len); - if (write_count < 0) - die(1, connection->params, "Error writing pid", ""); - - /* TODO: handle short writes */ - if (write_count != pid_s_len) - die(1, connection->params, NULL, - "Error writing pid %s to socket: only wrote %d bytes", - pid_s, write_count); + write_exactly("pid", connection->sock, pid_s, pid_s_len); free(pid_s); } From b9ff275a88385e7deca7d5b68aec3be4d7d85f5a Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 14:41:00 +0000 Subject: [PATCH 4/9] transfused: replace brittle init control message writing with write_exactly Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index 33df672d8..2148921df 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -927,19 +927,14 @@ void *determine_mount_suitability(parameters *params, int allow_empty, void *init_thread(void *params_ptr) { parameters *params = params_ptr; - int write_count, read_count, len; + int read_count, len; char init_msg[6] = {'\6', '\0', '\0', '\0', '\0', '\0'}; void *buf, *response; uint16_t msg_type; params->ctl_sock = connect_socket(params->server); - /* TODO: handle short write/socket conditions */ - write_count = write(params->ctl_sock, init_msg, sizeof(init_msg)); - if (write_count < 0) - die(1, NULL, "init thread: couldn't write init", ""); - if (write_count != sizeof(init_msg)) - die(1, NULL, "init thread: incomplete write", ""); + write_exactly("init", params->ctl_sock, init_msg, sizeof(init_msg)); buf = must_malloc("incoming control message buffer", CTL_BUFSZ); From 8580cfec80f0e380a6ee0a998a4fea3bde78b688 Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 14:44:09 +0000 Subject: [PATCH 5/9] transfused: minor formatting fixup Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index 2148921df..d786f46db 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -820,7 +820,7 @@ void perform_syscall(connection_t *conn, uint8_t syscall, char path[]) name, path, strerror(errno)); } -void * event_thread(void *connection_ptr) +void *event_thread(void *connection_ptr) { int read_count, path_len; void *buf; From fdc8afd32da6980e33f4b6295c27dd2bb14994c2 Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 15:07:15 +0000 Subject: [PATCH 6/9] transfused: factor read_exactly out of read_message Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 57 ++++++++++++------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index d786f46db..c00ac583a 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -182,47 +182,46 @@ uint64_t message_id(uint64_t *message) return message[1]; } -int read_message(char *descr, parameters *params, int fd, - char *buf, size_t max_read){ +void read_exactly(char *descr, int fd, void *p, size_t nbyte) +{ ssize_t read_count; - size_t upto = 0; - size_t nbyte; - uint32_t len; + char *buf = p; - do { - read_count = read(fd, buf + upto, sizeof(uint32_t) - upto); - if (read_count == -1) { + while (nbyte > 0) { + read_count = read(fd, buf, nbyte); + if (read_count < 0) { if (errno == EAGAIN || errno == EINTR) continue; - die(1, params, "", "read %s: error reading: ", descr); + die(1, NULL, "", "read %s: error reading: ", descr); } if (read_count == 0) - die(1, params, NULL, - "read %s: EOF reading length", descr); - upto += read_count; - } while (upto < sizeof(uint32_t)); + die(1, NULL, NULL, "read %s: EOF reading", descr); + nbyte -= read_count; + buf += read_count; + } +} +int read_message(char *descr, parameters *params, int fd, + char *buf, size_t max_read) +{ + size_t nbyte = sizeof(uint32_t); + uint32_t len; + + read_exactly(descr, fd, buf, nbyte); len = *((uint32_t *) buf); if (len > max_read) die(1, params, NULL, "read %s: message size %d exceeds buffer capacity %d", len, max_read); + if (len < nbyte) + die(1, params, NULL, + "read %s: message size is %d but must be at least %d", + len, nbyte); - nbyte = (size_t)(len - 4); - buf += 4; + buf += nbyte; + nbyte = (size_t)(len - nbyte); - do { - read_count = read(fd, buf, nbyte); - if (read_count < 0) { - if (errno == EAGAIN || errno == EINTR) - continue; - die(1, params, "", "read %s: error reading: ", descr); - } - if (read_count == 0) - die(1, params, NULL, "read %s: EOF reading", descr); - nbyte -= read_count; - buf += read_count; - } while (nbyte != 0); + read_exactly(descr, fd, buf, nbyte); return (int)len; } @@ -302,7 +301,7 @@ void write_exactly(char *descr, int fd, void *p, size_t nbyte) int write_count; char *buf = p; - do { + while (nbyte > 0) { write_count = write(fd, buf, nbyte); if (write_count < 0) { if (errno == EINTR || errno == EAGAIN) @@ -314,7 +313,7 @@ void write_exactly(char *descr, int fd, void *p, size_t nbyte) nbyte -= write_count; buf += write_count; - } while (nbyte != 0); + } } void copy_outof_fuse(copy_thread_state *copy_state) From ac7316427c55a2c14f2d91978a3343b3ccff8fe5 Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 15:10:55 +0000 Subject: [PATCH 7/9] transfused: use read_exactly in init thread message reading Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index c00ac583a..d780a856c 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -937,13 +937,8 @@ void *init_thread(void *params_ptr) buf = must_malloc("incoming control message buffer", CTL_BUFSZ); - /* TODO: handle short read / socket conditions */ - read_count = read(params->ctl_sock, buf, 6); - if (read_count < 0) - die(1, params, "init thread: error reading", ""); /* TODO: handle other messages */ - if (read_count != 6) - die(1, params, NULL, "init thread: response not 6"); + read_exactly("init thread", params->ctl_sock, buf, 6); for (int i = 0; i < sizeof(init_msg); i++) if (((char *)buf)[i] != init_msg[i]) die(1, params, NULL, "init thread: unexpected message"); From 9d5309953fdb59c6629fe5a07cd8b0b8042caf09 Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 15:11:17 +0000 Subject: [PATCH 8/9] transfused: use read_exactly in subproto reading Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index d780a856c..d94fd54ab 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -1085,7 +1085,6 @@ void parse_parameters(int argc, char *argv[], parameters *params) void serve(parameters *params) { - ssize_t read_count; char subproto_selector; pthread_t child; connection_t *conn; @@ -1109,10 +1108,7 @@ void serve(parameters *params) if (conn->sock < 0) die(1, params, "accept", ""); - /* TODO: check for socket read conditions e.g.EAGAIN */ - read_count = read(conn->sock, &subproto_selector, 1); - if (read_count <= 0) - die(1, params, "read subprotocol selector", ""); + read_exactly("subproto", conn->sock, &subproto_selector, 1); switch (subproto_selector) { case 'm': From 58706fd84ee23b07de012d5844cbb63ebb4fd2ef Mon Sep 17 00:00:00 2001 From: David Sheets Date: Wed, 14 Dec 2016 16:20:29 +0000 Subject: [PATCH 9/9] transfused: use memcmp instead of a loop for checking init message Signed-off-by: David Sheets --- alpine/packages/transfused/transfused.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/alpine/packages/transfused/transfused.c b/alpine/packages/transfused/transfused.c index d94fd54ab..be219e131 100644 --- a/alpine/packages/transfused/transfused.c +++ b/alpine/packages/transfused/transfused.c @@ -938,10 +938,9 @@ void *init_thread(void *params_ptr) buf = must_malloc("incoming control message buffer", CTL_BUFSZ); /* TODO: handle other messages */ - read_exactly("init thread", params->ctl_sock, buf, 6); - for (int i = 0; i < sizeof(init_msg); i++) - if (((char *)buf)[i] != init_msg[i]) - die(1, params, NULL, "init thread: unexpected message"); + read_exactly("init thread", params->ctl_sock, buf, sizeof(init_msg)); + if (memcmp(buf, init_msg, sizeof(init_msg))) + die(1, params, NULL, "init thread: unexpected message"); /* we've gotten Continue so write the pidfile */ if (params->pidfile != NULL)