]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
journal-native: ignore server_process_native_file error on caller's side 31425/head
authorMike Yuan <me@yhndnzj.com>
Wed, 21 Feb 2024 06:03:55 +0000 (14:03 +0800)
committerMike Yuan <me@yhndnzj.com>
Wed, 21 Feb 2024 22:17:54 +0000 (06:17 +0800)
Also, stop saying ", ignoring". It is unclear whether the message
or the error is ignored. "ignoring message" or "refusing" is OK.

src/journal/fuzz-journald-native-fd.c
src/journal/journald-native.c
src/journal/journald-native.h
src/journal/journald-server.c

index 26395dfbcb4ecc16d8780a1310ecb47e357c5d3e..07a8eb55e18f86b3c39f88793e4c9cfab8dc37e3 100644 (file)
@@ -30,13 +30,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
                 .uid = geteuid(),
                 .gid = getegid(),
         };
-        server_process_native_file(s, sealed_fd, &ucred, tv, label, label_len);
+        (void) server_process_native_file(s, sealed_fd, &ucred, tv, label, label_len);
 
         unsealed_fd = mkostemp_safe(name);
         assert_se(unsealed_fd >= 0);
         assert_se(write(unsealed_fd, data, size) == (ssize_t) size);
         assert_se(lseek(unsealed_fd, 0, SEEK_SET) == 0);
-        server_process_native_file(s, unsealed_fd, &ucred, tv, label, label_len);
+        (void) server_process_native_file(s, unsealed_fd, &ucred, tv, label, label_len);
 
         return 0;
 }
index 579a03811b9f6f463422432b3257cb1ba08c7a39..d6bceb0e4a2d4e789239509e270c7679b94e58f8 100644 (file)
@@ -326,7 +326,7 @@ void server_process_native_message(
         } while (r == 0);
 }
 
-void server_process_native_file(
+int server_process_native_file(
                 Server *s,
                 int fd,
                 const struct ucred *ucred,
@@ -342,27 +342,25 @@ void server_process_native_file(
         assert(s);
         assert(fd >= 0);
 
-        if (fstat(fd, &st) < 0) {
-                log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT, "Failed to stat passed file, ignoring: %m");
-                return;
-        }
+        if (fstat(fd, &st) < 0)
+                return log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
+                                                 "Failed to stat passed file: %m");
 
         r = stat_verify_regular(&st);
-        if (r < 0) {
-                log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT, "File passed is not regular, ignoring: %m");
-                return;
-        }
+        if (r < 0)
+                return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
+                                                 "File passed is not regular, ignoring message: %m");
 
         if (st.st_size <= 0)
-                return;
+                return 0;
 
         r = fd_verify_safe_flags(fd);
         if (r == -EREMOTEIO)
-                return (void) log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
-                                                        "Unexpected flags of passed memory fd, ignoring message.");
+                return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
+                                                 "Unexpected flags of passed memory fd, ignoring message.");
         if (r < 0)
-                return (void) log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
-                                                        "Failed to get flags of passed file: %m");
+                return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
+                                                 "Failed to get flags of passed file: %m");
 
         /* If it's a memfd, check if it is sealed. If so, we can just mmap it and use it, and do not need to
          * copy the data out. */
@@ -376,34 +374,26 @@ void server_process_native_file(
                  * path. */
 
                 r = fd_get_path(fd, &k);
-                if (r < 0) {
-                        log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
-                                                  "readlink(/proc/self/fd/%i) failed: %m", fd);
-                        return;
-                }
+                if (r < 0)
+                        return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
+                                                         "Failed to get path of passed fd: %m");
 
                 e = PATH_STARTSWITH_SET(k, "/dev/shm/", "/tmp/", "/var/tmp/");
-                if (!e) {
-                        log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
-                                            "Received file outside of allowed directories. Refusing.");
-                        return;
-                }
+                if (!e)
+                        return log_ratelimit_error_errno(SYNTHETIC_ERRNO(EPERM), JOURNAL_LOG_RATELIMIT,
+                                                         "Received file outside of allowed directories, refusing.");
 
-                if (!filename_is_valid(e)) {
-                        log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
-                                            "Received file in subdirectory of allowed directories. Refusing.");
-                        return;
-                }
+                if (!filename_is_valid(e))
+                        return log_ratelimit_error_errno(SYNTHETIC_ERRNO(EPERM), JOURNAL_LOG_RATELIMIT,
+                                                         "Received file in subdirectory of allowed directories, refusing.");
         }
 
         /* When !sealed, set a lower memory limit. We have to read the file, effectively doubling memory
          * use. */
-        if (st.st_size > ENTRY_SIZE_MAX / (sealed ? 1 : 2)) {
-                log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
-                                    "File passed too large (%"PRIu64" bytes). Ignoring.",
-                                    (uint64_t) st.st_size);
-                return;
-        }
+        if (st.st_size > ENTRY_SIZE_MAX / (sealed ? 1 : 2))
+                return log_ratelimit_error_errno(SYNTHETIC_ERRNO(EFBIG), JOURNAL_LOG_RATELIMIT,
+                                                 "File passed too large (%"PRIu64" bytes), refusing.",
+                                                 (uint64_t) st.st_size);
 
         if (sealed) {
                 void *p;
@@ -414,62 +404,54 @@ void server_process_native_file(
                 ps = PAGE_ALIGN(st.st_size);
                 assert(ps < SIZE_MAX);
                 p = mmap(NULL, ps, PROT_READ, MAP_PRIVATE, fd, 0);
-                if (p == MAP_FAILED) {
-                        log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
-                                                  "Failed to map memfd, ignoring: %m");
-                        return;
-                }
+                if (p == MAP_FAILED)
+                        return log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
+                                                         "Failed to map memfd: %m");
 
                 server_process_native_message(s, p, st.st_size, ucred, tv, label, label_len);
                 assert_se(munmap(p, ps) >= 0);
-        } else {
-                _cleanup_free_ void *p = NULL;
-                struct statvfs vfs;
-                ssize_t n;
-
-                if (fstatvfs(fd, &vfs) < 0) {
-                        log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
-                                                  "Failed to stat file system of passed file, not processing it: %m");
-                        return;
-                }
 
-                /* Refuse operating on file systems that have mandatory locking enabled, see:
-                 *
-                 * https://github.com/systemd/systemd/issues/1822
-                 */
-                if (vfs.f_flag & ST_MANDLOCK) {
-                        log_ratelimit_error(JOURNAL_LOG_RATELIMIT,
-                                            "Received file descriptor from file system with mandatory locking enabled, not processing it.");
-                        return;
-                }
+                return 0;
+        }
 
-                /* Make the fd non-blocking. On regular files this has the effect of bypassing mandatory
-                 * locking. Of course, this should normally not be necessary given the check above, but let's
-                 * better be safe than sorry, after all NFS is pretty confusing regarding file system flags,
-                 * and we better don't trust it, and so is SMB. */
-                r = fd_nonblock(fd, true);
-                if (r < 0) {
-                        log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
-                                                  "Failed to make fd non-blocking, not processing it: %m");
-                        return;
-                }
+        _cleanup_free_ void *p = NULL;
+        struct statvfs vfs;
+        ssize_t n;
+
+        if (fstatvfs(fd, &vfs) < 0)
+                return log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
+                                                 "Failed to stat file system of passed file: %m");
+
+        /* Refuse operating on file systems that have mandatory locking enabled.
+         * See also: https://github.com/systemd/systemd/issues/1822 */
+        if (FLAGS_SET(vfs.f_flag, ST_MANDLOCK))
+                return log_ratelimit_error_errno(SYNTHETIC_ERRNO(EPERM), JOURNAL_LOG_RATELIMIT,
+                                                 "Received file descriptor from file system with mandatory locking enabled, not processing it.");
+
+        /* Make the fd non-blocking. On regular files this has the effect of bypassing mandatory
+         * locking. Of course, this should normally not be necessary given the check above, but let's
+         * better be safe than sorry, after all NFS is pretty confusing regarding file system flags,
+         * and we better don't trust it, and so is SMB. */
+        r = fd_nonblock(fd, true);
+        if (r < 0)
+                return log_ratelimit_error_errno(r, JOURNAL_LOG_RATELIMIT,
+                                                 "Failed to make fd non-blocking: %m");
 
-                /* The file is not sealed, we can't map the file here, since clients might then truncate it
-                 * and trigger a SIGBUS for us. So let's stupidly read it. */
+        /* The file is not sealed, we can't map the file here, since clients might then truncate it
+         * and trigger a SIGBUS for us. So let's stupidly read it. */
 
-                p = malloc(st.st_size);
-                if (!p) {
-                        log_oom();
-                        return;
-                }
+        p = malloc(st.st_size);
+        if (!p)
+                return log_oom();
 
-                n = pread(fd, p, st.st_size, 0);
-                if (n < 0)
-                        log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
-                                                  "Failed to read file, ignoring: %m");
-                else if (n > 0)
-                        server_process_native_message(s, p, n, ucred, tv, label, label_len);
-        }
+        n = pread(fd, p, st.st_size, 0);
+        if (n < 0)
+                return log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT,
+                                                 "Failed to read file: %m");
+        if (n > 0)
+                server_process_native_message(s, p, n, ucred, tv, label, label_len);
+
+        return 0;
 }
 
 int server_open_native_socket(Server *s, const char *native_socket) {
index 7bbaaed181886debbc69643dceed6b8b10d9b5c7..10db26796692b9306db8ba470306281ea5d43673 100644 (file)
@@ -12,7 +12,7 @@ void server_process_native_message(
                 const char *label,
                 size_t label_len);
 
-void server_process_native_file(
+int server_process_native_file(
                 Server *s,
                 int fd,
                 const struct ucred *ucred,
index a8c186dc2094fad53b69ee8b282d8e67527d69ae..2c6f6e301df68166e9c4dc9ea7edc9eaf8ff129a 100644 (file)
@@ -1538,7 +1538,7 @@ int server_process_datagram(
                 if (n > 0 && n_fds == 0)
                         server_process_native_message(s, s->buffer, n, ucred, tv, label, label_len);
                 else if (n == 0 && n_fds == 1)
-                        server_process_native_file(s, fds[0], ucred, tv, label, label_len);
+                        (void) server_process_native_file(s, fds[0], ucred, tv, label, label_len);
                 else if (n_fds > 0)
                         log_ratelimit_warning(JOURNAL_LOG_RATELIMIT,
                                               "Got too many file descriptors via native socket. Ignoring.");