fileio: introduce read_full_virtual_file() for reading virtual files in sysfs, procfs

Virtual filesystems such as sysfs or procfs use kernfs, and kernfs can work
with two sorts of virtual files.

One sort uses "seq_file", and the results of the first read are buffered for
the second read. The other sort uses "raw" reads which always go direct to the
device.

In the later case, the content of the virtual file must be retrieved with a
single read otherwise subsequent read might get the new value instead of
finding EOF immediately. That's the reason why the usage of fread(3) is
prohibited in this case as it always performs a second call to read(2) looking
for EOF which is subject to the race described previously.

Fixes: #13585.

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index a9c0fd20e14fa989a0f42701a760b9afa6442d8e..e1600a1e6ae89103ab386aa0a96ce8d04cb83c4a 100644 (file)
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -310,6 +310,113 @@ int verify_file(const char *fn, const char *blob, bool accept_extra_nl) {
         return 1;
 }
 
+int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size) {
+        _cleanup_free_ char *buf = NULL;
+        _cleanup_close_ int fd = -1;
+        struct stat st;
+        size_t n, size;
+        int n_retries;
+        char *p;
+
+        assert(ret_contents);
+
+        /* Virtual filesystems such as sysfs or procfs use kernfs, and kernfs can work
+         * with two sorts of virtual files. One sort uses "seq_file", and the results of
+         * the first read are buffered for the second read. The other sort uses "raw"
+         * reads which always go direct to the device. In the latter case, the content of
+         * the virtual file must be retrieved with a single read otherwise a second read
+         * might get the new value instead of finding EOF immediately. That's the reason
+         * why the usage of fread(3) is prohibited in this case as it always performs a
+         * second call to read(2) looking for EOF. See issue 13585. */
+
+        fd = open(filename, O_RDONLY|O_CLOEXEC);
+        if (fd < 0)
+                return -errno;
+
+        /* Start size for files in /proc which usually report a file size of 0. */
+        size = LINE_MAX / 2;
+
+        /* Limit the number of attempts to read the number of bytes returned by fstat(). */
+        n_retries = 3;
+
+        for (;;) {
+                if (n_retries <= 0)
+                        return -EIO;
+
+                if (fstat(fd, &st) < 0)
+                        return -errno;
+
+                if (!S_ISREG(st.st_mode))
+                        return -EBADF;
+
+                /* Be prepared for files from /proc which generally report a file size of 0. */
+                if (st.st_size > 0) {
+                        size = st.st_size;
+                        n_retries--;
+                } else
+                        size = size * 2;
+
+                if (size > READ_FULL_BYTES_MAX)
+                        return -E2BIG;
+
+                p = realloc(buf, size + 1);
+                if (!p)
+                        return -ENOMEM;
+                buf = TAKE_PTR(p);
+
+                for (;;) {
+                        ssize_t k;
+
+                        /* Read one more byte so we can detect whether the content of the
+                         * file has already changed or the guessed size for files from /proc
+                         * wasn't large enough . */
+                        k = read(fd, buf, size + 1);
+                        if (k >= 0) {
+                                n = k;
+                                break;
+                        }
+
+                        if (errno != -EINTR)
+                                return -errno;
+                }
+
+                /* Consider a short read as EOF */
+                if (n <= size)
+                        break;
+
+                /* Hmm... either we read too few bytes from /proc or less likely the content
+                 * of the file might have been changed (and is now bigger) while we were
+                 * processing, let's try again either with a bigger guessed size or the new
+                 * file size. */
+
+                if (lseek(fd, 0, SEEK_SET) < 0)
+                        return -errno;
+        }
+
+        if (n < size) {
+                p = realloc(buf, n + 1);
+                if (!p)
+                        return -ENOMEM;
+                buf = TAKE_PTR(p);
+        }
+
+        if (!ret_size) {
+                /* Safety check: if the caller doesn't want to know the size of what we
+                 * just read it will rely on the trailing NUL byte. But if there's an
+                 * embedded NUL byte, then we should refuse operation as otherwise
+                 * there'd be ambiguity about what we just read. */
+
+                if (memchr(buf, 0, n))
+                        return -EBADMSG;
+        } else
+                *ret_size = n;
+
+        buf[n] = 0;
+        *ret_contents = TAKE_PTR(buf);
+
+        return 0;
+}
+
 int read_full_stream_full(
                 FILE *f,
                 const char *filename,
@@ -342,9 +449,9 @@ int read_full_stream_full(
                         if (st.st_size > READ_FULL_BYTES_MAX)
                                 return -E2BIG;
 
-                        /* Start with the right file size, but be prepared for files from /proc which generally report a file
-                         * size of 0. Note that we increase the size to read here by one, so that the first read attempt
-                         * already makes us notice the EOF. */
+                        /* Start with the right file size. Note that we increase the size
+                         * to read here by one, so that the first read attempt already
+                         * makes us notice the EOF. */
                         if (st.st_size > 0)
                                 n_next = st.st_size + 1;
 
@@ -502,7 +609,7 @@ int get_proc_field(const char *filename, const char *pattern, const char *termin
         assert(pattern);
         assert(field);
 
-        r = read_full_file(filename, &status, NULL);
+        r = read_full_virtual_file(filename, &status, NULL);
         if (r < 0)
                 return r;
 

diff --git a/src/basic/fileio.h b/src/basic/fileio.h
index 05f6c89da09dcdc52d337af9b74d9fd8ea9f3b87..31bfef33ac941a5eef3430d97ef695b9a43e3de0 100644 (file)
--- a/src/basic/fileio.h
+++ b/src/basic/fileio.h
@@ -56,6 +56,7 @@ int read_full_file_full(const char *filename, ReadFullFileFlags flags, char **co
 static inline int read_full_file(const char *filename, char **contents, size_t *size) {
         return read_full_file_full(filename, 0, contents, size);
 }
+int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size);
 int read_full_stream_full(FILE *f, const char *filename, ReadFullFileFlags flags, char **contents, size_t *size);
 static inline int read_full_stream(FILE *f, char **contents, size_t *size) {
         return read_full_stream_full(f, NULL, 0, contents, size);

diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
index 26ae453ea777564d78789d1f68920afd54fbde1c..f35612fe12bc7cc2f9b3bd2f8066d59c49a4f0d4 100644 (file)
--- a/src/libsystemd/sd-device/sd-device.c
+++ b/src/libsystemd/sd-device/sd-device.c
@@ -1786,7 +1786,7 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr,
                 size_t size;
 
                 /* read attribute value */
-                r = read_full_file(path, &value, &size);
+                r = read_full_virtual_file(path, &value, &size);
                 if (r < 0)
                         return r;