fileio: fileno() can realistically return -1

An stdio FILE* stream usually refers to something with a file
descriptor, but that's just "usually". It doesn't have to, when taking
fmemopen() and similar into account. Most of our calls to fileno()
assumed the call couldn't fail. In most cases this was correct, but in
some cases where we didn't know whether we work on files or memory we'd
use the returned fd as if it was unconditionally valid while it wasn't,
and passed it to a multitude of kernel syscalls. Let's fix that, and do
something reasonably smart when encountering this case.

(Running test-fileio with this patch applied will remove tons of ioctl()
calls on -1).

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 4c365ad6fa9149355a7cc8a0530cd4868a419c5e..387b3253b5e3e449e3cae50715d02545419f51b0 100644 (file)
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -119,7 +119,7 @@ int write_string_stream_ts(
                 struct timespec *ts) {
 
         bool needs_nl;
-        int r;
+        int r, fd;
 
         assert(f);
         assert(line);
@@ -127,6 +127,14 @@ int write_string_stream_ts(
         if (ferror(f))
                 return -EIO;
 
+        if (ts) {
+                /* If we shall set the timestamp we need the fd. But fmemopen() streams generally don't have
+                 * an fd. Let's fail early in that case. */
+                fd = fileno(f);
+                if (fd < 0)
+                        return -EBADF;
+        }
+
         needs_nl = !(flags & WRITE_STRING_FILE_AVOID_NEWLINE) && !endswith(line, "\n");
 
         if (needs_nl && (flags & WRITE_STRING_FILE_DISABLE_BUFFER)) {
@@ -154,7 +162,7 @@ int write_string_stream_ts(
         if (ts) {
                 struct timespec twice[2] = {*ts, *ts};
 
-                if (futimens(fileno(f), twice) < 0)
+                if (futimens(fd, twice) < 0)
                         return -errno;
         }
 
@@ -886,7 +894,7 @@ int fflush_and_check(FILE *f) {
 }
 
 int fflush_sync_and_check(FILE *f) {
-        int r;
+        int r, fd;
 
         assert(f);
 
@@ -894,10 +902,16 @@ int fflush_sync_and_check(FILE *f) {
         if (r < 0)
                 return r;
 
-        if (fsync(fileno(f)) < 0)
+        /* Not all file streams have an fd associated (think: fmemopen()), let's handle this gracefully and
+         * assume that in that case we need no explicit syncing */
+        fd = fileno(f);
+        if (fd < 0)
+                return 0;
+
+        if (fsync(fd) < 0)
                 return -errno;
 
-        r = fsync_directory_of_file(fileno(f));
+        r = fsync_directory_of_file(fd);
         if (r < 0)
                 return r;
 
@@ -1074,8 +1088,16 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
                                  * \n as the single EOL marker, so there is no need to wait. We check
                                  * this condition last to avoid isatty() check if not necessary. */
 
-                                if (tty < 0)
-                                        tty = isatty(fileno(f));
+                                if (tty < 0) {
+                                        int fd;
+
+                                        fd = fileno(f);
+                                        if (fd < 0) /* Maybe an fmemopen() stream? Handle this gracefully,
+                                                     * and don't call isatty() on an invalid fd */
+                                                tty = false;
+                                        else
+                                                tty = isatty(fd);
+                                }
                                 if (tty > 0)
                                         break;
                         }

diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c
index 511734cbbb8fd085c208b7b040be858d1c692edf..6cacde90bac6b58333368ed1cf6dfc301c338a17 100644 (file)
--- a/src/basic/terminal-util.c
+++ b/src/basic/terminal-util.c
@@ -81,31 +81,34 @@ int chvt(int vt) {
 int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) {
         _cleanup_free_ char *line = NULL;
         struct termios old_termios;
-        int r;
+        int r, fd;
 
         assert(f);
         assert(ret);
 
-        /* If this is a terminal, then switch canonical mode off, so that we can read a single character */
-        if (tcgetattr(fileno(f), &old_termios) >= 0) {
+        /* If this is a terminal, then switch canonical mode off, so that we can read a single
+         * character. (Note that fmemopen() streams do not have an fd associated with them, let's handle that
+         * nicely.) */
+        fd = fileno(f);
+        if (fd >= 0 && tcgetattr(fd, &old_termios) >= 0) {
                 struct termios new_termios = old_termios;
 
                 new_termios.c_lflag &= ~ICANON;
                 new_termios.c_cc[VMIN] = 1;
                 new_termios.c_cc[VTIME] = 0;
 
-                if (tcsetattr(fileno(f), TCSADRAIN, &new_termios) >= 0) {
+                if (tcsetattr(fd, TCSADRAIN, &new_termios) >= 0) {
                         char c;
 
                         if (t != USEC_INFINITY) {
-                                if (fd_wait_for_event(fileno(f), POLLIN, t) <= 0) {
-                                        (void) tcsetattr(fileno(f), TCSADRAIN, &old_termios);
+                                if (fd_wait_for_event(fd, POLLIN, t) <= 0) {
+                                        (void) tcsetattr(fd, TCSADRAIN, &old_termios);
                                         return -ETIMEDOUT;
                                 }
                         }
 
                         r = safe_fgetc(f, &c);
-                        (void) tcsetattr(fileno(f), TCSADRAIN, &old_termios);
+                        (void) tcsetattr(fd, TCSADRAIN, &old_termios);
                         if (r < 0)
                                 return r;
                         if (r == 0)
@@ -119,8 +122,13 @@ int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) {
                 }
         }
 
-        if (t != USEC_INFINITY) {
-                if (fd_wait_for_event(fileno(f), POLLIN, t) <= 0)
+        if (t != USEC_INFINITY && fd > 0) {
+                /* Let's wait the specified amount of time for input. When we have no fd we skip this, under
+                 * the assumption that this is an fmemopen() stream or so where waiting doesn't make sense
+                 * anyway, as the data is either already in the stream or cannot possible be placed there
+                 * while we access the stream */
+
+                if (fd_wait_for_event(fd, POLLIN, t) <= 0)
                         return -ETIMEDOUT;
         }
 
@@ -778,6 +786,9 @@ const char *default_term_for_tty(const char *tty) {
 int fd_columns(int fd) {
         struct winsize ws = {};
 
+        if (fd < 0)
+                return -EBADF;
+
         if (ioctl(fd, TIOCGWINSZ, &ws) < 0)
                 return -errno;
 
@@ -812,6 +823,9 @@ unsigned columns(void) {
 int fd_lines(int fd) {
         struct winsize ws = {};
 
+        if (fd < 0)
+                return -EBADF;
+
         if (ioctl(fd, TIOCGWINSZ, &ws) < 0)
                 return -errno;
 

diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index 657df0a517a1bb96a9547f7f21c3ae1df9a2ecd6..3ba33606fbcdfaaa3676404a6f33bd7428d5d351 100644 (file)
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -294,7 +294,7 @@ int config_parse(const char *unit,
         _cleanup_fclose_ FILE *ours = NULL;
         unsigned line = 0, section_line = 0;
         bool section_ignored = false, bom_seen = false;
-        int r;
+        int r, fd;
 
         assert(filename);
         assert(lookup);
@@ -311,7 +311,9 @@ int config_parse(const char *unit,
                 }
         }
 
-        fd_warn_permissions(filename, fileno(f));
+        fd = fileno(f);
+        if (fd >= 0) /* stream might not have an fd, let's be careful hence */
+                fd_warn_permissions(filename, fd);
 
         for (;;) {
                 _cleanup_free_ char *buf = NULL;