From f7db9fd5e71d3c55f83b0b6142b5c13f2fdef18c Mon Sep 17 00:00:00 2001
From: "Nicholas J. Kain" <nicholas@kain.us>
Date: Wed, 7 Sep 2022 17:08:43 -0400
Subject: [PATCH] Correct regression introduced by previous commit.

safe_recv(..., len), when used on a blocking fd, will attempt
to call recv and collect data until either EOF, a hard error,
or len bytes are collected.

The previous commit used safe_recv() in a blocking mode to read
a single byte into a buffer that was larger than a byte.  This
would cause ndhc to stall as safe_recv() would try to fill that
buffer when no more data would ever be sent.

This issue would only happen if ndhc is supposed to run a script.

Introduce and use safe_recv_once() that will correct this problem and
fill the semantic gap for blocking fds.  I add a new call because in
some cases the above behavior might be required for a blocking fd, too.

Note that the above issue is not a problem for nonblocking fds; the
EAGAIN or EWOULDBLOCK path will return a short read.
---
 nk/io.c   | 26 +-------------------------
 nk/io.h   | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 scriptd.c |  2 +-
 3 files changed, 52 insertions(+), 29 deletions(-)

diff --git a/nk/io.c b/nk/io.c
index 78fb2ae..b0fef9f 100644
--- a/nk/io.c
+++ b/nk/io.c
@@ -1,10 +1,6 @@
-// Copyright 2010-2018 Nicholas J. Kain <njkain at gmail dot com>
+// Copyright 2010-2022 Nicholas J. Kain <njkain at gmail dot com>
 // SPDX-License-Identifier: MIT
-#include <unistd.h>
-#include <sys/types.h>
-#include <errno.h>
 #include "nk/io.h"
-#include <limits.h>
 
 // POSIX says read/write/etc() with len param > SSIZE_MAX is implementation defined.
 // So we avoid implementation-defined behavior with the bounding in each safe_* fn.
@@ -92,23 +88,3 @@ ssize_t safe_recv(int fd, char *buf, size_t len, int flags)
     }
     return (ssize_t)s;
 }
-
-ssize_t safe_recvmsg(int fd, struct msghdr *msg, int flags)
-{
-    ssize_t r;
-    for (;;) {
-        r = recvmsg(fd, msg, flags);
-        if (r >= 0 || errno != EINTR) break;
-    }
-    return r;
-}
-
-int safe_ftruncate(int fd, off_t length)
-{
-    int r;
-    for (;;) {
-        r = ftruncate(fd, length);
-        if (!r || errno != EINTR) break;
-    }
-    return r;
-}
diff --git a/nk/io.h b/nk/io.h
index 874d922..1872bd0 100644
--- a/nk/io.h
+++ b/nk/io.h
@@ -1,17 +1,64 @@
-// Copyright 2010-2015 Nicholas J. Kain <njkain at gmail dot com>
+// Copyright 2010-2022 Nicholas J. Kain <njkain at gmail dot com>
 // SPDX-License-Identifier: MIT
 #ifndef NCM_IO_H_
 #define NCM_IO_H_
 
+#include <unistd.h>
+#include <errno.h>
+#include <limits.h>
 #include <sys/socket.h>
+#include <sys/types.h>
 
 ssize_t safe_read(int fd, char *buf, size_t len);
+// Same as above, but will only call read one time.
+// Meant to be used with a blocking fd where we need <= len bytes.
+static inline ssize_t safe_read_once(int fd, char *buf, size_t len)
+{
+    if (len > SSIZE_MAX) len = SSIZE_MAX;
+    ssize_t r;
+    for (;;) {
+        r = read(fd, buf, len);
+        if (r >= 0 || errno != EINTR) break;
+    }
+    return r;
+}
+
 ssize_t safe_write(int fd, const char *buf, size_t len);
 ssize_t safe_sendto(int fd, const char *buf, size_t len, int flags,
                     const struct sockaddr *dest_addr, socklen_t addrlen);
+
 ssize_t safe_recv(int fd, char *buf, size_t len, int flags);
-ssize_t safe_recvmsg(int fd, struct msghdr *msg, int flags);
-int safe_ftruncate(int fd, off_t length);
+// Same as above, but will only call read one time.
+// Meant to be used with a blocking fd where we need <= len bytes.
+static inline ssize_t safe_recv_once(int fd, char *buf, size_t len, int flags)
+{
+    if (len > SSIZE_MAX) len = SSIZE_MAX;
+    ssize_t r;
+    for (;;) {
+        r = recv(fd, buf, len, flags);
+        if (r >= 0 || errno != EINTR) break;
+    }
+    return r;
+}
+
+static inline ssize_t safe_recvmsg(int fd, struct msghdr *msg, int flags)
+{
+    ssize_t r;
+    for (;;) {
+        r = recvmsg(fd, msg, flags);
+        if (r >= 0 || errno != EINTR) break;
+    }
+    return r;
+}
+static inline int safe_ftruncate(int fd, off_t length)
+{
+    int r;
+    for (;;) {
+        r = ftruncate(fd, length);
+        if (!r || errno != EINTR) break;
+    }
+    return r;
+}
 
 #endif
 
diff --git a/scriptd.c b/scriptd.c
index c746fea..cdfa717 100644
--- a/scriptd.c
+++ b/scriptd.c
@@ -32,7 +32,7 @@ void request_scriptd_run(void)
         suicide("%s: (%s) write failed: %zd", client_config.interface,
                 __func__, r);
     char buf[16];
-    r = safe_recv(scriptdSock[0], buf, sizeof buf, 0);
+    r = safe_recv_once(scriptdSock[0], buf, sizeof buf, 0);
     if (r == 0) {
         // Remote end hung up.
         exit(EXIT_SUCCESS);