[PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()

[PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()

[Pavel Begunkov]

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.

Fixes: 7d67af2c0134 ("io_uring: add splice(2) support")
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 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

Re: [PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()

[Clay Harris]

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... */ .

> Fixes: 7d67af2c0134 ("io_uring: add splice(2) support")
> Reported-by: Jann Horn <jannh@google.com>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  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

Re: [PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()

[Pavel Begunkov]

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().

> 
>> Fixes: 7d67af2c0134 ("io_uring: add splice(2) support")
>> Reported-by: Jann Horn <jannh@google.com>
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>  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

Re: [PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()

[Clay Harris]

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 <jannh@google.com>
> >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> >> ---
> >>  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

Re: [PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()

[Pavel Begunkov]

On 06/05/2020 01:00, Clay Harris wrote:
> 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.

I move them from sys_splice() later to do_splice(). Even though the patch
doesn't touch io_uring directly, it fixes a problem in there. And as a nice
addition, it looks prettier and removes a couple of useless checks.


-- 
Pavel Begunkov

Re: [PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()

[Clay Harris]

On Tue, May 05 2020 at 17:00:32 -0500, Clay Harris quoth thus:

> 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.

My bad.  I meant to say *later*.  (After the syscall entry, at the spot
io_uring would call.)

Thanks for looking at it!

> > > 
> > >> Fixes: 7d67af2c0134 ("io_uring: add splice(2) support")
> > >> Reported-by: Jann Horn <jannh@google.com>
> > >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> > >> ---
> > >>  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

Re: [PATCH for-5.7] splice: move f_mode checks to do_{splice,tee}()

[Jens Axboe]

On 5/4/20 1:39 PM, Pavel Begunkov wrote:
> 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.

Applied for 5.7, thanks.

-- 
Jens Axboe