treewide: sanitize loop_write

loop_write() didn't follow the usual systemd rules and returned status
partially in errno and required extensive checks from callers. Some of
the callers dealt with this properly, but many did not, treating
partial writes as successful. Simplify things by conforming to usual rules.

diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 3416802bcb4d69acc7adad9aa03e6737008646e3..3470ca1768cd2a28d15bc4418b0313e463f919dd 100644 (file)
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -42,13 +42,13 @@
 #define IMA_POLICY_PATH "/etc/ima/ima-policy"
 
 int ima_setup(void) {
+        int r = 0;
 
 #ifdef HAVE_IMA
         struct stat st;
-        ssize_t policy_size = 0, written = 0;
+        ssize_t policy_size = 0;
         char *policy;
         _cleanup_close_ int policyfd = -1, imafd = -1;
-        int result = 0;
 
         if (stat(IMA_POLICY_PATH, &st) < 0)
                 return 0;
@@ -81,13 +81,13 @@ int ima_setup(void) {
         policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0);
         if (policy == MAP_FAILED) {
                 log_error_errno(errno, "mmap() failed (%m), freezing");
-                result = -errno;
+                r = -errno;
                 goto out;
         }
 
-        written = loop_write(imafd, policy, (size_t)policy_size, false);
-        if (written != policy_size) {
-                log_error_errno(errno, "Failed to load the IMA custom policy file %s (%m), ignoring.",
+        r = loop_write(imafd, policy, (size_t)policy_size, false);
+        if (r < 0) {
+                log_error_errno(r, "Failed to load the IMA custom policy file %s (%m), ignoring.",
                                 IMA_POLICY_PATH);
                 goto out_mmap;
         }
@@ -97,9 +97,6 @@ int ima_setup(void) {
 out_mmap:
         munmap(policy, policy_size);
 out:
-        if (result)
-                 return result;
 #endif /* HAVE_IMA */
-
-        return 0;
+        return r;
 }

diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index 74582a5dcd0be2121778197e6301bc0f922fc4b7..d91a02cf15b9689d82187393b82f416e79455bb3 100644 (file)
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -182,7 +182,7 @@ static int write_machine_id(int fd, char id[34]) {
         assert(id);
         lseek(fd, 0, SEEK_SET);
 
-        if (loop_write(fd, id, 33, false) == 33)
+        if (loop_write(fd, id, 33, false) == 0)
                 return 0;
 
         return -errno;
@@ -329,10 +329,9 @@ int machine_id_setup(const char *root) {
         if (r < 0)
                 return r;
 
-        if (S_ISREG(st.st_mode) && writable) {
+        if (S_ISREG(st.st_mode) && writable)
                 if (write_machine_id(fd, id) == 0)
                         return 0;
-        }
 
         fd = safe_close(fd);
 

diff --git a/src/journal/compress.c b/src/journal/compress.c
index c4c715be2f5ad9d254efc3be27418dee65ae1056..9440fcd60ef4638b4aa93d5ba591cdb6eaabe756 100644 (file)
--- a/src/journal/compress.c
+++ b/src/journal/compress.c
@@ -400,12 +400,9 @@ int compress_stream_xz(int fdf, int fdt, off_t max_bytes) {
 
                         n = sizeof(out) - s.avail_out;
 
-                        errno = 0;
                         k = loop_write(fdt, out, n, false);
                         if (k < 0)
                                 return k;
-                        if (k != n)
-                                return errno ? -errno : -EIO;
 
                         if (ret == LZMA_STREAM_END) {
                                 log_debug("XZ compression finished (%"PRIu64" -> %"PRIu64" bytes, %.1f%%)",
@@ -478,8 +475,6 @@ int compress_stream_lz4(int fdf, int fdt, off_t max_bytes) {
                 n = loop_write(fdt, out, r, false);
                 if (n < 0)
                         return n;
-                if (n != r)
-                        return errno ? -errno : -EIO;
 
                 total_out += sizeof(header) + r;
 
@@ -559,12 +554,9 @@ int decompress_stream_xz(int fdf, int fdt, off_t max_bytes) {
                                 max_bytes -= n;
                         }
 
-                        errno = 0;
                         k = loop_write(fdt, out, n, false);
                         if (k < 0)
                                 return k;
-                        if (k != n)
-                                return errno ? -errno : -EIO;
 
                         if (ret == LZMA_STREAM_END) {
                                 log_debug("XZ decompression finished (%"PRIu64" -> %"PRIu64" bytes, %.1f%%)",
@@ -645,12 +637,9 @@ int decompress_stream_lz4(int fdf, int fdt, off_t max_bytes) {
                         return -EFBIG;
                 }
 
-                errno = 0;
                 n = loop_write(fdt, out, r, false);
                 if (n < 0)
                         return n;
-                if (n != r)
-                        return errno ? -errno : -EIO;
         }
 
         log_debug("LZ4 decompression finished (%zu -> %zu bytes, %.1f%%)",

diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c
index 887b957c4d3751c14287f374b679ecb6903226b1..56a96c55dd0104ff315bc02161e7447d2527421a 100644 (file)
--- a/src/journal/journal-send.c
+++ b/src/journal/journal-send.c
@@ -453,13 +453,10 @@ _public_ int sd_journal_stream_fd(const char *identifier, int priority, int leve
         header[l++] = '0';
         header[l++] = '\n';
 
-        r = (int) loop_write(fd, header, l, false);
+        r = loop_write(fd, header, l, false);
         if (r < 0)
                 return r;
 
-        if ((size_t) r != l)
-                return -errno;
-
         r = fd;
         fd = -1;
         return r;

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 3cec9a0c84e936e8d720873bf78983cdb76d3704..b2f6966fcabb63e40f4b7c44c7e2778f91e3b629 100644 (file)
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -1406,17 +1406,15 @@ static int setup_keys(void) {
         h.fsprg_secpar = htole16(FSPRG_RECOMMENDED_SECPAR);
         h.fsprg_state_size = htole64(state_size);
 
-        l = loop_write(fd, &h, sizeof(h), false);
-        if (l < 0 || (size_t) l != sizeof(h)) {
-                log_error_errno(EIO, "Failed to write header: %m");
-                r = -EIO;
+        r = loop_write(fd, &h, sizeof(h), false);
+        if (r < 0) {
+                log_error_errno(r, "Failed to write header: %m");
                 goto finish;
         }
 
-        l = loop_write(fd, state, state_size, false);
-        if (l < 0 || (size_t) l != state_size) {
-                log_error_errno(EIO, "Failed to write state: %m");
-                r = -EIO;
+        r = loop_write(fd, state, state_size, false);
+        if (r < 0) {
+                log_error_errno(r, "Failed to write state: %m");
                 goto finish;
         }
 

diff --git a/src/libsystemd-terminal/subterm.c b/src/libsystemd-terminal/subterm.c
index 75a25e5590e3ada98ee3e0ca23784fa6efe9e106..78efc9d7c0f6d7e31f1636cbf7a2effcdee95d2a 100644 (file)
--- a/src/libsystemd-terminal/subterm.c
+++ b/src/libsystemd-terminal/subterm.c
@@ -117,14 +117,14 @@ static int output_winch(Output *o) {
 }
 
 static int output_flush(Output *o) {
-        ssize_t len;
+        int r;
 
         if (o->n_obuf < 1)
                 return 0;
 
-        len = loop_write(o->fd, o->obuf, o->n_obuf, false);
-        if (len < 0)
-                return log_error_errno(len, "error: cannot write to TTY (%zd): %m", len);
+        r = loop_write(o->fd, o->obuf, o->n_obuf, false);
+        if (r < 0)
+                return log_error_errno(r, "error: cannot write to TTY: %m");
 
         o->n_obuf = 0;
 

diff --git a/src/random-seed/random-seed.c b/src/random-seed/random-seed.c
index 40eaaf46d90fed860e3faf1ec9fbb3167cf8adeb..06c12396017843629ffcc100586f9122f9f3bd93 100644 (file)
--- a/src/random-seed/random-seed.c
+++ b/src/random-seed/random-seed.c
@@ -113,12 +113,9 @@ int main(int argc, char *argv[]) {
                 } else {
                         lseek(seed_fd, 0, SEEK_SET);
 
-                        k = loop_write(random_fd, buf, (size_t) k, false);
-                        if (k <= 0) {
-                                log_error("Failed to write seed to /dev/urandom: %s", r < 0 ? strerror(-r) : "short write");
-
-                                r = k == 0 ? -EIO : (int) k;
-                        }
+                        r = loop_write(random_fd, buf, (size_t) k, false);
+                        if (r < 0)
+                                log_error_errno(r, "Failed to write seed to /dev/urandom: %m");
                 }
 
         } else if (streq(argv[1], "save")) {
@@ -155,10 +152,8 @@ int main(int argc, char *argv[]) {
                 r = k == 0 ? -EIO : (int) k;
         } else {
                 r = loop_write(seed_fd, buf, (size_t) k, false);
-                if (r <= 0) {
-                        log_error("Failed to write new random seed file: %s", r < 0 ? strerror(-r) : "short write");
-                        r = r == 0 ? -EIO : r;
-                }
+                if (r < 0)
+                        log_error_errno(r, "Failed to write new random seed file: %m");
         }
 
 finish:

diff --git a/src/shared/copy.c b/src/shared/copy.c
index abb7fbc52b7bdbda65bd3defeb1331770ef7ba0c..b8b1ba18664da6d615a5e98e831dbd505211adb4 100644 (file)
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -63,7 +63,7 @@ int copy_bytes(int fdf, int fdt, off_t max_bytes) {
                 /* As a fallback just copy bits by hand */
                 {
                         char buf[m];
-                        ssize_t k;
+                        int r;
 
                         n = read(fdf, buf, m);
                         if (n < 0)
@@ -71,12 +71,9 @@ int copy_bytes(int fdf, int fdt, off_t max_bytes) {
                         if (n == 0) /* EOF */
                                 break;
 
-                        errno = 0;
-                        k = loop_write(fdt, buf, n, false);
-                        if (k < 0)
-                                return k;
-                        if (k != n)
-                                return errno ? -errno : -EIO;
+                        r = loop_write(fdt, buf, n, false);
+                        if (r < 0)
+                                return r;
 
                 }
 

diff --git a/src/shared/util.c b/src/shared/util.c
index ff8835b72ddf7944d150ad83ef1a8d7c2be65e1e..26a4f72b4310f46bd85e4a8652c26e19a8a71203 100644 (file)
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -2292,13 +2292,15 @@ ssize_t loop_read(int fd, void *buf, size_t nbytes, bool do_poll) {
         return n;
 }
 
-ssize_t loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
+int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
         const uint8_t *p = buf;
         ssize_t n = 0;
 
         assert(fd >= 0);
         assert(buf);
 
+        errno = 0;
+
         while (nbytes > 0) {
                 ssize_t k;
 
@@ -2317,14 +2319,15 @@ ssize_t loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
                 }
 
                 if (k <= 0)
-                        return n > 0 ? n : (k < 0 ? -errno : 0);
+                        /* We were not done yet, and a write error occured. */
+                        return errno ? -errno : -EIO;
 
                 p += k;
                 nbytes -= k;
                 n += k;
         }
 
-        return n;
+        return 0;
 }
 
 int parse_size(const char *t, off_t base, off_t *size) {

diff --git a/src/shared/util.h b/src/shared/util.h
index 61094cca2fbb4f45d544178a7d4c0cf41eb616a5..73bd9012fd6d1b15358b923961be136337c4a0ed 100644 (file)
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -425,7 +425,7 @@ int sigaction_many(const struct sigaction *sa, ...);
 int fopen_temporary(const char *path, FILE **_f, char **_temp_path);
 
 ssize_t loop_read(int fd, void *buf, size_t nbytes, bool do_poll);
-ssize_t loop_write(int fd, const void *buf, size_t nbytes, bool do_poll);
+int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll);
 
 bool is_device_path(const char *path);
 

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index d356686f78e482c23e3717c47d040e8212a4d0dd..6e48671ee61ce277c40db5b1bc05b61db03abdf9 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -7246,12 +7246,9 @@ static int talk_initctl(void) {
                 return -errno;
         }
 
-        errno = 0;
-        r = loop_write(fd, &request, sizeof(request), false) != sizeof(request);
-        if (r) {
-                log_error_errno(errno, "Failed to write to "INIT_FIFO": %m");
-                return errno > 0 ? -errno : -EIO;
-        }
+        r = loop_write(fd, &request, sizeof(request), false);
+        if (r < 0)
+                return log_error_errno(r, "Failed to write to "INIT_FIFO": %m");
 
         return 1;
 }

diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
index fa8448cfaec51128ec54b7e56d899b91068e00e1..5fc27f9ae54dd50ecda5b9ba744a98fa8420ed2c 100644 (file)
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -103,9 +103,9 @@ static int ask_password_plymouth(
         if (!packet)
                 return log_oom();
 
-        k = loop_write(fd, packet, n + 1, true);
-        if (k != n + 1)
-                return k < 0 ? (int) k : -EIO;
+        r = loop_write(fd, packet, n + 1, true);
+        if (r < 0)
+                return r;
 
         pollfd[POLL_SOCKET].fd = fd;
         pollfd[POLL_SOCKET].events = POLLIN;
@@ -165,9 +165,9 @@ static int ask_password_plymouth(
                                 if (asprintf(&packet, "*\002%c%s%n", (int) (strlen(message) + 1), message, &n) < 0)
                                         return -ENOMEM;
 
-                                k = loop_write(fd, packet, n+1, true);
-                                if (k != n + 1)
-                                        return k < 0 ? (int) k : -EIO;
+                                r = loop_write(fd, packet, n+1, true);
+                                if (r < 0)
+                                        return r;
 
                                 accept_cached = false;
                                 p = 0;

diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c
index b7a536b983a46ff3666323022d643efc40fc1866..28371711b6badd2404dd1d6221d0bdfda4027538 100644 (file)
--- a/src/vconsole/vconsole-setup.c
+++ b/src/vconsole/vconsole-setup.c
@@ -54,8 +54,9 @@ static int disable_utf8(int fd) {
         if (ioctl(fd, KDSKBMODE, K_XLATE) < 0)
                 r = -errno;
 
-        if (loop_write(fd, "\033%@", 3, false) < 0)
-                r = -errno;
+        k = loop_write(fd, "\033%@", 3, false);
+        if (k < 0)
+                r = k;
 
         k = write_string_file("/sys/module/vt/parameters/default_utf8", "0");
         if (k < 0)
@@ -86,8 +87,9 @@ static int enable_utf8(int fd) {
                         r = -errno;
         }
 
-        if (loop_write(fd, "\033%G", 3, false) < 0)
-                r = -errno;
+        k = loop_write(fd, "\033%G", 3, false);
+        if (k < 0)
+                r = k;
 
         k = write_string_file("/sys/module/vt/parameters/default_utf8", "1");
         if (k < 0)