| Message ID | 1455656045-21463-1-git-send-email-zohar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 16, 2016 at 03:54:05PM -0500, Mimi Zohar wrote: > From: Dmitry Kasatkin <d.kasatkin@samsung.com> > > This patch is based on top of the "vfs: support for a common kernel file > loader" patch set. In general when the kernel is reading a file into > memory it does not want anything else writing to it. > > The kernel currently only forbids write access to a file being executed. > This patch extends this locking to files being read by the kernel. > > Changelog: > - moved function to kernel_read_file() - Mimi > - updated patch description - Mimi > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com> > Cc: Al Viro <viro@ZenIV.linux.org.uk> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> BTW I don't see your common read work on linux-next yet, would you resend this later after that queue of patches get merged? Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org> Luis -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 16, 2016 at 12:54 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > From: Dmitry Kasatkin <d.kasatkin@samsung.com> > > This patch is based on top of the "vfs: support for a common kernel file > loader" patch set. In general when the kernel is reading a file into > memory it does not want anything else writing to it. Oh yes please. Thanks for this! That had bothered me for a long time. :) Acked-by: Kees Cook <keescook@chromium.org> -Kees > > The kernel currently only forbids write access to a file being executed. > This patch extends this locking to files being read by the kernel. > > Changelog: > - moved function to kernel_read_file() - Mimi > - updated patch description - Mimi > > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com> > Cc: Al Viro <viro@ZenIV.linux.org.uk> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> > --- > fs/exec.c | 29 +++++++++++++++++++++-------- > 1 file changed, 21 insertions(+), 8 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index 604f669..1b7d617 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -846,15 +846,25 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, > if (ret) > return ret; > > + ret = deny_write_access(file); > + if (ret) > + return ret; > + > i_size = i_size_read(file_inode(file)); > - if (max_size > 0 && i_size > max_size) > - return -EFBIG; > - if (i_size <= 0) > - return -EINVAL; > + if (max_size > 0 && i_size > max_size) { > + ret = -EFBIG; > + goto out; > + } > + if (i_size <= 0) { > + ret = -EINVAL; > + goto out; > + } > > *buf = vmalloc(i_size); > - if (!*buf) > - return -ENOMEM; > + if (!*buf) { > + ret = -ENOMEM; > + goto out; > + } > > pos = 0; > while (pos < i_size) { > @@ -872,18 +882,21 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, > > if (pos != i_size) { > ret = -EIO; > - goto out; > + goto out_free; > } > > ret = security_kernel_post_read_file(file, *buf, i_size, id); > if (!ret) > *size = pos; > > -out: > +out_free: > if (ret < 0) { > vfree(*buf); > *buf = NULL; > } > + > +out: > + allow_write_access(file); > return ret; > } > EXPORT_SYMBOL_GPL(kernel_read_file); > -- > 2.1.0 >
diff --git a/fs/exec.c b/fs/exec.c index 604f669..1b7d617 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -846,15 +846,25 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, if (ret) return ret; + ret = deny_write_access(file); + if (ret) + return ret; + i_size = i_size_read(file_inode(file)); - if (max_size > 0 && i_size > max_size) - return -EFBIG; - if (i_size <= 0) - return -EINVAL; + if (max_size > 0 && i_size > max_size) { + ret = -EFBIG; + goto out; + } + if (i_size <= 0) { + ret = -EINVAL; + goto out; + } *buf = vmalloc(i_size); - if (!*buf) - return -ENOMEM; + if (!*buf) { + ret = -ENOMEM; + goto out; + } pos = 0; while (pos < i_size) { @@ -872,18 +882,21 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, if (pos != i_size) { ret = -EIO; - goto out; + goto out_free; } ret = security_kernel_post_read_file(file, *buf, i_size, id); if (!ret) *size = pos; -out: +out_free: if (ret < 0) { vfree(*buf); *buf = NULL; } + +out: + allow_write_access(file); return ret; } EXPORT_SYMBOL_GPL(kernel_read_file);