[PATCH 0/2] add tee(2) support

[PATCH 0/2] add tee(2) support

[Pavel Begunkov]

Add tee() reusing splice() bits. Pretty straightforward.

Pavel Begunkov (2):
  splice: export do_tee()
  io_uring: add tee(2) support

 fs/io_uring.c                 | 64 +++++++++++++++++++++++++++++++++--
 fs/splice.c                   |  3 +-
 include/linux/splice.h        |  3 ++
 include/uapi/linux/io_uring.h |  1 +
 4 files changed, 66 insertions(+), 5 deletions(-)

-- 
2.24.0

[PATCH 1/2] splice: export do_tee()

[Pavel Begunkov]

export do_tee() for use in io_uring

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/splice.c            | 3 +--
 include/linux/splice.h | 3 +++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/splice.c b/fs/splice.c
index 4735defc46ee..000be62c5146 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1763,8 +1763,7 @@ static int link_pipe(struct pipe_inode_info *ipipe,
  * The 'flags' used are the SPLICE_F_* variants, currently the only
  * applicable one is SPLICE_F_NONBLOCK.
  */
-static long do_tee(struct file *in, struct file *out, size_t len,
-		   unsigned int flags)
+long do_tee(struct file *in, struct file *out, size_t len, unsigned int flags)
 {
 	struct pipe_inode_info *ipipe = get_pipe_info(in);
 	struct pipe_inode_info *opipe = get_pipe_info(out);
diff --git a/include/linux/splice.h b/include/linux/splice.h
index ebbbfea48aa0..5c47013f708e 100644
--- a/include/linux/splice.h
+++ b/include/linux/splice.h
@@ -82,6 +82,9 @@ extern long do_splice(struct file *in, loff_t __user *off_in,
 		      struct file *out, loff_t __user *off_out,
 		      size_t len, unsigned int flags);
 
+extern long do_tee(struct file *in, struct file *out, size_t len,
+		   unsigned int flags);
+
 /*
  * for dynamic pipe sizing
  */
-- 
2.24.0

[PATCH 2/2] io_uring: add tee(2) support

[Pavel Begunkov]

Add IORING_OP_TEE implementing tee(2) support. Almost identical to
splice bits, but without offsets.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c                 | 64 +++++++++++++++++++++++++++++++++--
 include/uapi/linux/io_uring.h |  1 +
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 4ed82d39540b..dc314f66fbc9 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -855,6 +855,11 @@ static const struct io_op_def io_op_defs[] = {
 	},
 	[IORING_OP_PROVIDE_BUFFERS] = {},
 	[IORING_OP_REMOVE_BUFFERS] = {},
+	[IORING_OP_TEE] = {
+		.needs_file		= 1,
+		.hash_reg_file		= 1,
+		.unbound_nonreg_file	= 1,
+	},
 };
 
 static void io_wq_submit_work(struct io_wq_work **workptr);
@@ -2754,7 +2759,8 @@ static int io_write(struct io_kiocb *req, bool force_nonblock)
 	return ret;
 }
 
-static int io_splice_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+static int __io_splice_prep(struct io_kiocb *req,
+			    const struct io_uring_sqe *sqe)
 {
 	struct io_splice* sp = &req->splice;
 	unsigned int valid_flags = SPLICE_F_FD_IN_FIXED | SPLICE_F_ALL;
@@ -2764,8 +2770,6 @@ static int io_splice_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		return 0;
 
 	sp->file_in = NULL;
-	sp->off_in = READ_ONCE(sqe->splice_off_in);
-	sp->off_out = READ_ONCE(sqe->off);
 	sp->len = READ_ONCE(sqe->len);
 	sp->flags = READ_ONCE(sqe->splice_flags);
 
@@ -2784,6 +2788,48 @@ static int io_splice_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	return 0;
 }
 
+static int io_tee_prep(struct io_kiocb *req,
+		       const struct io_uring_sqe *sqe)
+{
+	if (READ_ONCE(sqe->splice_off_in) || READ_ONCE(sqe->off))
+		return -EINVAL;
+	return __io_splice_prep(req, sqe);
+}
+
+static int io_tee(struct io_kiocb *req, bool force_nonblock)
+{
+	struct io_splice *sp = &req->splice;
+	struct file *in = sp->file_in;
+	struct file *out = sp->file_out;
+	unsigned int flags = sp->flags & ~SPLICE_F_FD_IN_FIXED;
+	long ret;
+
+	if (force_nonblock)
+		return -EAGAIN;
+
+	ret = do_tee(in, out, sp->len, flags);
+	if (force_nonblock && ret == -EAGAIN)
+		return -EAGAIN;
+
+	io_put_file(req, in, (sp->flags & SPLICE_F_FD_IN_FIXED));
+	req->flags &= ~REQ_F_NEED_CLEANUP;
+
+	io_cqring_add_event(req, ret);
+	if (ret != sp->len)
+		req_set_fail_links(req);
+	io_put_req(req);
+	return 0;
+}
+
+static int io_splice_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+	struct io_splice* sp = &req->splice;
+
+	sp->off_in = READ_ONCE(sqe->splice_off_in);
+	sp->off_out = READ_ONCE(sqe->off);
+	return __io_splice_prep(req, sqe);
+}
+
 static int io_splice(struct io_kiocb *req, bool force_nonblock)
 {
 	struct io_splice *sp = &req->splice;
@@ -4978,6 +5024,9 @@ static int io_req_defer_prep(struct io_kiocb *req,
 	case IORING_OP_REMOVE_BUFFERS:
 		ret = io_remove_buffers_prep(req, sqe);
 		break;
+	case IORING_OP_TEE:
+		ret = io_tee_prep(req, sqe);
+		break;
 	default:
 		printk_once(KERN_WARNING "io_uring: unhandled opcode %d\n",
 				req->opcode);
@@ -5051,6 +5100,7 @@ static void io_cleanup_req(struct io_kiocb *req)
 		putname(req->open.filename);
 		break;
 	case IORING_OP_SPLICE:
+	case IORING_OP_TEE:
 		io_put_file(req, req->splice.file_in,
 			    (req->splice.flags & SPLICE_F_FD_IN_FIXED));
 		break;
@@ -5281,6 +5331,14 @@ static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe,
 		}
 		ret = io_remove_buffers(req, force_nonblock);
 		break;
+	case IORING_OP_TEE:
+		if (sqe) {
+			ret = io_tee_prep(req, sqe);
+			if (ret < 0)
+				break;
+		}
+		ret = io_tee(req, force_nonblock);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index e48d746b8e2a..a279151437fc 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -129,6 +129,7 @@ enum {
 	IORING_OP_SPLICE,
 	IORING_OP_PROVIDE_BUFFERS,
 	IORING_OP_REMOVE_BUFFERS,
+	IORING_OP_TEE,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
-- 
2.24.0

Re: [PATCH 1/2] splice: export do_tee()

[Jann Horn]

On Sat, May 2, 2020 at 2:10 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
> export do_tee() for use in io_uring
[...]
> diff --git a/fs/splice.c b/fs/splice.c
[...]
>   * The 'flags' used are the SPLICE_F_* variants, currently the only
>   * applicable one is SPLICE_F_NONBLOCK.
>   */
> -static long do_tee(struct file *in, struct file *out, size_t len,
> -                  unsigned int flags)
> +long do_tee(struct file *in, struct file *out, size_t len, unsigned int flags)
>  {
>         struct pipe_inode_info *ipipe = get_pipe_info(in);
>         struct pipe_inode_info *opipe = get_pipe_info(out);

AFAICS do_tee() in its current form is not something you should be
making available to anything else, because the file mode checks are
performed in sys_tee() instead of in do_tee(). (And I don't see any
check for file modes in your uring patch, but maybe I missed it?) If
you want to make do_tee() available elsewhere, please refactor the
file mode checks over into do_tee().

The same thing seems to be true for the splice support, which luckily
hasn't landed in a kernel release yet... while do_splice() does a
random assortment of checks, the checks that actually consistently
enforce the rules happen in sys_splice(). From a quick look,
do_splice() doesn't seem to check:

 - when splicing from a pipe to a non-pipe: whether read access to the
input pipe exists
 - when splicing from a non-pipe to a pipe: whether write access to
the output pipe exists

... which AFAICS means that io_uring probably lets you get full R/W
access to any pipe to which you're supposed to have either read or
write access. (Although admittedly it is rare in practice that you get
one end of a pipe and can't access the other one.)

When you expose previously internal helpers to io_uring, please have a
look at their callers and see whether they perform any checks that
look relevant.

Re: [PATCH 1/2] splice: export do_tee()

[Pavel Begunkov]

On 04/05/2020 14:09, Jann Horn wrote:
> On Sat, May 2, 2020 at 2:10 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>> export do_tee() for use in io_uring
> [...]
>> diff --git a/fs/splice.c b/fs/splice.c
> [...]
>>   * The 'flags' used are the SPLICE_F_* variants, currently the only
>>   * applicable one is SPLICE_F_NONBLOCK.
>>   */
>> -static long do_tee(struct file *in, struct file *out, size_t len,
>> -                  unsigned int flags)
>> +long do_tee(struct file *in, struct file *out, size_t len, unsigned int flags)
>>  {
>>         struct pipe_inode_info *ipipe = get_pipe_info(in);
>>         struct pipe_inode_info *opipe = get_pipe_info(out);
> 
> AFAICS do_tee() in its current form is not something you should be
> making available to anything else, because the file mode checks are
> performed in sys_tee() instead of in do_tee(). (And I don't see any
> check for file modes in your uring patch, but maybe I missed it?) If
> you want to make do_tee() available elsewhere, please refactor the
> file mode checks over into do_tee().

Overlooked it indeed. Glad you found it

> 
> The same thing seems to be true for the splice support, which luckily
> hasn't landed in a kernel release yet... while do_splice() does a
> random assortment of checks, the checks that actually consistently
> enforce the rules happen in sys_splice(). From a quick look,
> do_splice() doesn't seem to check:
> 
>  - when splicing from a pipe to a non-pipe: whether read access to the
> input pipe exists
>  - when splicing from a non-pipe to a pipe: whether write access to
> the output pipe exists
> 
> ... which AFAICS means that io_uring probably lets you get full R/W
> access to any pipe to which you're supposed to have either read or
> write access. (Although admittedly it is rare in practice that you get
> one end of a pipe and can't access the other one.)
> 
> When you expose previously internal helpers to io_uring, please have a
> look at their callers and see whether they perform any checks that
> look relevant.
> 

-- 
Pavel Begunkov

Re: [PATCH 1/2] splice: export do_tee()

[Jens Axboe]

On 5/4/20 6:31 AM, Pavel Begunkov wrote:
> On 04/05/2020 14:09, Jann Horn wrote:
>> On Sat, May 2, 2020 at 2:10 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>> export do_tee() for use in io_uring
>> [...]
>>> diff --git a/fs/splice.c b/fs/splice.c
>> [...]
>>>   * The 'flags' used are the SPLICE_F_* variants, currently the only
>>>   * applicable one is SPLICE_F_NONBLOCK.
>>>   */
>>> -static long do_tee(struct file *in, struct file *out, size_t len,
>>> -                  unsigned int flags)
>>> +long do_tee(struct file *in, struct file *out, size_t len, unsigned int flags)
>>>  {
>>>         struct pipe_inode_info *ipipe = get_pipe_info(in);
>>>         struct pipe_inode_info *opipe = get_pipe_info(out);
>>
>> AFAICS do_tee() in its current form is not something you should be
>> making available to anything else, because the file mode checks are
>> performed in sys_tee() instead of in do_tee(). (And I don't see any
>> check for file modes in your uring patch, but maybe I missed it?) If
>> you want to make do_tee() available elsewhere, please refactor the
>> file mode checks over into do_tee().
> 
> Overlooked it indeed. Glad you found it

Yeah indeed, that's a glaring oversight on my part too. Will you send
a patch for 5.7-rc as well for splice?

-- 
Jens Axboe

Re: [PATCH 1/2] splice: export do_tee()

[Pavel Begunkov]

On 04/05/2020 16:43, Jens Axboe wrote:
> On 5/4/20 6:31 AM, Pavel Begunkov wrote:
>> On 04/05/2020 14:09, Jann Horn wrote:
>>> On Sat, May 2, 2020 at 2:10 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>> export do_tee() for use in io_uring
>>> [...]
>>>> diff --git a/fs/splice.c b/fs/splice.c
>>> [...]
>>>>   * The 'flags' used are the SPLICE_F_* variants, currently the only
>>>>   * applicable one is SPLICE_F_NONBLOCK.
>>>>   */
>>>> -static long do_tee(struct file *in, struct file *out, size_t len,
>>>> -                  unsigned int flags)
>>>> +long do_tee(struct file *in, struct file *out, size_t len, unsigned int flags)
>>>>  {
>>>>         struct pipe_inode_info *ipipe = get_pipe_info(in);
>>>>         struct pipe_inode_info *opipe = get_pipe_info(out);
>>>
>>> AFAICS do_tee() in its current form is not something you should be
>>> making available to anything else, because the file mode checks are
>>> performed in sys_tee() instead of in do_tee(). (And I don't see any
>>> check for file modes in your uring patch, but maybe I missed it?) If
>>> you want to make do_tee() available elsewhere, please refactor the
>>> file mode checks over into do_tee().
>>
>> Overlooked it indeed. Glad you found it
> 
> Yeah indeed, that's a glaring oversight on my part too. Will you send
> a patch for 5.7-rc as well for splice?

Absolutely

-- 
Pavel Begunkov

Re: [PATCH 1/2] splice: export do_tee()

[Pavel Begunkov]

On 04/05/2020 17:03, Pavel Begunkov wrote:
> On 04/05/2020 16:43, Jens Axboe wrote:
>> On 5/4/20 6:31 AM, Pavel Begunkov wrote:
>>> On 04/05/2020 14:09, Jann Horn wrote:
>>>> On Sat, May 2, 2020 at 2:10 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>>> export do_tee() for use in io_uring
>>>> [...]
>>>>> diff --git a/fs/splice.c b/fs/splice.c
>>>> [...]
>>>>>   * The 'flags' used are the SPLICE_F_* variants, currently the only
>>>>>   * applicable one is SPLICE_F_NONBLOCK.
>>>>>   */
>>>>> -static long do_tee(struct file *in, struct file *out, size_t len,
>>>>> -                  unsigned int flags)
>>>>> +long do_tee(struct file *in, struct file *out, size_t len, unsigned int flags)
>>>>>  {
>>>>>         struct pipe_inode_info *ipipe = get_pipe_info(in);
>>>>>         struct pipe_inode_info *opipe = get_pipe_info(out);
>>>>
>>>> AFAICS do_tee() in its current form is not something you should be
>>>> making available to anything else, because the file mode checks are
>>>> performed in sys_tee() instead of in do_tee(). (And I don't see any
>>>> check for file modes in your uring patch, but maybe I missed it?) If
>>>> you want to make do_tee() available elsewhere, please refactor the
>>>> file mode checks over into do_tee().
>>>
>>> Overlooked it indeed. Glad you found it
>>
>> Yeah indeed, that's a glaring oversight on my part too. Will you send
>> a patch for 5.7-rc as well for splice?
> 
> Absolutely

The right way would be to do as Jann proposed, but would you prefer an
io_uring.c local fix for-5.7 and then a proper one? I assume it could be easier
to manage.

-- 
Pavel Begunkov

Re: [PATCH 1/2] splice: export do_tee()

[Jens Axboe]

On 5/4/20 10:36 AM, Pavel Begunkov wrote:
> On 04/05/2020 17:03, Pavel Begunkov wrote:
>> On 04/05/2020 16:43, Jens Axboe wrote:
>>> On 5/4/20 6:31 AM, Pavel Begunkov wrote:
>>>> On 04/05/2020 14:09, Jann Horn wrote:
>>>>> On Sat, May 2, 2020 at 2:10 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>>>> export do_tee() for use in io_uring
>>>>> [...]
>>>>>> diff --git a/fs/splice.c b/fs/splice.c
>>>>> [...]
>>>>>>   * The 'flags' used are the SPLICE_F_* variants, currently the only
>>>>>>   * applicable one is SPLICE_F_NONBLOCK.
>>>>>>   */
>>>>>> -static long do_tee(struct file *in, struct file *out, size_t len,
>>>>>> -                  unsigned int flags)
>>>>>> +long do_tee(struct file *in, struct file *out, size_t len, unsigned int flags)
>>>>>>  {
>>>>>>         struct pipe_inode_info *ipipe = get_pipe_info(in);
>>>>>>         struct pipe_inode_info *opipe = get_pipe_info(out);
>>>>>
>>>>> AFAICS do_tee() in its current form is not something you should be
>>>>> making available to anything else, because the file mode checks are
>>>>> performed in sys_tee() instead of in do_tee(). (And I don't see any
>>>>> check for file modes in your uring patch, but maybe I missed it?) If
>>>>> you want to make do_tee() available elsewhere, please refactor the
>>>>> file mode checks over into do_tee().
>>>>
>>>> Overlooked it indeed. Glad you found it
>>>
>>> Yeah indeed, that's a glaring oversight on my part too. Will you send
>>> a patch for 5.7-rc as well for splice?
>>
>> Absolutely
> 
> The right way would be to do as Jann proposed, but would you prefer an
> io_uring.c local fix for-5.7 and then a proper one? I assume it could
> be easier to manage.

Let's just do a proper one for 5.7 as well.

-- 
Jens Axboe