[PATCH 13/13] fs/kernel_file_read: Add "offset" arg for partial reads
From: Kees Cook <hidden>
Date: 2020-07-17 17:43:46
Also in:
kexec, linux-fsdevel, linux-integrity, lkml, selinux
Subsystem:
driver core, kobjects, debugfs and sysfs, extended verification module (evm), filesystems (vfs and infrastructure), firmware loader (request_firmware), integrity measurement architecture (ima), kexec, security subsystem, the rest · Maintainers:
Greg Kroah-Hartman, "Rafael J. Wysocki", Danilo Krummrich, Mimi Zohar, Roberto Sassu, Alexander Viro, Christian Brauner, Luis Chamberlain, Russ Weight, Dmitry Kasatkin, Andrew Morton, Baoquan He, Mike Rapoport, Pasha Tatashin, Pratyush Yadav, Paul Moore, James Morris, "Serge E. Hallyn", Linus Torvalds
To perform partial reads, callers of kernel_read_file*() must have a non-NULL file_size argument and a preallocated buffer. The new "offset" argument can then be used to seek to specific locations in the file to fill the buffer to, at most, "buf_size" per call. Where possible, the LSM hooks can report whether a full file has been read or not so that the contents can be reasoned about. Signed-off-by: Kees Cook <redacted> --- drivers/base/firmware_loader/main.c | 2 +- fs/kernel_read_file.c | 78 ++++++++++++++++++++--------- include/linux/kernel_read_file.h | 8 +-- kernel/kexec_file.c | 4 +- kernel/module.c | 2 +- security/integrity/digsig.c | 2 +- security/integrity/ima/ima_fs.c | 3 +- 7 files changed, 65 insertions(+), 34 deletions(-)
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 3439a533927c..fa540ca51961 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c@@ -494,7 +494,7 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv, fw_priv->size = 0; /* load firmware files from the mount namespace of init */ - rc = kernel_read_file_from_path_initns(path, &buffer, msize, + rc = kernel_read_file_from_path_initns(path, 0, &buffer, msize, NULL, READING_FIRMWARE); if (rc < 0) {
diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index d73bc3fa710a..90d255fbdd9b 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c@@ -9,6 +9,7 @@ * kernel_read_file() - read file contents into a kernel buffer * * @file file to read from + * @offset where to start reading from (see below). * @buf pointer to a "void *" buffer for reading into (if * *@buf is NULL, a buffer will be allocated, and * @buf_size will be ignored)
@@ -19,19 +20,31 @@ * @id the kernel_read_file_id identifying the type of * file contents being read (for LSMs to examine) * + * @offset must be 0 unless both @buf and @file_size are non-NULL + * (i.e. the caller must be expecting to read partial file contents + * via an already-allocated @buf, in at most @buf_size chunks, and + * will be able to determine when the entire file was read by + * checking @file_size). This isn't a recommended way to read a + * file, though, since it is possible that the contents might + * change between calls to kernel_read_file(). + * * Returns number of bytes read (no single read will be bigger * than INT_MAX), or negative on error. * */ -int kernel_read_file(struct file *file, void **buf, +int kernel_read_file(struct file *file, loff_t offset, void **buf, size_t buf_size, size_t *file_size, enum kernel_read_file_id id) { loff_t i_size, pos; - ssize_t bytes = 0; + size_t copied; void *allocated = NULL; + bool whole_file; int ret; + if (offset != 0 && (!*buf || !file_size)) + return -EINVAL; + if (!S_ISREG(file_inode(file)->i_mode)) return -EINVAL;
@@ -39,19 +52,27 @@ int kernel_read_file(struct file *file, void **buf, if (ret) return ret; - ret = security_kernel_read_file(file, id, true); - if (ret) - goto out; - i_size = i_size_read(file_inode(file)); if (i_size <= 0) { ret = -EINVAL; goto out; } - if (i_size > INT_MAX || i_size > buf_size) { + /* The file is too big for sane activities. */ + if (i_size > INT_MAX) { + ret = -EFBIG; + goto out; + } + /* The entire file cannot be read in one buffer. */ + if (!file_size && offset == 0 && i_size > buf_size) { ret = -EFBIG; goto out; } + + whole_file = (offset == 0 && i_size <= buf_size); + ret = security_kernel_read_file(file, id, whole_file); + if (ret) + goto out; + if (file_size) *file_size = i_size;
@@ -62,9 +83,14 @@ int kernel_read_file(struct file *file, void **buf, goto out; } - pos = 0; - while (pos < i_size) { - bytes = kernel_read(file, *buf + pos, i_size - pos, &pos); + pos = offset; + copied = 0; + while (copied < buf_size) { + ssize_t bytes; + size_t wanted = min_t(size_t, buf_size - copied, + i_size - pos); + + bytes = kernel_read(file, *buf + copied, wanted, &pos); if (bytes < 0) { ret = bytes; goto out_free;
@@ -72,14 +98,17 @@ int kernel_read_file(struct file *file, void **buf, if (bytes == 0) break; + copied += bytes; } - if (pos != i_size) { - ret = -EIO; - goto out_free; - } + if (whole_file) { + if (pos != i_size) { + ret = -EIO; + goto out_free; + } - ret = security_kernel_post_read_file(file, *buf, i_size, id); + ret = security_kernel_post_read_file(file, *buf, i_size, id); + } out_free: if (ret < 0) {
@@ -91,11 +120,11 @@ int kernel_read_file(struct file *file, void **buf, out: allow_write_access(file); - return ret == 0 ? pos : ret; + return ret == 0 ? copied : ret; } EXPORT_SYMBOL_GPL(kernel_read_file); -int kernel_read_file_from_path(const char *path, void **buf, +int kernel_read_file_from_path(const char *path, loff_t offset, void **buf, size_t buf_size, size_t *file_size, enum kernel_read_file_id id) {
@@ -109,14 +138,15 @@ int kernel_read_file_from_path(const char *path, void **buf, if (IS_ERR(file)) return PTR_ERR(file); - ret = kernel_read_file(file, buf, buf_size, file_size, id); + ret = kernel_read_file(file, offset, buf, buf_size, file_size, id); fput(file); return ret; } EXPORT_SYMBOL_GPL(kernel_read_file_from_path); -int kernel_read_file_from_path_initns(const char *path, void **buf, - size_t buf_size, size_t *file_size, +int kernel_read_file_from_path_initns(const char *path, loff_t offset, + void **buf, size_t buf_size, + size_t *file_size, enum kernel_read_file_id id) { struct file *file;
@@ -135,14 +165,14 @@ int kernel_read_file_from_path_initns(const char *path, void **buf, if (IS_ERR(file)) return PTR_ERR(file); - ret = kernel_read_file(file, buf, buf_size, file_size, id); + ret = kernel_read_file(file, offset, buf, buf_size, file_size, id); fput(file); return ret; } EXPORT_SYMBOL_GPL(kernel_read_file_from_path_initns); -int kernel_read_file_from_fd(int fd, void **buf, size_t buf_size, - size_t *file_size, +int kernel_read_file_from_fd(int fd, loff_t offset, void **buf, + size_t buf_size, size_t *file_size, enum kernel_read_file_id id) { struct fd f = fdget(fd);
@@ -151,7 +181,7 @@ int kernel_read_file_from_fd(int fd, void **buf, size_t buf_size, if (!f.file) goto out; - ret = kernel_read_file(f.file, buf, buf_size, file_size, id); + ret = kernel_read_file(f.file, offset, buf, buf_size, file_size, id); out: fdput(f); return ret;
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 023293eaf948..575ffa1031d3 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h@@ -35,19 +35,19 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id) return kernel_read_file_str[id]; } -int kernel_read_file(struct file *file, +int kernel_read_file(struct file *file, loff_t offset, void **buf, size_t buf_size, size_t *file_size, enum kernel_read_file_id id); -int kernel_read_file_from_path(const char *path, +int kernel_read_file_from_path(const char *path, loff_t offset, void **buf, size_t buf_size, size_t *file_size, enum kernel_read_file_id id); -int kernel_read_file_from_path_initns(const char *path, +int kernel_read_file_from_path_initns(const char *path, loff_t offset, void **buf, size_t buf_size, size_t *file_size, enum kernel_read_file_id id); -int kernel_read_file_from_fd(int fd, +int kernel_read_file_from_fd(int fd, loff_t offset, void **buf, size_t buf_size, size_t *file_size, enum kernel_read_file_id id);
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 7b0ecdc621aa..c340a14c4cb9 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c@@ -221,7 +221,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, int ret; void *ldata; - ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf, + ret = kernel_read_file_from_fd(kernel_fd, 0, &image->kernel_buf, INT_MAX, NULL, READING_KEXEC_IMAGE); if (ret < 0) return ret;
@@ -241,7 +241,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, #endif /* It is possible that there no initramfs is being loaded */ if (!(flags & KEXEC_FILE_NO_INITRAMFS)) { - ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf, + ret = kernel_read_file_from_fd(initrd_fd, 0, &image->initrd_buf, INT_MAX, NULL, READING_KEXEC_INITRAMFS); if (ret < 0)
diff --git a/kernel/module.c b/kernel/module.c
index 90a4788dff9d..d353d1f67681 100644
--- a/kernel/module.c
+++ b/kernel/module.c@@ -4007,7 +4007,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags) |MODULE_INIT_IGNORE_VERMAGIC)) return -EINVAL; - err = kernel_read_file_from_fd(fd, &hdr, INT_MAX, NULL, + err = kernel_read_file_from_fd(fd, 0, &hdr, INT_MAX, NULL, READING_MODULE); if (err < 0) return err;
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 8a523dfd7fd7..0f518dcfde05 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c@@ -175,7 +175,7 @@ int __init integrity_load_x509(const unsigned int id, const char *path) int rc; key_perm_t perm; - rc = kernel_read_file_from_path(path, &data, INT_MAX, NULL, + rc = kernel_read_file_from_path(path, 0, &data, INT_MAX, NULL, READING_X509_CERTIFICATE); if (rc < 0) { pr_err("Unable to open file: %s (%d)", path, rc);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index cff04083e598..4212423714ee 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c@@ -284,7 +284,8 @@ static ssize_t ima_read_policy(char *path) datap = path; strsep(&datap, "\n"); - rc = kernel_read_file_from_path(path, &data, INT_MAX, NULL, READING_POLICY); + rc = kernel_read_file_from_path(path, 0, &data, INT_MAX, NULL, + READING_POLICY); if (rc < 0) { pr_err("Unable to open file: %s (%d)", path, rc); return rc;
--
2.25.1