From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 27 Feb 2024 16:50:45 +0000 (+0100)
Subject: fd-util: beef up fd_verify_safe_flags() features
X-Git-Tag: v256-rc1~576
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9f65355b85c59cea585ac3dce2305a1461c283a4;p=thirdparty%2Fsystemd.git

fd-util: beef up fd_verify_safe_flags() features

Let's make fd_verify_safe_flags() even more useful:

1. let's return the cleaned up flags (i.e. just the access mode) after
   validation, hiding all the noise, such as O_NOFOLLOW, O_LARGEFILE and
   similar.

2. let's add a "full" version of the call that allows passing additional
   flags that are OK to be set.
---

diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c
index c16a2ab658e..8372c54918d 100644
--- a/src/basic/fd-util.c
+++ b/src/basic/fd-util.c
@@ -913,21 +913,21 @@ int fd_is_opath(int fd) {
         return FLAGS_SET(r, O_PATH);
 }
 
-int fd_verify_safe_flags(int fd) {
+int fd_verify_safe_flags_full(int fd, int extra_flags) {
         int flags, unexpected_flags;
 
         /* Check if an extrinsic fd is safe to work on (by a privileged service). This ensures that clients
          * can't trick a privileged service into giving access to a file the client doesn't already have
          * access to (especially via something like O_PATH).
          *
-         * O_NOFOLLOW: For some reason the kernel will return this flag from fcntl; it doesn't go away
+         * O_NOFOLLOW: For some reason the kernel will return this flag from fcntl(); it doesn't go away
          *             immediately after open(). It should have no effect whatsoever to an already-opened FD,
          *             and since we refuse O_PATH it should be safe.
          *
          * RAW_O_LARGEFILE: glibc secretly sets this and neglects to hide it from us if we call fcntl.
          *                  See comment in missing_fcntl.h for more details about this.
          *
-         * O_DIRECTORY: this is set for directories, which are totally fine
+         * If 'extra_flags' is specified as non-zero the included flags are also allowed.
          */
 
         assert(fd >= 0);
@@ -936,13 +936,13 @@ int fd_verify_safe_flags(int fd) {
         if (flags < 0)
                 return -errno;
 
-        unexpected_flags = flags & ~(O_ACCMODE|O_NOFOLLOW|RAW_O_LARGEFILE|O_DIRECTORY);
+        unexpected_flags = flags & ~(O_ACCMODE|O_NOFOLLOW|RAW_O_LARGEFILE|extra_flags);
         if (unexpected_flags != 0)
                 return log_debug_errno(SYNTHETIC_ERRNO(EREMOTEIO),
                                        "Unexpected flags set for extrinsic fd: 0%o",
                                        (unsigned) unexpected_flags);
 
-        return 0;
+        return flags & (O_ACCMODE | extra_flags); /* return the flags variable, but remove the noise */
 }
 
 int read_nr_open(void) {
diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h
index f549831090a..1e591085ef8 100644
--- a/src/basic/fd-util.h
+++ b/src/basic/fd-util.h
@@ -114,7 +114,11 @@ int fd_reopen(int fd, int flags);
 int fd_reopen_condition(int fd, int flags, int mask, int *ret_new_fd);
 
 int fd_is_opath(int fd);
-int fd_verify_safe_flags(int fd);
+
+int fd_verify_safe_flags_full(int fd, int extra_flags);
+static inline int fd_verify_safe_flags(int fd) {
+        return fd_verify_safe_flags_full(fd, 0);
+}
 
 int read_nr_open(void);
 int fd_get_diskseq(int fd, uint64_t *ret);
diff --git a/src/import/importd.c b/src/import/importd.c
index 4bba46984ae..3bfa3cdd752 100644
--- a/src/import/importd.c
+++ b/src/import/importd.c
@@ -901,7 +901,7 @@ static int method_import_fs(sd_bus_message *msg, void *userdata, sd_bus_error *e
                 SET_FLAG(flags, IMPORT_READ_ONLY, read_only);
         }
 
-        r = fd_verify_safe_flags(fd);
+        r = fd_verify_safe_flags_full(fd, O_DIRECTORY);
         if (r < 0)
                 return r;