Merge pull request #15417 from poettering/fileno-can-fail

fileio: fileno() can realistically return -1

5 files changed, 68 insertions, 26 deletions

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 4c365ad6fa..34ee939526 100644
--- 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;
 
@@ -995,7 +1009,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, funlockfile);
 int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
         size_t n = 0, allocated = 0, count = 0;
         _cleanup_free_ char *buffer = NULL;
-        int r, tty = -1;
+        int r;
 
         assert(f);
 
@@ -1070,13 +1084,23 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
                         count++;
 
                         if (eol != EOL_NONE) {
-                                /* If we are on a tty, we can't wait for more input. But we expect only
-                                 * \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)
+                                /* If we are on a tty, we can't shouldn't wait for more input, because that
+                                 * generally means waiting for the user, interactively. In the case of a TTY
+                                 * we expect only \n as the single EOL marker, so we are in the lucky
+                                 * position that there is no need to wait. We check this condition last, to
+                                 * avoid isatty() check if not necessary. */
+
+                                if ((flags & (READ_LINE_IS_A_TTY|READ_LINE_NOT_A_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 */
+                                                flags |= READ_LINE_NOT_A_TTY;
+                                        else
+                                                flags |= isatty(fd) ? READ_LINE_IS_A_TTY : READ_LINE_NOT_A_TTY;
+                                }
+                                if (FLAGS_SET(flags, READ_LINE_IS_A_TTY))
                                         break;
                         }
 
diff --git a/src/basic/fileio.h b/src/basic/fileio.h
index 58daabaa8f..e6062121a3 100644
--- a/src/basic/fileio.h
+++ b/src/basic/fileio.h
@@ -88,7 +88,9 @@ int read_timestamp_file(const char *fn, usec_t *ret);
 int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space);
 
 typedef enum ReadLineFlags {
-        READ_LINE_ONLY_NUL = 1 << 0,
+        READ_LINE_ONLY_NUL  = 1 << 0,
+        READ_LINE_IS_A_TTY  = 1 << 1,
+        READ_LINE_NOT_A_TTY = 1 << 2,
 } ReadLineFlags;
 
 int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret);
diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c
index 511734cbbb..6cacde90ba 100644
--- 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/libsystemd/sd-hwdb/hwdb-util.c b/src/libsystemd/sd-hwdb/hwdb-util.c
index d790e8fd0b..5c7521695e 100644
--- a/src/libsystemd/sd-hwdb/hwdb-util.c
+++ b/src/libsystemd/sd-hwdb/hwdb-util.c
@@ -488,7 +488,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
                 size_t len;
                 char *pos;
 
-                r = read_line(f, LONG_LINE_MAX, &line);
+                r = read_line_full(f, LONG_LINE_MAX, READ_LINE_NOT_A_TTY, &line);
                 if (r < 0)
                         return r;
                 if (r == 0)
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index 657df0a517..3ba33606fb 100644
--- 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;