fs-utils: new wrapper fd_reopen_propagate_append_and_position()

We may want to propagate O_APPEND, or (try to) keep the current file position,
even if we use fd_reopen() to re-initialize (and "unshare") other file
description status.

For now, used only with --pty to keep/propagate O_APPEND (and/or) position
if set on stdin/stdout.

If we re-open stdout and "drop" the O_APPEND,
we get rather "unexpected" behavior,
for example with repeated "systemd-run --pty >> some-log".

If someone carefully pre-positioned the passed in original file descriptors,
we avoid surprises if we do not reset file postition to zero.

fcntl F_GETFL first, and propagate O_APPEND if present in the existing flags.

Then use lseek to propagate the file position.

diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c
index 8372c54918d07159dd9643d650eb1959e9e777a5..5d0053029698bab308869047de40dccb2119062c 100644 (file)
--- a/src/basic/fd-util.c
+++ b/src/basic/fd-util.c
@@ -869,6 +869,49 @@ int fd_reopen(int fd, int flags) {
         return new_fd;
 }
 
+int fd_reopen_propagate_append_and_position(int fd, int flags) {
+        /* Invokes fd_reopen(fd, flags), but propagates O_APPEND if set on original fd, and also tries to
+         * keep current file position.
+         *
+         * You should use this if the original fd potentially is O_APPEND, otherwise we get rather
+         * "unexpected" behavior. Unless you intentionally want to overwrite pre-existing data, and have
+         * your output overwritten by the next user.
+         *
+         * Use case: "systemd-run --pty >> some-log".
+         *
+         * The "keep position" part is obviously nonsense for the O_APPEND case, but should reduce surprises
+         * if someone carefully pre-positioned the passed in original input or non-append output FDs. */
+
+        assert(fd >= 0);
+        assert(!(flags & (O_APPEND|O_DIRECTORY)));
+
+        int existing_flags = fcntl(fd, F_GETFL);
+        if (existing_flags < 0)
+                return -errno;
+
+        int new_fd = fd_reopen(fd, flags | (existing_flags & O_APPEND));
+        if (new_fd < 0)
+                return new_fd;
+
+        /* Try to adjust the offset, but ignore errors for now. */
+        off_t p = lseek(fd, 0, SEEK_CUR);
+        if (p <= 0)
+                return new_fd;
+
+        off_t new_p = lseek(new_fd, p, SEEK_SET);
+        if (new_p == (off_t) -1)
+                log_debug_errno(errno,
+                                "Failed to propagate file position for re-opened fd %d, ignoring: %m",
+                                fd);
+        else if (new_p != p)
+                log_debug("Failed to propagate file position for re-opened fd %d (%lld != %lld), ignoring: %m",
+                          fd,
+                          (long long) new_p,
+                          (long long) p);
+
+        return new_fd;
+}
+
 int fd_reopen_condition(
                 int fd,
                 int flags,

diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h
index 1e591085ef88c24e62352575b2d1aa7828239ea8..af17481dd868afcfdd885ae6d73b5013f6ff2362 100644 (file)
--- a/src/basic/fd-util.h
+++ b/src/basic/fd-util.h
@@ -111,6 +111,7 @@ static inline int make_null_stdio(void) {
         })
 
 int fd_reopen(int fd, int flags);
+int fd_reopen_propagate_append_and_position(int fd, int flags);
 int fd_reopen_condition(int fd, int flags, int mask, int *ret_new_fd);
 
 int fd_is_opath(int fd);

diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c
index d572a02ca486e52e2f3e0e4874b7f78f75f59b4b..11997130540a071ce8ecb4e4a56941568d03290c 100644 (file)
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -830,9 +830,13 @@ int pty_forward_new(
                  * them. This has two advantages: when we are killed abruptly the stdin/stdout fds won't be
                  * left in O_NONBLOCK state for the next process using them. In addition, if some process
                  * running in the background wants to continue writing to our stdout it can do so without
-                 * being confused by O_NONBLOCK. */
+                 * being confused by O_NONBLOCK.
+                 * We keep O_APPEND (if present) on the output FD and (try to) keep current file position on
+                 * both input and output FD (principle of least surprise).
+                 */
 
-                f->input_fd = fd_reopen(STDIN_FILENO, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
+                f->input_fd = fd_reopen_propagate_append_and_position(
+                                STDIN_FILENO, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
                 if (f->input_fd < 0) {
                         /* Handle failures gracefully, after all certain fd types cannot be reopened
                          * (sockets, …) */
@@ -846,7 +850,8 @@ int pty_forward_new(
                 } else
                         f->close_input_fd = true;
 
-                f->output_fd = fd_reopen(STDOUT_FILENO, O_WRONLY|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
+                f->output_fd = fd_reopen_propagate_append_and_position(
+                                STDOUT_FILENO, O_WRONLY|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
                 if (f->output_fd < 0) {
                         log_debug_errno(f->output_fd, "Failed to reopen stdout, using original fd: %m");