From 1f47e27a290f0cfbf55caad1573cc1b857f67af3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 14 Feb 2024 10:55:22 +0100 Subject: [PATCH 1/2] journald: when getting journal data via memfd, check flags are valid Add some extra safety checks: refuse weird open flags. And while we are at it, also use stat_verify_regular() --- src/journal/journald-native.c | 49 +++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c index 4f030dda86..315ec0b513 100644 --- a/src/journal/journald-native.c +++ b/src/journal/journald-native.c @@ -22,6 +22,7 @@ #include "journald-wall.h" #include "memfd-util.h" #include "memory-util.h" +#include "missing_fcntl.h" #include "parse-util.h" #include "path-util.h" #include "process-util.h" @@ -342,8 +343,33 @@ void server_process_native_file( assert(s); assert(fd >= 0); - /* 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. */ + if (fstat(fd, &st) < 0) { + log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT, "Failed to stat passed file, ignoring: %m"); + return; + } + + 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 (st.st_size <= 0) + return; + + int flags = fcntl(fd, F_GETFL); + if (flags < 0) { + log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT, "Failed to get flags of passed file, ignoring: %m"); + return; + } + + if ((flags & ~(O_ACCMODE|RAW_O_LARGEFILE)) != 0) { + log_ratelimit_error(JOURNAL_LOG_RATELIMIT, "Unexpected flags of passed memory fd, ignoring message: %m"); + return; + } + + /* 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. */ sealed = memfd_get_sealed(fd) > 0; if (!sealed && (!ucred || ucred->uid != 0)) { @@ -374,23 +400,8 @@ void server_process_native_file( } } - if (fstat(fd, &st) < 0) { - log_ratelimit_error_errno(errno, JOURNAL_LOG_RATELIMIT, - "Failed to stat passed file, ignoring: %m"); - return; - } - - if (!S_ISREG(st.st_mode)) { - log_ratelimit_error(JOURNAL_LOG_RATELIMIT, - "File passed is not regular. Ignoring."); - return; - } - - if (st.st_size <= 0) - return; - - /* When !sealed, set a lower memory limit. We have to read the file, - * effectively doubling memory use. */ + /* 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.", From 296de3867c74ca13d491cfb084b13bb7602858a3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 14 Feb 2024 10:56:09 +0100 Subject: [PATCH 2/2] journald: rebreak some comments --- src/journal/journald-native.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c index 315ec0b513..75d9082abf 100644 --- a/src/journal/journald-native.c +++ b/src/journal/journald-native.c @@ -376,8 +376,8 @@ void server_process_native_file( _cleanup_free_ char *k = NULL; const char *e; - /* If this is not a sealed memfd, and the peer is unknown or - * unprivileged, then verify the path. */ + /* If this is not a sealed memfd, and the peer is unknown or unprivileged, then verify the + * path. */ r = fd_get_path(fd, &k); if (r < 0) { @@ -437,8 +437,7 @@ void server_process_native_file( return; } - /* Refuse operating on file systems that have - * mandatory locking enabled, see: + /* Refuse operating on file systems that have mandatory locking enabled, see: * * https://github.com/systemd/systemd/issues/1822 */ @@ -448,13 +447,10 @@ void server_process_native_file( return; } - /* 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. */ + /* 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, @@ -462,9 +458,8 @@ void server_process_native_file( return; } - /* 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) {