diff --git a/common/ecdh_hsmd.c b/common/ecdh_hsmd.c index 68be59841..c55a4a60d 100644 --- a/common/ecdh_hsmd.c +++ b/common/ecdh_hsmd.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -13,16 +14,22 @@ static void (*stashed_failed)(enum status_failreason, const char *fmt, ...); void ecdh(const struct pubkey *point, struct secret *ss) { const u8 *msg = towire_hsmd_ecdh_req(NULL, point); + u8 *resp; assert(stashed_hsm_fd >= 0); assert(stashed_failed != NULL); + /* wire_sync_read/write tolerate a non-blocking fd (the HSM socketpair + * can be O_NONBLOCK on macOS), so no blocking toggling is needed here. + * Report errno so a failure is diagnosable from the daemon log. */ if (!wire_sync_write(stashed_hsm_fd, take(msg))) - stashed_failed(STATUS_FAIL_HSM_IO, "Write ECDH to hsmd failed"); + stashed_failed(STATUS_FAIL_HSM_IO, "Write ECDH to hsmd failed: %s", + strerror(errno)); - msg = wire_sync_read(tmpctx, stashed_hsm_fd); - if (!msg) - stashed_failed(STATUS_FAIL_HSM_IO, "No hsmd ECDH response"); + resp = wire_sync_read(tmpctx, stashed_hsm_fd); + if (!resp) + stashed_failed(STATUS_FAIL_HSM_IO, "No hsmd ECDH response: %s", + strerror(errno)); if (!fromwire_hsmd_ecdh_resp(msg, ss)) stashed_failed(STATUS_FAIL_HSM_IO, "Invalid hsmd ECDH response"); @@ -34,6 +41,4 @@ void ecdh_hsmd_setup(int hsm_fd, { stashed_hsm_fd = hsm_fd; stashed_failed = failed; - /* Like read_fds in subd.c: don't trust sender's O_NONBLOCK state (issue #9060). */ - io_fd_block(hsm_fd, true); } diff --git a/wire/wire_sync.c b/wire/wire_sync.c index 954515136..dfc3d592d 100644 --- a/wire/wire_sync.c +++ b/wire/wire_sync.c @@ -1,19 +1,83 @@ #include "config.h" #include -#include #include #include +#include #include #include +/* Wait until fd is ready (readable or writable), tolerating EINTR. */ +static bool wait_fd(int fd, short events) +{ + int r; + + do { + r = poll(&(struct pollfd){.fd = fd, .events = events}, 1, -1); + } while (r < 0 && errno == EINTR); + + return r > 0; +} + +/* Like read_all, but tolerates an fd with O_NONBLOCK set: on EAGAIN we poll + * for readability and resume, preserving any partial read. Subdaemon HSM fds + * can be O_NONBLOCK on macOS (the flag follows the shared open file + * description as the socketpair is sent via SCM_RIGHTS), so without this we + * would spuriously fail with EAGAIN. */ +static bool read_all_tolerant(int fd, void *buf, size_t size) +{ + while (size) { + ssize_t done = read(fd, buf, size); + if (done < 0) { + if (errno == EINTR) + continue; + if (errno == EAGAIN || errno == EWOULDBLOCK) { + if (!wait_fd(fd, POLLIN)) + return false; + continue; + } + return false; + } + if (done == 0) + return false; + buf = (char *)buf + done; + size -= done; + } + + return true; +} + +/* Mirror of read_all_tolerant for writes. */ +static bool write_all_tolerant(int fd, const void *buf, size_t size) +{ + while (size) { + ssize_t done = write(fd, buf, size); + if (done < 0) { + if (errno == EINTR) + continue; + if (errno == EAGAIN || errno == EWOULDBLOCK) { + if (!wait_fd(fd, POLLOUT)) + return false; + continue; + } + return false; + } + if (done == 0) + return false; + buf = (const char *)buf + done; + size -= done; + } + + return true; +} + bool wire_sync_write(int fd, const void *msg TAKES) { wire_len_t hdr = cpu_to_wirelen(tal_bytelen(msg)); bool ret; assert(tal_bytelen(msg) < WIRE_LEN_LIMIT); - ret = write_all(fd, &hdr, sizeof(hdr)) - && write_all(fd, msg, tal_count(msg)); + ret = write_all_tolerant(fd, &hdr, sizeof(hdr)) + && write_all_tolerant(fd, msg, tal_count(msg)); tal_free_if_taken(msg); return ret; @@ -24,14 +88,14 @@ u8 *wire_sync_read(const tal_t *ctx, int fd) wire_len_t len; u8 *msg; - if (!read_all(fd, &len, sizeof(len))) + if (!read_all_tolerant(fd, &len, sizeof(len))) return NULL; if (wirelen_to_cpu(len) >= WIRE_LEN_LIMIT) { errno = E2BIG; return NULL; } msg = tal_arr(ctx, u8, wirelen_to_cpu(len)); - if (!read_all(fd, msg, wirelen_to_cpu(len))) + if (!read_all_tolerant(fd, msg, wirelen_to_cpu(len))) return tal_free(msg); return msg; }