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 <ja...@google.com>
>> Signed-off-by: Pavel Begunkov <asml.sile...@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
