Re: [PATCH] vfs: Move kernel_read_file() to fs/read_write.c

[Mimi Zohar]

On Mon, 2019-03-04 at 15:11 +0000, David Howells wrote:
> Move kernel_read_file() to fs/read_write.c and out of fs/exec.c as it's not
> actually used by anything in the execve subsystem.

All files being opened by the kernel should be calling one of these
helper routines. ÂHas that changed?

Mimi

> 
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> cc: Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx>
> ---
> 
>  fs/exec.c       |  106 -------------------------------------------------------
>  fs/read_write.c |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 106 insertions(+), 106 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index fb72d36f7823..cbb1a9cd25ca 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -61,7 +61,6 @@
>  #include <linux/pipe_fs_i.h>
>  #include <linux/oom.h>
>  #include <linux/compat.h>
> -#include <linux/vmalloc.h>
> 
>  #include <linux/uaccess.h>
>  #include <asm/mmu_context.h>
> @@ -892,111 +891,6 @@ struct file *open_exec(const char *name)
>  }
>  EXPORT_SYMBOL(open_exec);
> 
> -int kernel_read_file(struct file *file, void **buf, loff_t *size,
> -		     loff_t max_size, enum kernel_read_file_id id)
> -{
> -	loff_t i_size, pos;
> -	ssize_t bytes = 0;
> -	int ret;
> -
> -	if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
> -		return -EINVAL;
> -
> -	ret = deny_write_access(file);
> -	if (ret)
> -		return ret;
> -
> -	ret = security_kernel_read_file(file, id);
> -	if (ret)
> -		goto out;
> -
> -	i_size = i_size_read(file_inode(file));
> -	if (i_size <= 0) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -	if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
> -		ret = -EFBIG;
> -		goto out;
> -	}
> -
> -	if (id != READING_FIRMWARE_PREALLOC_BUFFER)
> -		*buf = vmalloc(i_size);
> -	if (!*buf) {
> -		ret = -ENOMEM;
> -		goto out;
> -	}
> -
> -	pos = 0;
> -	while (pos < i_size) {
> -		bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
> -		if (bytes < 0) {
> -			ret = bytes;
> -			goto out;
> -		}
> -
> -		if (bytes == 0)
> -			break;
> -	}
> -
> -	if (pos != i_size) {
> -		ret = -EIO;
> -		goto out_free;
> -	}
> -
> -	ret = security_kernel_post_read_file(file, *buf, i_size, id);
> -	if (!ret)
> -		*size = pos;
> -
> -out_free:
> -	if (ret < 0) {
> -		if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
> -			vfree(*buf);
> -			*buf = NULL;
> -		}
> -	}
> -
> -out:
> -	allow_write_access(file);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(kernel_read_file);
> -
> -int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
> -			       loff_t max_size, enum kernel_read_file_id id)
> -{
> -	struct file *file;
> -	int ret;
> -
> -	if (!path || !*path)
> -		return -EINVAL;
> -
> -	file = filp_open(path, O_RDONLY, 0);
> -	if (IS_ERR(file))
> -		return PTR_ERR(file);
> -
> -	ret = kernel_read_file(file, buf, size, max_size, id);
> -	fput(file);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
> -
> -int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
> -			     enum kernel_read_file_id id)
> -{
> -	struct fd f = fdget(fd);
> -	int ret = -EBADF;
> -
> -	if (!f.file)
> -		goto out;
> -
> -	ret = kernel_read_file(f.file, buf, size, max_size, id);
> -out:
> -	fdput(f);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
> -
>  ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
>  {
>  	ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
> diff --git a/fs/read_write.c b/fs/read_write.c
> index ff3c5e6f87cf..555dcaec00ac 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -20,6 +20,7 @@
>  #include <linux/compat.h>
>  #include <linux/mount.h>
>  #include <linux/fs.h>
> +#include <linux/vmalloc.h>
>  #include "internal.h"
> 
>  #include <linux/uaccess.h>
> @@ -1362,6 +1363,111 @@ COMPAT_SYSCALL_DEFINE6(pwritev2, compat_ulong_t, fd,
> 
>  #endif
> 
> +int kernel_read_file(struct file *file, void **buf, loff_t *size,
> +		     loff_t max_size, enum kernel_read_file_id id)
> +{
> +	loff_t i_size, pos;
> +	ssize_t bytes = 0;
> +	int ret;
> +
> +	if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
> +		return -EINVAL;
> +
> +	ret = deny_write_access(file);
> +	if (ret)
> +		return ret;
> +
> +	ret = security_kernel_read_file(file, id);
> +	if (ret)
> +		goto out;
> +
> +	i_size = i_size_read(file_inode(file));
> +	if (i_size <= 0) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
> +		ret = -EFBIG;
> +		goto out;
> +	}
> +
> +	if (id != READING_FIRMWARE_PREALLOC_BUFFER)
> +		*buf = vmalloc(i_size);
> +	if (!*buf) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	pos = 0;
> +	while (pos < i_size) {
> +		bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
> +		if (bytes < 0) {
> +			ret = bytes;
> +			goto out;
> +		}
> +
> +		if (bytes == 0)
> +			break;
> +	}
> +
> +	if (pos != i_size) {
> +		ret = -EIO;
> +		goto out_free;
> +	}
> +
> +	ret = security_kernel_post_read_file(file, *buf, i_size, id);
> +	if (!ret)
> +		*size = pos;
> +
> +out_free:
> +	if (ret < 0) {
> +		if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
> +			vfree(*buf);
> +			*buf = NULL;
> +		}
> +	}
> +
> +out:
> +	allow_write_access(file);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(kernel_read_file);
> +
> +int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
> +			       loff_t max_size, enum kernel_read_file_id id)
> +{
> +	struct file *file;
> +	int ret;
> +
> +	if (!path || !*path)
> +		return -EINVAL;
> +
> +	file = filp_open(path, O_RDONLY, 0);
> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +
> +	ret = kernel_read_file(file, buf, size, max_size, id);
> +	fput(file);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
> +
> +int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
> +			     enum kernel_read_file_id id)
> +{
> +	struct fd f = fdget(fd);
> +	int ret = -EBADF;
> +
> +	if (!f.file)
> +		goto out;
> +
> +	ret = kernel_read_file(f.file, buf, size, max_size, id);
> +out:
> +	fdput(f);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
> +
>  static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
>  		  	   size_t count, loff_t max)
>  {
>