Re: [RFC PATCH v2 02/11] vfs: define a generic function to read a file from the kernel
From: Luis R. Rodriguez <hidden>
Date: 2016-01-20 01:10:00
Also in:
kexec
On Mon, Jan 18, 2016 at 10:11:17AM -0500, Mimi Zohar wrote:
quoted hunk ↗ jump to hunk
diff --git a/fs/exec.c b/fs/exec.c index b06623a..6d623c2 100644 --- a/fs/exec.c +++ b/fs/exec.c@@ -831,6 +832,58 @@ int kernel_read(struct file *file, loff_t offset, EXPORT_SYMBOL(kernel_read); +int kernel_read_file(struct file *file, void **buf, loff_t *size, + loff_t max_size) +{ + loff_t i_size, pos; + ssize_t bytes = 0; + int ret; + + if (!S_ISREG(file_inode(file)->i_mode)) + return -EINVAL; + + i_size = i_size_read(file_inode(file)); + if (max_size > 0 && i_size > max_size) + return -EFBIG;
loff_t is a __kernel_loff_t, which in turn is a long long, and that's signed. We don't catch a negative value here, for max_size, we could return -EINVAL if its < 0.
+ if (i_size == 0) + return -EINVAL;
Likewise for i_size. The setter of the size will depend on how the code calling this routine setup the struct file passed. So how about adding a i_size <= 0 check here as well here? At least fw_read_file_contents() has historically done this, so if this generic read is going to skip that I'd like to see why. We're unifying so I rather be more pedantic. Provided this is addressed feel free to peg: Reviewed-by: Luis R. Rodriguez <redacted> Luis