95602a
From 7a3843972ea290daf1bec5e1133db654749b8c02 Mon Sep 17 00:00:00 2001
95602a
From: Franck Bui <fbui@suse.com>
95602a
Date: Tue, 22 Oct 2019 16:09:21 +0200
95602a
Subject: [PATCH] fileio: introduce read_full_virtual_file() for reading
95602a
 virtual files in sysfs, procfs
95602a
95602a
Virtual filesystems such as sysfs or procfs use kernfs, and kernfs can work
95602a
with two sorts of virtual files.
95602a
95602a
One sort uses "seq_file", and the results of the first read are buffered for
95602a
the second read. The other sort uses "raw" reads which always go direct to the
95602a
device.
95602a
95602a
In the later case, the content of the virtual file must be retrieved with a
95602a
single read otherwise subsequent read might get the new value instead of
95602a
finding EOF immediately. That's the reason why the usage of fread(3) is
95602a
prohibited in this case as it always performs a second call to read(2) looking
95602a
for EOF which is subject to the race described previously.
95602a
95602a
Fixes: #13585.
95602a
(cherry picked from commit 21b40f16622f171a9969dc334d74fb5eb2f575c2)
95602a
95602a
Related: #2117948
95602a
---
95602a
 src/basic/fileio.c                   | 115 ++++++++++++++++++++++++++-
95602a
 src/basic/fileio.h                   |   1 +
95602a
 src/libsystemd/sd-device/sd-device.c |   2 +-
95602a
 3 files changed, 113 insertions(+), 5 deletions(-)
95602a
95602a
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
95602a
index 6b0bad5b71..733fb42463 100644
95602a
--- a/src/basic/fileio.c
95602a
+++ b/src/basic/fileio.c
95602a
@@ -276,6 +276,113 @@ int verify_file(const char *fn, const char *blob, bool accept_extra_nl) {
95602a
         return 1;
95602a
 }
95602a
 
95602a
+int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size) {
95602a
+        _cleanup_free_ char *buf = NULL;
95602a
+        _cleanup_close_ int fd = -1;
95602a
+        struct stat st;
95602a
+        size_t n, size;
95602a
+        int n_retries;
95602a
+        char *p;
95602a
+
95602a
+        assert(ret_contents);
95602a
+
95602a
+        /* Virtual filesystems such as sysfs or procfs use kernfs, and kernfs can work
95602a
+         * with two sorts of virtual files. One sort uses "seq_file", and the results of
95602a
+         * the first read are buffered for the second read. The other sort uses "raw"
95602a
+         * reads which always go direct to the device. In the latter case, the content of
95602a
+         * the virtual file must be retrieved with a single read otherwise a second read
95602a
+         * might get the new value instead of finding EOF immediately. That's the reason
95602a
+         * why the usage of fread(3) is prohibited in this case as it always performs a
95602a
+         * second call to read(2) looking for EOF. See issue 13585. */
95602a
+
95602a
+        fd = open(filename, O_RDONLY|O_CLOEXEC);
95602a
+        if (fd < 0)
95602a
+                return -errno;
95602a
+
95602a
+        /* Start size for files in /proc which usually report a file size of 0. */
95602a
+        size = LINE_MAX / 2;
95602a
+
95602a
+        /* Limit the number of attempts to read the number of bytes returned by fstat(). */
95602a
+        n_retries = 3;
95602a
+
95602a
+        for (;;) {
95602a
+                if (n_retries <= 0)
95602a
+                        return -EIO;
95602a
+
95602a
+                if (fstat(fd, &st) < 0)
95602a
+                        return -errno;
95602a
+
95602a
+                if (!S_ISREG(st.st_mode))
95602a
+                        return -EBADF;
95602a
+
95602a
+                /* Be prepared for files from /proc which generally report a file size of 0. */
95602a
+                if (st.st_size > 0) {
95602a
+                        size = st.st_size;
95602a
+                        n_retries--;
95602a
+                } else
95602a
+                        size = size * 2;
95602a
+
95602a
+                if (size > READ_FULL_BYTES_MAX)
95602a
+                        return -E2BIG;
95602a
+
95602a
+                p = realloc(buf, size + 1);
95602a
+                if (!p)
95602a
+                        return -ENOMEM;
95602a
+                buf = TAKE_PTR(p);
95602a
+
95602a
+                for (;;) {
95602a
+                        ssize_t k;
95602a
+
95602a
+                        /* Read one more byte so we can detect whether the content of the
95602a
+                         * file has already changed or the guessed size for files from /proc
95602a
+                         * wasn't large enough . */
95602a
+                        k = read(fd, buf, size + 1);
95602a
+                        if (k >= 0) {
95602a
+                                n = k;
95602a
+                                break;
95602a
+                        }
95602a
+
95602a
+                        if (errno != -EINTR)
95602a
+                                return -errno;
95602a
+                }
95602a
+
95602a
+                /* Consider a short read as EOF */
95602a
+                if (n <= size)
95602a
+                        break;
95602a
+
95602a
+                /* Hmm... either we read too few bytes from /proc or less likely the content
95602a
+                 * of the file might have been changed (and is now bigger) while we were
95602a
+                 * processing, let's try again either with a bigger guessed size or the new
95602a
+                 * file size. */
95602a
+
95602a
+                if (lseek(fd, 0, SEEK_SET) < 0)
95602a
+                        return -errno;
95602a
+        }
95602a
+
95602a
+        if (n < size) {
95602a
+                p = realloc(buf, n + 1);
95602a
+                if (!p)
95602a
+                        return -ENOMEM;
95602a
+                buf = TAKE_PTR(p);
95602a
+        }
95602a
+
95602a
+        if (!ret_size) {
95602a
+                /* Safety check: if the caller doesn't want to know the size of what we
95602a
+                 * just read it will rely on the trailing NUL byte. But if there's an
95602a
+                 * embedded NUL byte, then we should refuse operation as otherwise
95602a
+                 * there'd be ambiguity about what we just read. */
95602a
+
95602a
+                if (memchr(buf, 0, n))
95602a
+                        return -EBADMSG;
95602a
+        } else
95602a
+                *ret_size = n;
95602a
+
95602a
+        buf[n] = 0;
95602a
+        *ret_contents = TAKE_PTR(buf);
95602a
+
95602a
+        return 0;
95602a
+}
95602a
+
95602a
 int read_full_stream(FILE *f, char **contents, size_t *size) {
95602a
         _cleanup_free_ char *buf = NULL;
95602a
         struct stat st;
95602a
@@ -300,9 +407,9 @@ int read_full_stream(FILE *f, char **contents, size_t *size) {
95602a
                         if (st.st_size > READ_FULL_BYTES_MAX)
95602a
                                 return -E2BIG;
95602a
 
95602a
-                        /* Start with the right file size, but be prepared for files from /proc which generally report a file
95602a
-                         * size of 0. Note that we increase the size to read here by one, so that the first read attempt
95602a
-                         * already makes us notice the EOF. */
95602a
+                        /* Start with the right file size. Note that we increase the size
95602a
+                         * to read here by one, so that the first read attempt already
95602a
+                         * makes us notice the EOF. */
95602a
                         if (st.st_size > 0)
95602a
                                 n = st.st_size + 1;
95602a
                 }
95602a
@@ -986,7 +1093,7 @@ int get_proc_field(const char *filename, const char *pattern, const char *termin
95602a
         assert(pattern);
95602a
         assert(field);
95602a
 
95602a
-        r = read_full_file(filename, &status, NULL);
95602a
+        r = read_full_virtual_file(filename, &status, NULL);
95602a
         if (r < 0)
95602a
                 return r;
95602a
 
95602a
diff --git a/src/basic/fileio.h b/src/basic/fileio.h
95602a
index 77e6206e95..c6ad375b8d 100644
95602a
--- a/src/basic/fileio.h
95602a
+++ b/src/basic/fileio.h
95602a
@@ -38,6 +38,7 @@ int write_string_filef(const char *fn, WriteStringFileFlags flags, const char *f
95602a
 int read_one_line_file(const char *fn, char **line);
95602a
 int read_full_file(const char *fn, char **contents, size_t *size);
95602a
 int read_full_stream(FILE *f, char **contents, size_t *size);
95602a
+int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size);
95602a
 
95602a
 int verify_file(const char *fn, const char *blob, bool accept_extra_nl);
95602a
 
95602a
diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
95602a
index be29053f8c..49750ba9d7 100644
95602a
--- a/src/libsystemd/sd-device/sd-device.c
95602a
+++ b/src/libsystemd/sd-device/sd-device.c
95602a
@@ -1798,7 +1798,7 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr,
95602a
                 size_t size;
95602a
 
95602a
                 /* read attribute value */
95602a
-                r = read_full_file(path, &value, &size);
95602a
+                r = read_full_virtual_file(path, &value, &size);
95602a
                 if (r < 0)
95602a
                         return r;
95602a