public inbox for [email protected]
 help / color / mirror / Atom feed
From: Clay Harris <[email protected]>
To: Pavel Begunkov <[email protected]>
Cc: Jens Axboe <[email protected]>,
	Alexander Viro <[email protected]>,
	Jann Horn <[email protected]>,
	[email protected], [email protected],
	[email protected]
Subject: Re: [PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()
Date: Tue, 5 May 2020 17:00:32 -0500	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On Wed, May 06 2020 at 00:38:05 +0300, Pavel Begunkov quoth thus:

> On 06/05/2020 00:10, Clay Harris wrote:
> > On Mon, May 04 2020 at 22:39:35 +0300, Pavel Begunkov quoth thus:
> > 
> >> do_splice() is used by io_uring, as will be do_tee(). Move f_mode
> >> checks from sys_{splice,tee}() to do_{splice,tee}(), so they're
> >> enforced for io_uring as well.
> > 
> > I'm not seeing any check against splicing a pipe to itself in the
> > io_uring path, although maybe I just missed it.  As the comment
> > below says: /* Splicing to self would be fun, but... */ .
> 
> io_uring just forwards a request to do_splice(), which do the check at the exact
> place you mentioned. The similar story is with do_tee().

Okay.  I'd been thinking that since you were moving the file mode
checks into io_uring that the previous place they were called wasn't
on the path.  Evidently, you're just moving the mode checks earlier.

> > 
> >> Fixes: 7d67af2c0134 ("io_uring: add splice(2) support")
> >> Reported-by: Jann Horn <[email protected]>
> >> Signed-off-by: Pavel Begunkov <[email protected]>
> >> ---
> >>  fs/splice.c | 45 ++++++++++++++++++---------------------------
> >>  1 file changed, 18 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/fs/splice.c b/fs/splice.c
> >> index 4735defc46ee..fd0a1e7e5959 100644
> >> --- a/fs/splice.c
> >> +++ b/fs/splice.c
> >> @@ -1118,6 +1118,10 @@ long do_splice(struct file *in, loff_t __user *off_in,
> >>  	loff_t offset;
> >>  	long ret;
> >>  
> >> +	if (unlikely(!(in->f_mode & FMODE_READ) ||
> >> +		     !(out->f_mode & FMODE_WRITE)))
> >> +		return -EBADF;
> >> +
> >>  	ipipe = get_pipe_info(in);
> >>  	opipe = get_pipe_info(out);
> >>  
> >> @@ -1125,12 +1129,6 @@ long do_splice(struct file *in, loff_t __user *off_in,
> >>  		if (off_in || off_out)
> >>  			return -ESPIPE;
> >>  
> >> -		if (!(in->f_mode & FMODE_READ))
> >> -			return -EBADF;
> >> -
> >> -		if (!(out->f_mode & FMODE_WRITE))
> >> -			return -EBADF;
> >> -
> >>  		/* Splicing to self would be fun, but... */
> >>  		if (ipipe == opipe)
> >>  			return -EINVAL;
> >> @@ -1153,9 +1151,6 @@ long do_splice(struct file *in, loff_t __user *off_in,
> >>  			offset = out->f_pos;
> >>  		}
> >>  
> >> -		if (unlikely(!(out->f_mode & FMODE_WRITE)))
> >> -			return -EBADF;
> >> -
> >>  		if (unlikely(out->f_flags & O_APPEND))
> >>  			return -EINVAL;
> >>  
> >> @@ -1440,15 +1435,11 @@ SYSCALL_DEFINE6(splice, int, fd_in, loff_t __user *, off_in,
> >>  	error = -EBADF;
> >>  	in = fdget(fd_in);
> >>  	if (in.file) {
> >> -		if (in.file->f_mode & FMODE_READ) {
> >> -			out = fdget(fd_out);
> >> -			if (out.file) {
> >> -				if (out.file->f_mode & FMODE_WRITE)
> >> -					error = do_splice(in.file, off_in,
> >> -							  out.file, off_out,
> >> -							  len, flags);
> >> -				fdput(out);
> >> -			}
> >> +		out = fdget(fd_out);
> >> +		if (out.file) {
> >> +			error = do_splice(in.file, off_in, out.file, off_out,
> >> +					  len, flags);
> >> +			fdput(out);
> >>  		}
> >>  		fdput(in);
> >>  	}
> >> @@ -1770,6 +1761,10 @@ static long do_tee(struct file *in, struct file *out, size_t len,
> >>  	struct pipe_inode_info *opipe = get_pipe_info(out);
> >>  	int ret = -EINVAL;
> >>  
> >> +	if (unlikely(!(in->f_mode & FMODE_READ) ||
> >> +		     !(out->f_mode & FMODE_WRITE)))
> >> +		return -EBADF;
> >> +
> >>  	/*
> >>  	 * Duplicate the contents of ipipe to opipe without actually
> >>  	 * copying the data.
> >> @@ -1795,7 +1790,7 @@ static long do_tee(struct file *in, struct file *out, size_t len,
> >>  
> >>  SYSCALL_DEFINE4(tee, int, fdin, int, fdout, size_t, len, unsigned int, flags)
> >>  {
> >> -	struct fd in;
> >> +	struct fd in, out;
> >>  	int error;
> >>  
> >>  	if (unlikely(flags & ~SPLICE_F_ALL))
> >> @@ -1807,14 +1802,10 @@ SYSCALL_DEFINE4(tee, int, fdin, int, fdout, size_t, len, unsigned int, flags)
> >>  	error = -EBADF;
> >>  	in = fdget(fdin);
> >>  	if (in.file) {
> >> -		if (in.file->f_mode & FMODE_READ) {
> >> -			struct fd out = fdget(fdout);
> >> -			if (out.file) {
> >> -				if (out.file->f_mode & FMODE_WRITE)
> >> -					error = do_tee(in.file, out.file,
> >> -							len, flags);
> >> -				fdput(out);
> >> -			}
> >> +		out = fdget(fdout);
> >> +		if (out.file) {
> >> +			error = do_tee(in.file, out.file, len, flags);
> >> +			fdput(out);
> >>  		}
> >>   		fdput(in);
> >>   	}
> >> -- 
> >> 2.24.0
> 
> -- 
> Pavel Begunkov

  reply	other threads:[~2020-05-05 22:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-04 19:39 [PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}() Pavel Begunkov
2020-05-05 21:10 ` Clay Harris
2020-05-05 21:38   ` Pavel Begunkov
2020-05-05 22:00     ` Clay Harris [this message]
2020-05-05 22:13       ` Pavel Begunkov
2020-05-05 22:26       ` Clay Harris
2020-05-07 15:45 ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200505220032.fm5vqf3xuaucnjle@ps29521.dreamhostps.com \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox