diff mbox series

[05/15] splice: remove permission hook from iter_file_splice_write()

Message ID 20231114153321.1716028-6-amir73il@gmail.com (mailing list archive)
State New
Headers show
Series Tidy up file permission hooks | expand

Commit Message

Amir Goldstein Nov. 14, 2023, 3:33 p.m. UTC
All the callers of ->splice_write(), (e.g. do_splice_direct() and
do_splice()) already check rw_verify_area() for the entire range
and perform all the other checks that are in vfs_write_iter().

Create a helper do_iter_writev(), that performs the write without the
checks and use it in iter_file_splice_write() to avoid the redundant
rw_verify_area() checks.

This is needed for fanotify "pre content" events.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/internal.h   | 8 +++++++-
 fs/read_write.c | 7 +++++++
 fs/splice.c     | 9 ++++++---
 3 files changed, 20 insertions(+), 4 deletions(-)

Comments

Christian Brauner Nov. 21, 2023, 2:56 p.m. UTC | #1
On Tue, Nov 14, 2023 at 05:33:11PM +0200, Amir Goldstein wrote:
> All the callers of ->splice_write(), (e.g. do_splice_direct() and
> do_splice()) already check rw_verify_area() for the entire range
> and perform all the other checks that are in vfs_write_iter().
> 
> Create a helper do_iter_writev(), that performs the write without the
> checks and use it in iter_file_splice_write() to avoid the redundant
> rw_verify_area() checks.
> 
> This is needed for fanotify "pre content" events.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

If you resend anyway, for the low-level splice helpers specifically it
would be nice to add a comment that mentions that the caller is expected
to perform basic rw checks.
Amir Goldstein Nov. 21, 2023, 3:18 p.m. UTC | #2
On Tue, Nov 21, 2023 at 4:56 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Tue, Nov 14, 2023 at 05:33:11PM +0200, Amir Goldstein wrote:
> > All the callers of ->splice_write(), (e.g. do_splice_direct() and
> > do_splice()) already check rw_verify_area() for the entire range
> > and perform all the other checks that are in vfs_write_iter().
> >
> > Create a helper do_iter_writev(), that performs the write without the
> > checks and use it in iter_file_splice_write() to avoid the redundant
> > rw_verify_area() checks.
> >
> > This is needed for fanotify "pre content" events.
> >
> > Suggested-by: Jan Kara <jack@suse.cz>
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
>
> If you resend anyway, for the low-level splice helpers specifically it
> would be nice to add a comment that mentions that the caller is expected
> to perform basic rw checks.

Will do.

Thanks,
Amir.
diff mbox series

Patch

diff --git a/fs/internal.h b/fs/internal.h
index 58e43341aebf..c114b85e27a7 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -298,7 +298,13 @@  static inline ssize_t do_get_acl(struct mnt_idmap *idmap,
 }
 #endif
 
-ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from, loff_t *pos);
+/*
+ * fs/read_write.c
+ */
+ssize_t __kernel_write_iter(struct file *file, struct iov_iter *from,
+			    loff_t *pos);
+ssize_t do_iter_writev(struct file *file, struct iov_iter *iter, loff_t *ppos,
+		       rwf_t flags);
 
 /*
  * fs/attr.c
diff --git a/fs/read_write.c b/fs/read_write.c
index 4771701c896b..590ab228fa98 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -739,6 +739,13 @@  static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter,
 	return ret;
 }
 
+ssize_t do_iter_writev(struct file *filp, struct iov_iter *iter, loff_t *ppos,
+		       rwf_t flags)
+{
+	return do_iter_readv_writev(filp, iter, ppos, WRITE, flags);
+}
+
+
 /* Do it by hand, with file-ops */
 static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,
 		loff_t *ppos, int type, rwf_t flags)
diff --git a/fs/splice.c b/fs/splice.c
index d4fdd44c0b32..decbace5d812 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -673,10 +673,13 @@  iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
 		.u.file = out,
 	};
 	int nbufs = pipe->max_usage;
-	struct bio_vec *array = kcalloc(nbufs, sizeof(struct bio_vec),
-					GFP_KERNEL);
+	struct bio_vec *array;
 	ssize_t ret;
 
+	if (!out->f_op->write_iter)
+		return -EINVAL;
+
+	array = kcalloc(nbufs, sizeof(struct bio_vec), GFP_KERNEL);
 	if (unlikely(!array))
 		return -ENOMEM;
 
@@ -733,7 +736,7 @@  iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
 		}
 
 		iov_iter_bvec(&from, ITER_SOURCE, array, n, sd.total_len - left);
-		ret = vfs_iter_write(out, &from, &sd.pos, 0);
+		ret = do_iter_writev(out, &from, &sd.pos, 0);
 		if (ret <= 0)
 			break;