switch in-kernel read/write calls to kernel_read/write

switch in-kernel read/write calls to kernel_read/write

[Christoph Hellwig]

Hi all,

this series moves almost all kernel callers of vfs_read/write and
friends to the kernel_read/write family that includes the set_fs calls
inside the called routines.  The exceptions are the integrity code
(Mimi has a pending series for that one) and the splice readv code
which doesn't fit those helpers.  But at least no modular users
are left.

[PATCH 01/16] ashmem: switch to ->read_iter

[Christoph Hellwig]

And use the proper VFS helper for using the backing file.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/staging/android/ashmem.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 6ba270e0494d..b3ebbb71fd3e 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -294,19 +294,9 @@ static int ashmem_release(struct inode *ignored, struct file *file)
 	return 0;
 }
 
-/**
- * ashmem_read() - Reads a set of bytes from an Ashmem-enabled file
- * @file:	   The associated backing file.
- * @buf:	   The buffer of data being written to
- * @len:	   The number of bytes being read
- * @pos:	   The position of the first byte to read.
- *
- * Return: 0 if successful, or another return code if not.
- */
-static ssize_t ashmem_read(struct file *file, char __user *buf,
-			   size_t len, loff_t *pos)
+static ssize_t ashmem_read_iter(struct kiocb *iocb, struct iov_iter *iter)
 {
-	struct ashmem_area *asma = file->private_data;
+	struct ashmem_area *asma = iocb->ki_filp->private_data;
 	int ret = 0;
 
 	mutex_lock(&ashmem_mutex);
@@ -328,10 +318,9 @@ static ssize_t ashmem_read(struct file *file, char __user *buf,
 	 * be destroyed until all references to the file are dropped and
 	 * ashmem_release is called.
 	 */
-	ret = __vfs_read(asma->file, buf, len, pos);
-	if (ret >= 0)
-		/** Update backing file pos, since f_ops->read() doesn't */
-		asma->file->f_pos = *pos;
+	ret = vfs_iter_read(asma->file, iter, &iocb->ki_pos, 0);
+	if (ret > 0)
+		asma->file->f_pos = iocb->ki_pos;
 	return ret;
 
 out_unlock:
@@ -834,7 +823,7 @@ static const struct file_operations ashmem_fops = {
 	.owner = THIS_MODULE,
 	.open = ashmem_open,
 	.release = ashmem_release,
-	.read = ashmem_read,
+	.read_iter = ashmem_read_iter,
 	.llseek = ashmem_llseek,
 	.mmap = ashmem_mmap,
 	.unlocked_ioctl = ashmem_ioctl,
-- 
2.11.0

Re: [PATCH 01/16] ashmem: switch to ->read_iter

[Al Viro]

On Wed, Aug 30, 2017 at 04:59:52PM +0200, Christoph Hellwig wrote:
> And use the proper VFS helper for using the backing file.

Actually, I wonder if that update of ->f_pos of the underlying file is
correct.  That's unrelated to this patch, but might be worth sorting
out while we are at it.

How about
        mutex_unlock(&ashmem_mutex);
        /*
         * asma and asma->file are used outside the lock here.  We assume
         * once asma->file is set it will never be changed, and will not
         * be destroyed until all references to the file are dropped and
         * ashmem_release is called.
         */
	ret = vfs_iter_read(asma->file, iter, &iocb->ki_pos, 0);
        mutex_lock(&ashmem_mutex);
        if (ret >= 0)
                asma->file->f_pos = *pos;
out_unlock:
        mutex_unlock(&ashmem_mutex);
        return ret;

in there?

Re: [PATCH 01/16] ashmem: switch to ->read_iter

[Christoph Hellwig]

On Wed, Aug 30, 2017 at 04:28:55PM +0100, Al Viro wrote:
> On Wed, Aug 30, 2017 at 04:59:52PM +0200, Christoph Hellwig wrote:
> > And use the proper VFS helper for using the backing file.
> 
> Actually, I wonder if that update of ->f_pos of the underlying file is
> correct.  That's unrelated to this patch, but might be worth sorting
> out while we are at it.

Sure, I'll add that.  If ashmem_mutex wasn't a global lock we could
just as easily keep it locked, but I'll leave that to the android
folks.

[PATCH 02/16] autofs4: switch autofs4_write to __kernel_write

[Christoph Hellwig]

Instead of playing games with the address limit..

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/autofs4/waitq.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index 24a58bf9ca72..4ac49d038bf3 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -56,19 +56,14 @@ static int autofs4_write(struct autofs_sb_info *sbi,
 			 struct file *file, const void *addr, int bytes)
 {
 	unsigned long sigpipe, flags;
-	mm_segment_t fs;
 	const char *data = (const char *)addr;
 	ssize_t wr = 0;
 
 	sigpipe = sigismember(&current->pending.signal, SIGPIPE);
 
-	/* Save pointer to user space and point back to kernel space */
-	fs = get_fs();
-	set_fs(KERNEL_DS);
-
 	mutex_lock(&sbi->pipe_mutex);
 	while (bytes) {
-		wr = __vfs_write(file, data, bytes, &file->f_pos);
+		wr = __kernel_write(file, data, bytes, &file->f_pos);
 		if (wr <= 0)
 			break;
 		data += wr;
@@ -76,8 +71,6 @@ static int autofs4_write(struct autofs_sb_info *sbi,
 	}
 	mutex_unlock(&sbi->pipe_mutex);
 
-	set_fs(fs);
-
 	/* Keep the currently executing process from receiving a
 	 * SIGPIPE unless it was already supposed to get one
 	 */
-- 
2.11.0

Re: [PATCH 02/16] autofs4: switch autofs4_write to __kernel_write

[Al Viro]

On Wed, Aug 30, 2017 at 04:59:53PM +0200, Christoph Hellwig wrote:
> Instead of playing games with the address limit..

I wonder...  Do we verify that it's a pipe?  If so, ->f_pos can
(and should) be left alone...

Re: [PATCH 02/16] autofs4: switch autofs4_write to __kernel_write

[Christoph Hellwig]

On Wed, Aug 30, 2017 at 04:30:26PM +0100, Al Viro wrote:
> On Wed, Aug 30, 2017 at 04:59:53PM +0200, Christoph Hellwig wrote:
> > Instead of playing games with the address limit..
> 
> I wonder...  Do we verify that it's a pipe?  If so, ->f_pos can
> (and should) be left alone...

currently nothing in autofs_dev_ioctl_setpipefd verifies that
it's a pipe.

[PATCH 03/16] fs: move kernel_write to fs/read_write.c

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/read_write.c | 17 ++++++++++++++++-
 fs/splice.c     | 16 ----------------
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 0cc7033aa413..417dbe199505 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -496,9 +496,24 @@ ssize_t __kernel_write(struct file *file, const char *buf, size_t count, loff_t
 	inc_syscw(current);
 	return ret;
 }
-
 EXPORT_SYMBOL(__kernel_write);
 
+ssize_t kernel_write(struct file *file, const char *buf, size_t count,
+			    loff_t pos)
+{
+	mm_segment_t old_fs;
+	ssize_t res;
+
+	old_fs = get_fs();
+	set_fs(get_ds());
+	/* The cast to a user pointer is valid due to the set_fs() */
+	res = vfs_write(file, (__force const char __user *)buf, count, &pos);
+	set_fs(old_fs);
+
+	return res;
+}
+EXPORT_SYMBOL(kernel_write);
+
 ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_t *pos)
 {
 	ssize_t ret;
diff --git a/fs/splice.c b/fs/splice.c
index ae41201d0325..f3084cce0ea6 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -364,22 +364,6 @@ static ssize_t kernel_readv(struct file *file, const struct kvec *vec,
 	return res;
 }
 
-ssize_t kernel_write(struct file *file, const char *buf, size_t count,
-			    loff_t pos)
-{
-	mm_segment_t old_fs;
-	ssize_t res;
-
-	old_fs = get_fs();
-	set_fs(get_ds());
-	/* The cast to a user pointer is valid due to the set_fs() */
-	res = vfs_write(file, (__force const char __user *)buf, count, &pos);
-	set_fs(old_fs);
-
-	return res;
-}
-EXPORT_SYMBOL(kernel_write);
-
 static ssize_t default_file_splice_read(struct file *in, loff_t *ppos,
 				 struct pipe_inode_info *pipe, size_t len,
 				 unsigned int flags)
-- 
2.11.0

[PATCH 04/16] fs: move kernel_read to fs/read_write.c

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/exec.c       | 17 -----------------
 fs/read_write.c | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 62175cbcc801..8adcc5eaa175 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -885,23 +885,6 @@ struct file *open_exec(const char *name)
 }
 EXPORT_SYMBOL(open_exec);
 
-int kernel_read(struct file *file, loff_t offset,
-		char *addr, unsigned long count)
-{
-	mm_segment_t old_fs;
-	loff_t pos = offset;
-	int result;
-
-	old_fs = get_fs();
-	set_fs(get_ds());
-	/* The cast to a user pointer is valid due to the set_fs() */
-	result = vfs_read(file, (void __user *)addr, count, &pos);
-	set_fs(old_fs);
-	return result;
-}
-
-EXPORT_SYMBOL(kernel_read);
-
 int kernel_read_file(struct file *file, void **buf, loff_t *size,
 		     loff_t max_size, enum kernel_read_file_id id)
 {
diff --git a/fs/read_write.c b/fs/read_write.c
index 417dbe199505..1ea862bc7efd 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -415,6 +415,22 @@ ssize_t __vfs_read(struct file *file, char __user *buf, size_t count,
 }
 EXPORT_SYMBOL(__vfs_read);
 
+int kernel_read(struct file *file, loff_t offset, char *addr,
+		unsigned long count)
+{
+	mm_segment_t old_fs;
+	loff_t pos = offset;
+	int result;
+
+	old_fs = get_fs();
+	set_fs(get_ds());
+	/* The cast to a user pointer is valid due to the set_fs() */
+	result = vfs_read(file, (void __user *)addr, count, &pos);
+	set_fs(old_fs);
+	return result;
+}
+EXPORT_SYMBOL(kernel_read);
+
 ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
 {
 	ssize_t ret;
-- 
2.11.0

[PATCH 05/16] fs: fix kernel_read prototype

[Christoph Hellwig]

Use proper ssize_t and size_t types for the return value and count
argument,  move the offset last and make it an in/out argument like
all other read/write helpers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/mips/kernel/elf.c                             | 12 ++++--------
 arch/x86/ia32/ia32_aout.c                          |  4 ++--
 drivers/media/pci/cx25821/cx25821-audio-upstream.c |  8 ++------
 drivers/mtd/nand/nandsim.c                         |  2 +-
 fs/binfmt_aout.c                                   |  3 ++-
 fs/binfmt_elf.c                                    | 22 +++++++++++++---------
 fs/binfmt_elf_fdpic.c                              | 17 +++++++++--------
 fs/binfmt_flat.c                                   | 18 +++++-------------
 fs/binfmt_misc.c                                   |  5 ++++-
 fs/coda/dir.c                                      |  5 +++--
 fs/ecryptfs/read_write.c                           |  2 +-
 fs/exec.c                                          |  8 ++++----
 fs/read_write.c                                    |  8 +++-----
 include/linux/fs.h                                 |  2 +-
 kernel/sysctl_binary.c                             | 12 ++++++++----
 net/9p/trans_fd.c                                  |  4 +++-
 security/keys/big_key.c                            |  3 ++-
 17 files changed, 67 insertions(+), 68 deletions(-)

diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
index 5c429d70e17f..290b8a12ad8c 100644
--- a/arch/mips/kernel/elf.c
+++ b/arch/mips/kernel/elf.c
@@ -87,6 +87,7 @@ int arch_elf_pt_proc(void *_ehdr, void *_phdr, struct file *elf,
 	bool elf32;
 	u32 flags;
 	int ret;
+	loff_t pos;
 
 	elf32 = ehdr->e32.e_ident[EI_CLASS] == ELFCLASS32;
 	flags = elf32 ? ehdr->e32.e_flags : ehdr->e64.e_flags;
@@ -108,21 +109,16 @@ int arch_elf_pt_proc(void *_ehdr, void *_phdr, struct file *elf,
 
 		if (phdr32->p_filesz < sizeof(abiflags))
 			return -EINVAL;
-
-		ret = kernel_read(elf, phdr32->p_offset,
-				  (char *)&abiflags,
-				  sizeof(abiflags));
+		pos = phdr32->p_offset;
 	} else {
 		if (phdr64->p_type != PT_MIPS_ABIFLAGS)
 			return 0;
 		if (phdr64->p_filesz < sizeof(abiflags))
 			return -EINVAL;
-
-		ret = kernel_read(elf, phdr64->p_offset,
-				  (char *)&abiflags,
-				  sizeof(abiflags));
+		pos = phdr64->p_offset;
 	}
 
+	ret = kernel_read(elf, (char *)&abiflags, sizeof(abiflags), &pos);
 	if (ret < 0)
 		return ret;
 	if (ret != sizeof(abiflags))
diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index 8d0879f1d42c..6ae2571afd40 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -407,10 +407,10 @@ static int load_aout_library(struct file *file)
 	unsigned long bss, start_addr, len, error;
 	int retval;
 	struct exec ex;
-
+	loff_t pos = 0;
 
 	retval = -ENOEXEC;
-	error = kernel_read(file, 0, (char *) &ex, sizeof(ex));
+	error = kernel_read(file, (char *) &ex, sizeof(ex), &pos);
 	if (error != sizeof(ex))
 		goto out;
 
diff --git a/drivers/media/pci/cx25821/cx25821-audio-upstream.c b/drivers/media/pci/cx25821/cx25821-audio-upstream.c
index b94eb1c0023d..ef1f6c423e6d 100644
--- a/drivers/media/pci/cx25821/cx25821-audio-upstream.c
+++ b/drivers/media/pci/cx25821/cx25821-audio-upstream.c
@@ -277,7 +277,7 @@ static int cx25821_get_audio_data(struct cx25821_dev *dev,
 		p = (char *)dev->_audiodata_buf_virt_addr + frame_offset;
 
 	for (i = 0; i < dev->_audio_lines_count; i++) {
-		int n = kernel_read(file, file_offset, mybuf, AUDIO_LINE_SIZE);
+		int n = kernel_read(file, mybuf, AUDIO_LINE_SIZE, &file_offset);
 		if (n < AUDIO_LINE_SIZE) {
 			pr_info("Done: exit %s() since no more bytes to read from Audio file\n",
 				__func__);
@@ -290,7 +290,6 @@ static int cx25821_get_audio_data(struct cx25821_dev *dev,
 			memcpy(p, mybuf, n);
 			p += n;
 		}
-		file_offset += n;
 	}
 	dev->_audioframe_count++;
 	fput(file);
@@ -331,8 +330,7 @@ static int cx25821_openfile_audio(struct cx25821_dev *dev,
 	for (j = 0, offset = 0; j < NUM_AUDIO_FRAMES; j++) {
 		for (i = 0; i < dev->_audio_lines_count; i++) {
 			char buf[AUDIO_LINE_SIZE];
-			int n = kernel_read(file, offset, buf,
-						AUDIO_LINE_SIZE);
+			int n = kernel_read(file, buf, AUDIO_LINE_SIZE, &offset);
 
 			if (n < AUDIO_LINE_SIZE) {
 				pr_info("Done: exit %s() since no more bytes to read from Audio file\n",
@@ -344,8 +342,6 @@ static int cx25821_openfile_audio(struct cx25821_dev *dev,
 
 			if (p)
 				memcpy(p + offset, buf, n);
-
-			offset += n;
 		}
 		dev->_audioframe_count++;
 	}
diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index e4211c3cc49b..a8089656879a 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -1379,7 +1379,7 @@ static ssize_t read_file(struct nandsim *ns, struct file *file, void *buf, size_
 	if (err)
 		return err;
 	noreclaim_flag = memalloc_noreclaim_save();
-	tx = kernel_read(file, pos, buf, count);
+	tx = kernel_read(file, buf, count, &pos);
 	memalloc_noreclaim_restore(noreclaim_flag);
 	put_pages(ns);
 	return tx;
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index 9be82c4e14a4..74e9893dbce6 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -341,11 +341,12 @@ static int load_aout_library(struct file *file)
 	unsigned long error;
 	int retval;
 	struct exec ex;
+	loff_t pos = 0;
 
 	inode = file_inode(file);
 
 	retval = -ENOEXEC;
-	error = kernel_read(file, 0, (char *) &ex, sizeof(ex));
+	error = kernel_read(file, (char *) &ex, sizeof(ex), &pos);
 	if (error != sizeof(ex))
 		goto out;
 
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 6466153f2bf0..42de02ab9d9e 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -409,6 +409,7 @@ static struct elf_phdr *load_elf_phdrs(struct elfhdr *elf_ex,
 {
 	struct elf_phdr *elf_phdata = NULL;
 	int retval, size, err = -1;
+	loff_t pos = elf_ex->e_phoff;
 
 	/*
 	 * If the size of this structure has changed, then punt, since
@@ -432,8 +433,7 @@ static struct elf_phdr *load_elf_phdrs(struct elfhdr *elf_ex,
 		goto out;
 
 	/* Read in the program headers */
-	retval = kernel_read(elf_file, elf_ex->e_phoff,
-			     (char *)elf_phdata, size);
+	retval = kernel_read(elf_file, (char *)elf_phdata, size, &pos);
 	if (retval != size) {
 		err = (retval < 0) ? retval : -EIO;
 		goto out;
@@ -698,6 +698,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
 		struct elfhdr interp_elf_ex;
 	} *loc;
 	struct arch_elf_state arch_state = INIT_ARCH_ELF_STATE;
+	loff_t pos;
 
 	loc = kmalloc(sizeof(*loc), GFP_KERNEL);
 	if (!loc) {
@@ -750,9 +751,9 @@ static int load_elf_binary(struct linux_binprm *bprm)
 			if (!elf_interpreter)
 				goto out_free_ph;
 
-			retval = kernel_read(bprm->file, elf_ppnt->p_offset,
-					     elf_interpreter,
-					     elf_ppnt->p_filesz);
+			pos = elf_ppnt->p_offset;
+			retval = kernel_read(bprm->file, elf_interpreter,
+					     elf_ppnt->p_filesz, &pos);
 			if (retval != elf_ppnt->p_filesz) {
 				if (retval >= 0)
 					retval = -EIO;
@@ -776,9 +777,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
 			would_dump(bprm, interpreter);
 
 			/* Get the exec headers */
-			retval = kernel_read(interpreter, 0,
+			pos = 0;
+			retval = kernel_read(interpreter,
 					     (void *)&loc->interp_elf_ex,
-					     sizeof(loc->interp_elf_ex));
+					     sizeof(loc->interp_elf_ex), &pos);
 			if (retval != sizeof(loc->interp_elf_ex)) {
 				if (retval >= 0)
 					retval = -EIO;
@@ -1175,9 +1177,10 @@ static int load_elf_library(struct file *file)
 	unsigned long elf_bss, bss, len;
 	int retval, error, i, j;
 	struct elfhdr elf_ex;
+	loff_t pos = 0;
 
 	error = -ENOEXEC;
-	retval = kernel_read(file, 0, (char *)&elf_ex, sizeof(elf_ex));
+	retval = kernel_read(file, (char *)&elf_ex, sizeof(elf_ex), &pos);
 	if (retval != sizeof(elf_ex))
 		goto out;
 
@@ -1201,7 +1204,8 @@ static int load_elf_library(struct file *file)
 
 	eppnt = elf_phdata;
 	error = -ENOEXEC;
-	retval = kernel_read(file, elf_ex.e_phoff, (char *)eppnt, j);
+	pos =  elf_ex.e_phoff;
+	retval = kernel_read(file, (char *)eppnt, j, &pos);
 	if (retval != j)
 		goto out_free_ph;
 
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index cf93a4fad012..f86222c82621 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -145,6 +145,7 @@ static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *params,
 	struct elf32_phdr *phdr;
 	unsigned long size;
 	int retval, loop;
+	loff_t pos = params->hdr.e_phoff;
 
 	if (params->hdr.e_phentsize != sizeof(struct elf_phdr))
 		return -ENOMEM;
@@ -156,8 +157,7 @@ static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *params,
 	if (!params->phdrs)
 		return -ENOMEM;
 
-	retval = kernel_read(file, params->hdr.e_phoff,
-			     (char *) params->phdrs, size);
+	retval = kernel_read(file, (char *) params->phdrs, size, &pos);
 	if (unlikely(retval != size))
 		return retval < 0 ? retval : -ENOEXEC;
 
@@ -199,6 +199,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
 	char *interpreter_name = NULL;
 	int executable_stack;
 	int retval, i;
+	loff_t pos;
 
 	kdebug("____ LOAD %d ____", current->pid);
 
@@ -246,10 +247,9 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
 			if (!interpreter_name)
 				goto error;
 
-			retval = kernel_read(bprm->file,
-					     phdr->p_offset,
-					     interpreter_name,
-					     phdr->p_filesz);
+			pos = phdr->p_offset;
+			retval = kernel_read(bprm->file, interpreter_name,
+					     phdr->p_filesz, &pos);
 			if (unlikely(retval != phdr->p_filesz)) {
 				if (retval >= 0)
 					retval = -ENOEXEC;
@@ -277,8 +277,9 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
 			 */
 			would_dump(bprm, interpreter);
 
-			retval = kernel_read(interpreter, 0, bprm->buf,
-					     BINPRM_BUF_SIZE);
+			pos = 0;
+			retval = kernel_read(interpreter, bprm->buf,
+					BINPRM_BUF_SIZE, &pos);
 			if (unlikely(retval != BINPRM_BUF_SIZE)) {
 				if (retval >= 0)
 					retval = -ENOEXEC;
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index a1e6860b6f46..afb7e9d521d2 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -176,19 +176,14 @@ static int create_flat_tables(struct linux_binprm *bprm, unsigned long arg_start
 #define ENCRYPTED    0x20 /* bit 5 set: file is encrypted */
 #define RESERVED     0xC0 /* bit 6,7:   reserved */
 
-static int decompress_exec(
-	struct linux_binprm *bprm,
-	unsigned long offset,
-	char *dst,
-	long len,
-	int fd)
+static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst,
+		long len, int fd)
 {
 	unsigned char *buf;
 	z_stream strm;
-	loff_t fpos;
 	int ret, retval;
 
-	pr_debug("decompress_exec(offset=%lx,buf=%p,len=%lx)\n", offset, dst, len);
+	pr_debug("decompress_exec(offset=%llx,buf=%p,len=%lx)\n", fpos, dst, len);
 
 	memset(&strm, 0, sizeof(strm));
 	strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
@@ -204,13 +199,11 @@ static int decompress_exec(
 	}
 
 	/* Read in first chunk of data and parse gzip header. */
-	fpos = offset;
-	ret = kernel_read(bprm->file, offset, buf, LBUFSIZE);
+	ret = kernel_read(bprm->file, buf, LBUFSIZE, &fpos);
 
 	strm.next_in = buf;
 	strm.avail_in = ret;
 	strm.total_in = 0;
-	fpos += ret;
 
 	retval = -ENOEXEC;
 
@@ -276,7 +269,7 @@ static int decompress_exec(
 	}
 
 	while ((ret = zlib_inflate(&strm, Z_NO_FLUSH)) == Z_OK) {
-		ret = kernel_read(bprm->file, fpos, buf, LBUFSIZE);
+		ret = kernel_read(bprm->file, buf, LBUFSIZE, &fpos);
 		if (ret <= 0)
 			break;
 		len -= ret;
@@ -284,7 +277,6 @@ static int decompress_exec(
 		strm.next_in = buf;
 		strm.avail_in = ret;
 		strm.total_in = 0;
-		fpos += ret;
 	}
 
 	if (ret < 0) {
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index f4718098ac31..ce7181ea60fa 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -218,12 +218,15 @@ static int load_misc_binary(struct linux_binprm *bprm)
 
 	bprm->file = interp_file;
 	if (fmt->flags & MISC_FMT_CREDENTIALS) {
+		loff_t pos = 0;
+
 		/*
 		 * No need to call prepare_binprm(), it's already been
 		 * done.  bprm->buf is stale, update from interp_file.
 		 */
 		memset(bprm->buf, 0, BINPRM_BUF_SIZE);
-		retval = kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
+		retval = kernel_read(bprm->file, bprm->buf, BINPRM_BUF_SIZE,
+				&pos);
 	} else
 		retval = prepare_binprm(bprm);
 
diff --git a/fs/coda/dir.c b/fs/coda/dir.c
index c0474ac6cbf2..86048b76b6d8 100644
--- a/fs/coda/dir.c
+++ b/fs/coda/dir.c
@@ -368,9 +368,10 @@ static int coda_venus_readdir(struct file *coda_file, struct dir_context *ctx)
 		goto out;
 
 	while (1) {
+		loff_t pos = ctx->pos - 2;
+
 		/* read entries from the directory file */
-		ret = kernel_read(host_file, ctx->pos - 2, (char *)vdir,
-				  sizeof(*vdir));
+		ret = kernel_read(host_file, (char *)vdir, sizeof(*vdir), &pos);
 		if (ret < 0) {
 			pr_err("%s: read dir %s failed %d\n",
 			       __func__, coda_f2s(&cii->c_fid), ret);
diff --git a/fs/ecryptfs/read_write.c b/fs/ecryptfs/read_write.c
index 039e627194a9..d8af0e99bfaf 100644
--- a/fs/ecryptfs/read_write.c
+++ b/fs/ecryptfs/read_write.c
@@ -237,7 +237,7 @@ int ecryptfs_read_lower(char *data, loff_t offset, size_t size,
 	lower_file = ecryptfs_inode_to_private(ecryptfs_inode)->lower_file;
 	if (!lower_file)
 		return -EIO;
-	return kernel_read(lower_file, offset, data, size);
+	return kernel_read(lower_file, data, size, &offset);
 }
 
 /**
diff --git a/fs/exec.c b/fs/exec.c
index 8adcc5eaa175..8d642e4f9857 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -922,8 +922,8 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 
 	pos = 0;
 	while (pos < i_size) {
-		bytes = kernel_read(file, pos, (char *)(*buf) + pos,
-				    i_size - pos);
+		bytes = kernel_read(file, (char *)(*buf) + pos,
+				    i_size - pos, &pos);
 		if (bytes < 0) {
 			ret = bytes;
 			goto out;
@@ -931,7 +931,6 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
 
 		if (bytes == 0)
 			break;
-		pos += bytes;
 	}
 
 	if (pos != i_size) {
@@ -1524,6 +1523,7 @@ static void bprm_fill_uid(struct linux_binprm *bprm)
 int prepare_binprm(struct linux_binprm *bprm)
 {
 	int retval;
+	loff_t pos = 0;
 
 	bprm_fill_uid(bprm);
 
@@ -1534,7 +1534,7 @@ int prepare_binprm(struct linux_binprm *bprm)
 	bprm->cred_prepared = 1;
 
 	memset(bprm->buf, 0, BINPRM_BUF_SIZE);
-	return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
+	return kernel_read(bprm->file, bprm->buf, BINPRM_BUF_SIZE, &pos);
 }
 
 EXPORT_SYMBOL(prepare_binprm);
diff --git a/fs/read_write.c b/fs/read_write.c
index 1ea862bc7efd..1966fd6e03ce 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -415,17 +415,15 @@ ssize_t __vfs_read(struct file *file, char __user *buf, size_t count,
 }
 EXPORT_SYMBOL(__vfs_read);
 
-int kernel_read(struct file *file, loff_t offset, char *addr,
-		unsigned long count)
+ssize_t kernel_read(struct file *file, char *buf, size_t count, loff_t *pos)
 {
 	mm_segment_t old_fs;
-	loff_t pos = offset;
-	int result;
+	ssize_t result;
 
 	old_fs = get_fs();
 	set_fs(get_ds());
 	/* The cast to a user pointer is valid due to the set_fs() */
-	result = vfs_read(file, (void __user *)addr, count, &pos);
+	result = vfs_read(file, (void __user *)buf, count, pos);
 	set_fs(old_fs);
 	return result;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index cbfe127bccf8..1b9d083adf94 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2772,13 +2772,13 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
 	return kernel_read_file_str[id];
 }
 
-extern int kernel_read(struct file *, loff_t, char *, unsigned long);
 extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
 			    enum kernel_read_file_id);
 extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t,
 				      enum kernel_read_file_id);
 extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t,
 				    enum kernel_read_file_id);
+extern ssize_t kernel_read(struct file *, char *, size_t, loff_t *);
 extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 extern struct file * open_exec(const char *);
diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 02e1859f2ca8..243fa1c28b4a 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -986,8 +986,9 @@ static ssize_t bin_intvec(struct file *file,
 		size_t length = oldlen / sizeof(*vec);
 		char *str, *end;
 		int i;
+		loff_t pos = 0;
 
-		result = kernel_read(file, 0, buffer, BUFSZ - 1);
+		result = kernel_read(file, buffer, BUFSZ - 1, &pos);
 		if (result < 0)
 			goto out_kfree;
 
@@ -1057,8 +1058,9 @@ static ssize_t bin_ulongvec(struct file *file,
 		size_t length = oldlen / sizeof(*vec);
 		char *str, *end;
 		int i;
+		loff_t pos = 0;
 
-		result = kernel_read(file, 0, buffer, BUFSZ - 1);
+		result = kernel_read(file, buffer, BUFSZ - 1, &pos);
 		if (result < 0)
 			goto out_kfree;
 
@@ -1120,8 +1122,9 @@ static ssize_t bin_uuid(struct file *file,
 	if (oldval && oldlen) {
 		char buf[UUID_STRING_LEN + 1];
 		uuid_t uuid;
+		loff_t pos = 0;
 
-		result = kernel_read(file, 0, buf, sizeof(buf) - 1);
+		result = kernel_read(file, buf, sizeof(buf) - 1, &pos);
 		if (result < 0)
 			goto out;
 
@@ -1154,8 +1157,9 @@ static ssize_t bin_dn_node_address(struct file *file,
 		char buf[15], *nodep;
 		unsigned long area, node;
 		__le16 dnaddr;
+		loff_t pos = 0;
 
-		result = kernel_read(file, 0, buf, sizeof(buf) - 1);
+		result = kernel_read(file, buf, sizeof(buf) - 1, &pos);
 		if (result < 0)
 			goto out;
 
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index ddfa86648f95..f12815777beb 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -272,6 +272,7 @@ static int p9_fd_read(struct p9_client *client, void *v, int len)
 {
 	int ret;
 	struct p9_trans_fd *ts = NULL;
+	loff_t pos;
 
 	if (client && client->status != Disconnected)
 		ts = client->trans;
@@ -282,7 +283,8 @@ static int p9_fd_read(struct p9_client *client, void *v, int len)
 	if (!(ts->rd->f_flags & O_NONBLOCK))
 		p9_debug(P9_DEBUG_ERROR, "blocking read ...\n");
 
-	ret = kernel_read(ts->rd, ts->rd->f_pos, v, len);
+	pos = ts->rd->f_pos;
+	ret = kernel_read(ts->rd, v, len, &pos);
 	if (ret <= 0 && ret != -ERESTARTSYS && ret != -EAGAIN)
 		client->status = Disconnected;
 	return ret;
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 835c1ab30d01..9f4c86cade8e 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -295,6 +295,7 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen)
 		u8 *data;
 		u8 *enckey = (u8 *)key->payload.data[big_key_data];
 		size_t enclen = ALIGN(datalen, crypto_skcipher_blocksize(big_key_skcipher));
+		loff_t pos = 0;
 
 		data = kmalloc(enclen, GFP_KERNEL);
 		if (!data)
@@ -307,7 +308,7 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen)
 		}
 
 		/* read file to kernel and decrypt */
-		ret = kernel_read(file, 0, data, enclen);
+		ret = kernel_read(file, data, enclen, &pos);
 		if (ret >= 0 && ret != enclen) {
 			ret = -EIO;
 			goto err_fput;
-- 
2.11.0

Re: [PATCH 05/16] fs: fix kernel_read prototype

[Al Viro]

On Wed, Aug 30, 2017 at 04:59:56PM +0200, Christoph Hellwig wrote:
> Use proper ssize_t and size_t types for the return value and count
> argument,  move the offset last and make it an in/out argument like
> all other read/write helpers.

Might be better to switch the third argument to void * at the same time
and lose those casts.

>  			if (p)
>  				memcpy(p + offset, buf, n);
> -
> -			offset += n;

Almost certainly broken - in effect, you've taken the update of offset
several lines prior, so that memcpy() is getting the wrong first argument.
The same needs to be watched out for in other similar places.

Re: [PATCH 05/16] fs: fix kernel_read prototype

[Christoph Hellwig]

On Wed, Aug 30, 2017 at 04:37:04PM +0100, Al Viro wrote:
> On Wed, Aug 30, 2017 at 04:59:56PM +0200, Christoph Hellwig wrote:
> > Use proper ssize_t and size_t types for the return value and count
> > argument,  move the offset last and make it an in/out argument like
> > all other read/write helpers.
> 
> Might be better to switch the third argument to void * at the same time
> and lose those casts.

Good point, updated.

> 
> >  			if (p)
> >  				memcpy(p + offset, buf, n);
> > -
> > -			offset += n;
> 
> Almost certainly broken - in effect, you've taken the update of offset
> several lines prior, so that memcpy() is getting the wrong first argument.
> The same needs to be watched out for in other similar places.

Fixed.  I did an audit and didn't find any others either for kernel_read
or kernel_write.

[PATCH 06/16] fs: fix kernel_write prototype

[Christoph Hellwig]

Make the position an in/out argument like all the other read/write
helpers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/mtd/nand/nandsim.c        | 2 +-
 drivers/target/target_core_alua.c | 3 ++-
 drivers/target/target_core_file.c | 3 +--
 drivers/target/target_core_pr.c   | 3 ++-
 fs/ecryptfs/read_write.c          | 2 +-
 fs/read_write.c                   | 4 ++--
 include/linux/fs.h                | 2 +-
 kernel/sysctl_binary.c            | 9 ++++++---
 security/keys/big_key.c           | 3 ++-
 9 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index a8089656879a..3300a77667fb 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -1395,7 +1395,7 @@ static ssize_t write_file(struct nandsim *ns, struct file *file, void *buf, size
 	if (err)
 		return err;
 	noreclaim_flag = memalloc_noreclaim_save();
-	tx = kernel_write(file, buf, count, pos);
+	tx = kernel_write(file, buf, count, &pos);
 	memalloc_noreclaim_restore(noreclaim_flag);
 	put_pages(ns);
 	return tx;
diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index a91b7c25ffd4..928127642574 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -896,13 +896,14 @@ static int core_alua_write_tpg_metadata(
 	u32 md_buf_len)
 {
 	struct file *file = filp_open(path, O_RDWR | O_CREAT | O_TRUNC, 0600);
+	loff_t pos = 0;
 	int ret;
 
 	if (IS_ERR(file)) {
 		pr_err("filp_open(%s) for ALUA metadata failed\n", path);
 		return -ENODEV;
 	}
-	ret = kernel_write(file, md_buf, md_buf_len, 0);
+	ret = kernel_write(file, md_buf, md_buf_len, &pos);
 	if (ret < 0)
 		pr_err("Error writing ALUA metadata file: %s\n", path);
 	fput(file);
diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 24cf11d9e50a..21e65d69ab2e 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -443,13 +443,12 @@ fd_do_prot_fill(struct se_device *se_dev, sector_t lba, sector_t nolb,
 
 	for (prot = 0; prot < prot_length;) {
 		sector_t len = min_t(sector_t, bufsize, prot_length - prot);
-		ssize_t ret = kernel_write(prot_fd, buf, len, pos + prot);
+		ssize_t ret = kernel_write(prot_fd, buf, len, &pos);
 
 		if (ret != len) {
 			pr_err("vfs_write to prot file failed: %zd\n", ret);
 			return ret < 0 ? ret : -ENODEV;
 		}
-		prot += ret;
 	}
 
 	return 0;
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 6d5def64db61..dd2cd8048582 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -1974,6 +1974,7 @@ static int __core_scsi3_write_aptpl_to_file(
 	char path[512];
 	u32 pr_aptpl_buf_len;
 	int ret;
+	loff_t pos = 0;
 
 	memset(path, 0, 512);
 
@@ -1993,7 +1994,7 @@ static int __core_scsi3_write_aptpl_to_file(
 
 	pr_aptpl_buf_len = (strlen(buf) + 1); /* Add extra for NULL */
 
-	ret = kernel_write(file, buf, pr_aptpl_buf_len, 0);
+	ret = kernel_write(file, buf, pr_aptpl_buf_len, &pos);
 
 	if (ret < 0)
 		pr_debug("Error writing APTPL metadata file: %s\n", path);
diff --git a/fs/ecryptfs/read_write.c b/fs/ecryptfs/read_write.c
index d8af0e99bfaf..c596e7c03424 100644
--- a/fs/ecryptfs/read_write.c
+++ b/fs/ecryptfs/read_write.c
@@ -47,7 +47,7 @@ int ecryptfs_write_lower(struct inode *ecryptfs_inode, char *data,
 	lower_file = ecryptfs_inode_to_private(ecryptfs_inode)->lower_file;
 	if (!lower_file)
 		return -EIO;
-	rc = kernel_write(lower_file, data, size, offset);
+	rc = kernel_write(lower_file, data, size, &offset);
 	mark_inode_dirty_sync(ecryptfs_inode);
 	return rc;
 }
diff --git a/fs/read_write.c b/fs/read_write.c
index 1966fd6e03ce..82f0111b98cf 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -513,7 +513,7 @@ ssize_t __kernel_write(struct file *file, const char *buf, size_t count, loff_t
 EXPORT_SYMBOL(__kernel_write);
 
 ssize_t kernel_write(struct file *file, const char *buf, size_t count,
-			    loff_t pos)
+			    loff_t *pos)
 {
 	mm_segment_t old_fs;
 	ssize_t res;
@@ -521,7 +521,7 @@ ssize_t kernel_write(struct file *file, const char *buf, size_t count,
 	old_fs = get_fs();
 	set_fs(get_ds());
 	/* The cast to a user pointer is valid due to the set_fs() */
-	res = vfs_write(file, (__force const char __user *)buf, count, &pos);
+	res = vfs_write(file, (__force const char __user *)buf, count, pos);
 	set_fs(old_fs);
 
 	return res;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1b9d083adf94..86f50568b758 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2779,7 +2779,7 @@ extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t,
 extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t,
 				    enum kernel_read_file_id);
 extern ssize_t kernel_read(struct file *, char *, size_t, loff_t *);
-extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
+extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t *);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 extern struct file * open_exec(const char *);
  
diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 243fa1c28b4a..58ea8c03662e 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -1017,6 +1017,7 @@ static ssize_t bin_intvec(struct file *file,
 		size_t length = newlen / sizeof(*vec);
 		char *str, *end;
 		int i;
+		loff_t pos = 0;
 
 		str = buffer;
 		end = str + BUFSZ;
@@ -1030,7 +1031,7 @@ static ssize_t bin_intvec(struct file *file,
 			str += scnprintf(str, end - str, "%lu\t", value);
 		}
 
-		result = kernel_write(file, buffer, str - buffer, 0);
+		result = kernel_write(file, buffer, str - buffer, &pos);
 		if (result < 0)
 			goto out_kfree;
 	}
@@ -1089,6 +1090,7 @@ static ssize_t bin_ulongvec(struct file *file,
 		size_t length = newlen / sizeof(*vec);
 		char *str, *end;
 		int i;
+		loff_t pos = 0;
 
 		str = buffer;
 		end = str + BUFSZ;
@@ -1102,7 +1104,7 @@ static ssize_t bin_ulongvec(struct file *file,
 			str += scnprintf(str, end - str, "%lu\t", value);
 		}
 
-		result = kernel_write(file, buffer, str - buffer, 0);
+		result = kernel_write(file, buffer, str - buffer, &pos);
 		if (result < 0)
 			goto out_kfree;
 	}
@@ -1192,6 +1194,7 @@ static ssize_t bin_dn_node_address(struct file *file,
 		__le16 dnaddr;
 		char buf[15];
 		int len;
+		loff_t pos = 0;
 
 		result = -EINVAL;
 		if (newlen != sizeof(dnaddr))
@@ -1205,7 +1208,7 @@ static ssize_t bin_dn_node_address(struct file *file,
 				le16_to_cpu(dnaddr) >> 10,
 				le16_to_cpu(dnaddr) & 0x3ff);
 
-		result = kernel_write(file, buf, len, 0);
+		result = kernel_write(file, buf, len, &pos);
 		if (result < 0)
 			goto out;
 	}
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 9f4c86cade8e..6acb00f6f22c 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -147,6 +147,7 @@ int big_key_preparse(struct key_preparsed_payload *prep)
 		 * File content is stored encrypted with randomly generated key.
 		 */
 		size_t enclen = ALIGN(datalen, crypto_skcipher_blocksize(big_key_skcipher));
+		loff_t pos = 0;
 
 		/* prepare aligned data to encrypt */
 		data = kmalloc(enclen, GFP_KERNEL);
@@ -179,7 +180,7 @@ int big_key_preparse(struct key_preparsed_payload *prep)
 			goto err_enckey;
 		}
 
-		written = kernel_write(file, data, enclen, 0);
+		written = kernel_write(file, data, enclen, &pos);
 		if (written != enclen) {
 			ret = written;
 			if (written >= 0)
-- 
2.11.0

Re: [PATCH 06/16] fs: fix kernel_write prototype

[Al Viro]

On Wed, Aug 30, 2017 at 04:59:57PM +0200, Christoph Hellwig wrote:

>  	for (prot = 0; prot < prot_length;) {
>  		sector_t len = min_t(sector_t, bufsize, prot_length - prot);
> -		ssize_t ret = kernel_write(prot_fd, buf, len, pos + prot);
> +		ssize_t ret = kernel_write(prot_fd, buf, len, &pos);
>  
>  		if (ret != len) {
>  			pr_err("vfs_write to prot file failed: %zd\n", ret);
>  			return ret < 0 ? ret : -ENODEV;
>  		}
> -		prot += ret;
>  	}

And that loop will terminate because of...?  IOW, the same bug class as in
the kernel_read() patch.

Re: [PATCH 06/16] fs: fix kernel_write prototype

[Christoph Hellwig]

On Wed, Aug 30, 2017 at 04:39:00PM +0100, Al Viro wrote:
> On Wed, Aug 30, 2017 at 04:59:57PM +0200, Christoph Hellwig wrote:
> 
> >  	for (prot = 0; prot < prot_length;) {
> >  		sector_t len = min_t(sector_t, bufsize, prot_length - prot);
> > -		ssize_t ret = kernel_write(prot_fd, buf, len, pos + prot);
> > +		ssize_t ret = kernel_write(prot_fd, buf, len, &pos);
> >  
> >  		if (ret != len) {
> >  			pr_err("vfs_write to prot file failed: %zd\n", ret);
> >  			return ret < 0 ? ret : -ENODEV;
> >  		}
> > -		prot += ret;
> >  	}
> 
> And that loop will terminate because of...?  IOW, the same bug class as in
> the kernel_read() patch.

Also fixed.

[PATCH 07/16] serial2002: switch serial2002_tty_write to kernel_{read/write}

[Christoph Hellwig]

Instead of playing games with the address limit.  This also gains
us proper usage of the write counter, time stamp updates and kvec
validation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/staging/comedi/drivers/serial2002.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/drivers/serial2002.c b/drivers/staging/comedi/drivers/serial2002.c
index 0d33e520f635..cc18e25103ca 100644
--- a/drivers/staging/comedi/drivers/serial2002.c
+++ b/drivers/staging/comedi/drivers/serial2002.c
@@ -106,16 +106,8 @@ static long serial2002_tty_ioctl(struct file *f, unsigned int op,
 
 static int serial2002_tty_write(struct file *f, unsigned char *buf, int count)
 {
-	const char __user *p = (__force const char __user *)buf;
-	int result;
-	loff_t offset = 0;
-	mm_segment_t oldfs;
-
-	oldfs = get_fs();
-	set_fs(KERNEL_DS);
-	result = __vfs_write(f, p, count, &offset);
-	set_fs(oldfs);
-	return result;
+	loff_t pos = 0;
+	return kernel_write(f, buf, count, &pos);
 }
 
 static void serial2002_tty_read_poll_wait(struct file *f, int timeout)
@@ -148,19 +140,14 @@ static int serial2002_tty_read(struct file *f, int timeout)
 {
 	unsigned char ch;
 	int result;
+	loff_t pos = 0;
 
 	result = -1;
 	if (!IS_ERR(f)) {
-		mm_segment_t oldfs;
-		char __user *p = (__force char __user *)&ch;
-		loff_t offset = 0;
-
-		oldfs = get_fs();
-		set_fs(KERNEL_DS);
 		if (f->f_op->poll) {
 			serial2002_tty_read_poll_wait(f, timeout);
 
-			if (__vfs_read(f, p, 1, &offset) == 1)
+			if (kernel_read(f, &ch, 1, &pos) == 1)
 				result = ch;
 		} else {
 			/* Device does not support poll, busy wait */
@@ -171,14 +158,13 @@ static int serial2002_tty_read(struct file *f, int timeout)
 				if (retries >= timeout)
 					break;
 
-				if (__vfs_read(f, p, 1, &offset) == 1) {
+				if (kernel_read(f, &ch, 1, &pos) == 1) {
 					result = ch;
 					break;
 				}
 				usleep_range(100, 1000);
 			}
 		}
-		set_fs(oldfs);
 	}
 	return result;
 }
-- 
2.11.0

[PATCH 08/16] mm/nommu: switch do_mmap_private to kernel_read

[Christoph Hellwig]

Instead of playing with the address limit.  This also gains us
validation of the kvec and proper atime updates.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 mm/nommu.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/mm/nommu.c b/mm/nommu.c
index fc184f597d59..379f1022bdea 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1164,17 +1164,8 @@ static int do_mmap_private(struct vm_area_struct *vma,
 
 	if (vma->vm_file) {
 		/* read the contents of a file into the copy */
-		mm_segment_t old_fs;
-		loff_t fpos;
-
-		fpos = vma->vm_pgoff;
-		fpos <<= PAGE_SHIFT;
-
-		old_fs = get_fs();
-		set_fs(KERNEL_DS);
-		ret = __vfs_read(vma->vm_file, base, len, &fpos);
-		set_fs(old_fs);
-
+		loff_t fpos = vma->vm_pgoff << PAGE_SHIFT;
+		ret = kernel_read(vma->vm_file, base, len, &fpos);
 		if (ret < 0)
 			goto error_free;
 
-- 
2.11.0

Re: [PATCH 08/16] mm/nommu: switch do_mmap_private to kernel_read

[Al Viro]

On Wed, Aug 30, 2017 at 04:59:59PM +0200, Christoph Hellwig wrote:
> -		fpos = vma->vm_pgoff;
> -		fpos <<= PAGE_SHIFT;

> +		loff_t fpos = vma->vm_pgoff << PAGE_SHIFT;

Umm...  Are you sure it's OK on 32bit hosts?

Re: [PATCH 08/16] mm/nommu: switch do_mmap_private to kernel_read

[Christoph Hellwig]

On Wed, Aug 30, 2017 at 04:41:28PM +0100, Al Viro wrote:
> On Wed, Aug 30, 2017 at 04:59:59PM +0200, Christoph Hellwig wrote:
> > -		fpos = vma->vm_pgoff;
> > -		fpos <<= PAGE_SHIFT;
> 
> > +		loff_t fpos = vma->vm_pgoff << PAGE_SHIFT;
> 
> Umm...  Are you sure it's OK on 32bit hosts?

It's not.  Fixed.

[PATCH 09/16] net/9p: switch p9_fd_read to kernel_write

[Christoph Hellwig]

Instead of playing with the addressing limits.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 net/9p/trans_fd.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index f12815777beb..903a190319b9 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -422,8 +422,7 @@ static void p9_read_work(struct work_struct *work)
 
 static int p9_fd_write(struct p9_client *client, void *v, int len)
 {
-	int ret;
-	mm_segment_t oldfs;
+	ssize_t ret;
 	struct p9_trans_fd *ts = NULL;
 
 	if (client && client->status != Disconnected)
@@ -435,12 +434,7 @@ static int p9_fd_write(struct p9_client *client, void *v, int len)
 	if (!(ts->wr->f_flags & O_NONBLOCK))
 		p9_debug(P9_DEBUG_ERROR, "blocking write ...\n");
 
-	oldfs = get_fs();
-	set_fs(get_ds());
-	/* The cast to a user pointer is valid due to the set_fs() */
-	ret = vfs_write(ts->wr, (__force void __user *)v, len, &ts->wr->f_pos);
-	set_fs(oldfs);
-
+	ret = kernel_write(ts->wr, v, len, &ts->wr->f_pos);
 	if (ret <= 0 && ret != -ERESTARTSYS && ret != -EAGAIN)
 		client->status = Disconnected;
 	return ret;
-- 
2.11.0

[PATCH 10/16] btrfs: switch write_buf to kernel_write

[Christoph Hellwig]

Instead of playing with the addressing limits.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/send.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index b082210df9c8..24b989fd130c 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -539,33 +539,23 @@ static struct btrfs_path *alloc_path_for_send(void)
 static int write_buf(struct file *filp, const void *buf, u32 len, loff_t *off)
 {
 	int ret;
-	mm_segment_t old_fs;
 	u32 pos = 0;
 
-	old_fs = get_fs();
-	set_fs(KERNEL_DS);
-
 	while (pos < len) {
-		ret = vfs_write(filp, (__force const char __user *)buf + pos,
-				len - pos, off);
+		ret = kernel_write(filp, buf + pos, len - pos, off);
 		/* TODO handle that correctly */
 		/*if (ret == -ERESTARTSYS) {
 			continue;
 		}*/
 		if (ret < 0)
-			goto out;
+			return ret;
 		if (ret == 0) {
-			ret = -EIO;
-			goto out;
+			return -EIO;
 		}
 		pos += ret;
 	}
 
-	ret = 0;
-
-out:
-	set_fs(old_fs);
-	return ret;
+	return 0;
 }
 
 static int tlv_put(struct send_ctx *sctx, u16 attr, const void *data, int len)
-- 
2.11.0

Re: [PATCH 10/16] btrfs: switch write_buf to kernel_write

[Nikolay Borisov]

On 30.08.2017 18:00, Christoph Hellwig wrote:
> Instead of playing with the addressing limits.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

>  fs/btrfs/send.c | 18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index b082210df9c8..24b989fd130c 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -539,33 +539,23 @@ static struct btrfs_path *alloc_path_for_send(void)
>  static int write_buf(struct file *filp, const void *buf, u32 len, loff_t *off)
>  {
>  	int ret;
> -	mm_segment_t old_fs;
>  	u32 pos = 0;
>  
> -	old_fs = get_fs();
> -	set_fs(KERNEL_DS);
> -
>  	while (pos < len) {
> -		ret = vfs_write(filp, (__force const char __user *)buf + pos,
> -				len - pos, off);
> +		ret = kernel_write(filp, buf + pos, len - pos, off);
>  		/* TODO handle that correctly */
>  		/*if (ret == -ERESTARTSYS) {
>  			continue;
>  		}*/
>  		if (ret < 0)
> -			goto out;
> +			return ret;
>  		if (ret == 0) {
> -			ret = -EIO;
> -			goto out;
> +			return -EIO;
>  		}
>  		pos += ret;
>  	}
>  
> -	ret = 0;
> -
> -out:
> -	set_fs(old_fs);
> -	return ret;
> +	return 0;
>  }
>  
>  static int tlv_put(struct send_ctx *sctx, u16 attr, const void *data, int len)
> 

[PATCH 11/16] mconsole: switch to kernel_read

[Christoph Hellwig]

Instead of playing with address limits.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/um/drivers/mconsole_kern.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c
index af326fb6510d..c4d162a94be9 100644
--- a/arch/um/drivers/mconsole_kern.c
+++ b/arch/um/drivers/mconsole_kern.c
@@ -148,12 +148,7 @@ void mconsole_proc(struct mc_request *req)
 	}
 
 	do {
-		loff_t pos = file->f_pos;
-		mm_segment_t old_fs = get_fs();
-		set_fs(KERNEL_DS);
-		len = vfs_read(file, buf, PAGE_SIZE - 1, &pos);
-		set_fs(old_fs);
-		file->f_pos = pos;
+		len = kernel_read(file, buf, PAGE_SIZE - 1, &file->f_pos);
 		if (len < 0) {
 			mconsole_reply(req, "Read of file failed", 1, 0);
 			goto out_free;
-- 
2.11.0

[PATCH 12/16] gadget/f_mass_storage: stop messing with the address limit

[Christoph Hellwig]

Instead use kernel_read/write consistently, which also makes sparse
happy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/usb/gadget/function/f_mass_storage.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index f95bddd6513f..d6bd0244b008 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -686,9 +686,8 @@ static int do_read(struct fsg_common *common)
 
 		/* Perform the read */
 		file_offset_tmp = file_offset;
-		nread = vfs_read(curlun->filp,
-				 (char __user *)bh->buf,
-				 amount, &file_offset_tmp);
+		nread = kernel_read(curlun->filp, bh->buf, amount,
+				&file_offset_tmp);
 		VLDBG(curlun, "file read %u @ %llu -> %d\n", amount,
 		      (unsigned long long)file_offset, (int)nread);
 		if (signal_pending(current))
@@ -883,8 +882,8 @@ static int do_write(struct fsg_common *common)
 
 		/* Perform the write */
 		file_offset_tmp = file_offset;
-		nwritten = vfs_write(curlun->filp, (char __user *)bh->buf,
-				amount, &file_offset_tmp);
+		nwritten = kernel_write(curlun->filp, bh->buf, amount,
+				&file_offset_tmp);
 		VLDBG(curlun, "file write %u @ %llu -> %d\n", amount,
 				(unsigned long long)file_offset, (int)nwritten);
 		if (signal_pending(current))
@@ -1021,9 +1020,8 @@ static int do_verify(struct fsg_common *common)
 
 		/* Perform the read */
 		file_offset_tmp = file_offset;
-		nread = vfs_read(curlun->filp,
-				(char __user *) bh->buf,
-				amount, &file_offset_tmp);
+		nread = kernel_read(curlun->filp, bh->buf, amount,
+				&file_offset_tmp);
 		VLDBG(curlun, "file read %u @ %llu -> %d\n", amount,
 				(unsigned long long) file_offset,
 				(int) nread);
@@ -2453,13 +2451,6 @@ static int fsg_main_thread(void *common_)
 	/* Allow the thread to be frozen */
 	set_freezable();
 
-	/*
-	 * Arrange for userspace references to be interpreted as kernel
-	 * pointers.  That way we can pass a kernel pointer to a routine
-	 * that expects a __user pointer and it will work okay.
-	 */
-	set_fs(get_ds());
-
 	/* The main loop */
 	while (common->state != FSG_STATE_TERMINATED) {
 		if (exception_in_progress(common) || signal_pending(current)) {
-- 
2.11.0

[PATCH 13/16] lustre: switch to kernel_write

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/staging/lustre/lnet/libcfs/tracefile.c      | 10 ++--------
 drivers/staging/lustre/lustre/obdclass/kernelcomm.c |  7 +------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/tracefile.c b/drivers/staging/lustre/lnet/libcfs/tracefile.c
index d1aa79bb2017..af61dd750033 100644
--- a/drivers/staging/lustre/lnet/libcfs/tracefile.c
+++ b/drivers/staging/lustre/lnet/libcfs/tracefile.c
@@ -731,8 +731,7 @@ int cfs_tracefile_dump_all_pages(char *filename)
 		__LASSERT_TAGE_INVARIANT(tage);
 
 		buf = kmap(tage->page);
-		rc = vfs_write(filp, (__force const char __user *)buf,
-			       tage->used, &filp->f_pos);
+		rc = kernel_write(filp, buf, tage->used, &filp->f_pos);
 		kunmap(tage->page);
 
 		if (rc != (int)tage->used) {
@@ -976,7 +975,6 @@ static int tracefiled(void *arg)
 	struct tracefiled_ctl *tctl = arg;
 	struct cfs_trace_page *tage;
 	struct cfs_trace_page *tmp;
-	mm_segment_t __oldfs;
 	struct file *filp;
 	char *buf;
 	int last_loop = 0;
@@ -1014,8 +1012,6 @@ static int tracefiled(void *arg)
 			__LASSERT(list_empty(&pc.pc_pages));
 			goto end_loop;
 		}
-		__oldfs = get_fs();
-		set_fs(get_ds());
 
 		list_for_each_entry_safe(tage, tmp, &pc.pc_pages, linkage) {
 			static loff_t f_pos;
@@ -1028,8 +1024,7 @@ static int tracefiled(void *arg)
 				f_pos = i_size_read(file_inode(filp));
 
 			buf = kmap(tage->page);
-			rc = vfs_write(filp, (__force const char __user *)buf,
-				       tage->used, &f_pos);
+			rc = kernel_write(filp, buf, tage->used, &f_pos);
 			kunmap(tage->page);
 
 			if (rc != (int)tage->used) {
@@ -1040,7 +1035,6 @@ static int tracefiled(void *arg)
 				break;
 			}
 		}
-		set_fs(__oldfs);
 
 		filp_close(filp, NULL);
 		put_pages_on_daemon_list(&pc);
diff --git a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
index a0f65c470f4d..7afe8471a762 100644
--- a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
+++ b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
@@ -52,7 +52,6 @@ int libcfs_kkuc_msg_put(struct file *filp, void *payload)
 	struct kuc_hdr *kuch = (struct kuc_hdr *)payload;
 	ssize_t count = kuch->kuc_msglen;
 	loff_t offset = 0;
-	mm_segment_t fs;
 	int rc = -ENXIO;
 
 	if (IS_ERR_OR_NULL(filp))
@@ -63,18 +62,14 @@ int libcfs_kkuc_msg_put(struct file *filp, void *payload)
 		return rc;
 	}
 
-	fs = get_fs();
-	set_fs(KERNEL_DS);
 	while (count > 0) {
-		rc = vfs_write(filp, (void __force __user *)payload,
-			       count, &offset);
+		rc = kernel_write(filp, payload, count, &offset);
 		if (rc < 0)
 			break;
 		count -= rc;
 		payload += rc;
 		rc = 0;
 	}
-	set_fs(fs);
 
 	if (rc < 0)
 		CWARN("message send failed (%d)\n", rc);
-- 
2.11.0

[PATCH 14/16] fs: unexport __vfs_read/__vfs_write

[Christoph Hellwig]

No modular users left, and any new ones should use kernel_read/write
or iov_iter variants instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/read_write.c    | 2 --
 include/linux/fs.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 82f0111b98cf..1a10ffff1619 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -413,7 +413,6 @@ ssize_t __vfs_read(struct file *file, char __user *buf, size_t count,
 	else
 		return -EINVAL;
 }
-EXPORT_SYMBOL(__vfs_read);
 
 ssize_t kernel_read(struct file *file, char *buf, size_t count, loff_t *pos)
 {
@@ -485,7 +484,6 @@ ssize_t __vfs_write(struct file *file, const char __user *p, size_t count,
 	else
 		return -EINVAL;
 }
-EXPORT_SYMBOL(__vfs_write);
 
 ssize_t __kernel_write(struct file *file, const char *buf, size_t count, loff_t *pos)
 {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 86f50568b758..0fd6109431d4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1754,7 +1754,6 @@ ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
 			      struct iovec **ret_pointer);
 
 extern ssize_t __vfs_read(struct file *, char __user *, size_t, loff_t *);
-extern ssize_t __vfs_write(struct file *, const char __user *, size_t, loff_t *);
 extern ssize_t vfs_read(struct file *, char __user *, size_t, loff_t *);
 extern ssize_t vfs_write(struct file *, const char __user *, size_t, loff_t *);
 extern ssize_t vfs_readv(struct file *, const struct iovec __user *,
-- 
2.11.0

[PATCH 15/16] fs: unexport vfs_read and vfs_write

[Christoph Hellwig]

No modular users left.  Given that they take user pointers there is no
good reason to export it to drivers to start with.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/read_write.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 1a10ffff1619..d3ff440d7084 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -454,8 +454,6 @@ ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
 	return ret;
 }
 
-EXPORT_SYMBOL(vfs_read);
-
 static ssize_t new_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos)
 {
 	struct iovec iov = { .iov_base = (void __user *)buf, .iov_len = len };
@@ -554,8 +552,6 @@ ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_
 	return ret;
 }
 
-EXPORT_SYMBOL(vfs_write);
-
 static inline loff_t file_pos_read(struct file *file)
 {
 	return file->f_pos;
-- 
2.11.0

[PATCH 16/16] fs: unexport vfs_readv and vfs_writev

[Christoph Hellwig]

We've got no modular users left, and any potential modular user is better
of with iov_iter based variants.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/read_write.c    | 4 +---
 include/linux/fs.h | 2 --
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index d3ff440d7084..f8491a81f623 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -982,9 +982,8 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
 
 	return ret;
 }
-EXPORT_SYMBOL(vfs_readv);
 
-ssize_t vfs_writev(struct file *file, const struct iovec __user *vec,
+static ssize_t vfs_writev(struct file *file, const struct iovec __user *vec,
 		   unsigned long vlen, loff_t *pos, int flags)
 {
 	struct iovec iovstack[UIO_FASTIOV];
@@ -1001,7 +1000,6 @@ ssize_t vfs_writev(struct file *file, const struct iovec __user *vec,
 	}
 	return ret;
 }
-EXPORT_SYMBOL(vfs_writev);
 
 static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec,
 			unsigned long vlen, int flags)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 0fd6109431d4..688e72309cde 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1758,8 +1758,6 @@ extern ssize_t vfs_read(struct file *, char __user *, size_t, loff_t *);
 extern ssize_t vfs_write(struct file *, const char __user *, size_t, loff_t *);
 extern ssize_t vfs_readv(struct file *, const struct iovec __user *,
 		unsigned long, loff_t *, int);
-extern ssize_t vfs_writev(struct file *, const struct iovec __user *,
-		unsigned long, loff_t *, int);
 extern ssize_t vfs_copy_file_range(struct file *, loff_t , struct file *,
 				   loff_t, size_t, unsigned int);
 extern int vfs_clone_file_prep_inodes(struct inode *inode_in, loff_t pos_in,
-- 
2.11.0