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.
This commit is contained in:
Lars Ellenberg 2024-02-07 13:12:50 +01:00 committed by Lennart Poettering
parent d3d880e558
commit b8e25bff38
3 changed files with 52 additions and 3 deletions

View file

@ -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,

View file

@ -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);

View file

@ -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");