From 21b40f16622f171a9969dc334d74fb5eb2f575c2 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 22 Oct 2019 16:09:21 +0200 Subject: [PATCH] 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. --- src/basic/fileio.c | 115 ++++++++++++++++++++++++++- src/basic/fileio.h | 1 + src/libsystemd/sd-device/sd-device.c | 2 +- 3 files changed, 113 insertions(+), 5 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index a9c0fd20e14..e1600a1e6ae 100644 --- 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 05f6c89da09..31bfef33ac9 100644 --- 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 26ae453ea77..f35612fe12b 100644 --- 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; -- 2.39.2