* [PATCH 0/2] add tee(2) support @ 2020-05-02 12:09 Pavel Begunkov 2020-05-02 12:09 ` [PATCH 1/2] splice: export do_tee() Pavel Begunkov 2020-05-02 12:09 ` [PATCH 2/2] io_uring: add tee(2) support Pavel Begunkov 0 siblings, 2 replies; 9+ messages in thread From: Pavel Begunkov @ 2020-05-02 12:09 UTC (permalink / raw) To: Jens Axboe, io-uring, linux-kernel, Alexander Viro, linux-fsdevel, Clay Harris 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] splice: export do_tee() 2020-05-02 12:09 [PATCH 0/2] add tee(2) support Pavel Begunkov @ 2020-05-02 12:09 ` Pavel Begunkov 2020-05-04 11:09 ` Jann Horn 2020-05-02 12:09 ` [PATCH 2/2] io_uring: add tee(2) support Pavel Begunkov 1 sibling, 1 reply; 9+ messages in thread From: Pavel Begunkov @ 2020-05-02 12:09 UTC (permalink / raw) To: Jens Axboe, io-uring, linux-kernel, Alexander Viro, linux-fsdevel, Clay Harris export do_tee() for use in io_uring Signed-off-by: Pavel Begunkov <[email protected]> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] splice: export do_tee() 2020-05-02 12:09 ` [PATCH 1/2] splice: export do_tee() Pavel Begunkov @ 2020-05-04 11:09 ` Jann Horn 2020-05-04 12:31 ` Pavel Begunkov 0 siblings, 1 reply; 9+ messages in thread From: Jann Horn @ 2020-05-04 11:09 UTC (permalink / raw) To: Pavel Begunkov, Jens Axboe Cc: io-uring, kernel list, Alexander Viro, linux-fsdevel, Clay Harris On Sat, May 2, 2020 at 2:10 PM Pavel Begunkov <[email protected]> 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] splice: export do_tee() 2020-05-04 11:09 ` Jann Horn @ 2020-05-04 12:31 ` Pavel Begunkov 2020-05-04 13:43 ` Jens Axboe 0 siblings, 1 reply; 9+ messages in thread From: Pavel Begunkov @ 2020-05-04 12:31 UTC (permalink / raw) To: Jann Horn, Jens Axboe Cc: io-uring, kernel list, Alexander Viro, linux-fsdevel, Clay Harris On 04/05/2020 14:09, Jann Horn wrote: > On Sat, May 2, 2020 at 2:10 PM Pavel Begunkov <[email protected]> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] splice: export do_tee() 2020-05-04 12:31 ` Pavel Begunkov @ 2020-05-04 13:43 ` Jens Axboe 2020-05-04 14:03 ` Pavel Begunkov 0 siblings, 1 reply; 9+ messages in thread From: Jens Axboe @ 2020-05-04 13:43 UTC (permalink / raw) To: Pavel Begunkov, Jann Horn Cc: io-uring, kernel list, Alexander Viro, linux-fsdevel, Clay Harris 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 <[email protected]> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] splice: export do_tee() 2020-05-04 13:43 ` Jens Axboe @ 2020-05-04 14:03 ` Pavel Begunkov 2020-05-04 16:36 ` Pavel Begunkov 0 siblings, 1 reply; 9+ messages in thread From: Pavel Begunkov @ 2020-05-04 14:03 UTC (permalink / raw) To: Jens Axboe, Jann Horn Cc: io-uring, kernel list, Alexander Viro, linux-fsdevel, Clay Harris 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 <[email protected]> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] splice: export do_tee() 2020-05-04 14:03 ` Pavel Begunkov @ 2020-05-04 16:36 ` Pavel Begunkov 2020-05-04 16:39 ` Jens Axboe 0 siblings, 1 reply; 9+ messages in thread From: Pavel Begunkov @ 2020-05-04 16:36 UTC (permalink / raw) To: Jens Axboe, Jann Horn Cc: io-uring, kernel list, Alexander Viro, linux-fsdevel, Clay Harris 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 <[email protected]> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] splice: export do_tee() 2020-05-04 16:36 ` Pavel Begunkov @ 2020-05-04 16:39 ` Jens Axboe 0 siblings, 0 replies; 9+ messages in thread From: Jens Axboe @ 2020-05-04 16:39 UTC (permalink / raw) To: Pavel Begunkov, Jann Horn Cc: io-uring, kernel list, Alexander Viro, linux-fsdevel, Clay Harris 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 <[email protected]> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] io_uring: add tee(2) support 2020-05-02 12:09 [PATCH 0/2] add tee(2) support Pavel Begunkov 2020-05-02 12:09 ` [PATCH 1/2] splice: export do_tee() Pavel Begunkov @ 2020-05-02 12:09 ` Pavel Begunkov 1 sibling, 0 replies; 9+ messages in thread From: Pavel Begunkov @ 2020-05-02 12:09 UTC (permalink / raw) To: Jens Axboe, io-uring, linux-kernel, Alexander Viro, linux-fsdevel, Clay Harris Add IORING_OP_TEE implementing tee(2) support. Almost identical to splice bits, but without offsets. Signed-off-by: Pavel Begunkov <[email protected]> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-04 16:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-02 12:09 [PATCH 0/2] add tee(2) support Pavel Begunkov 2020-05-02 12:09 ` [PATCH 1/2] splice: export do_tee() Pavel Begunkov 2020-05-04 11:09 ` Jann Horn 2020-05-04 12:31 ` Pavel Begunkov 2020-05-04 13:43 ` Jens Axboe 2020-05-04 14:03 ` Pavel Begunkov 2020-05-04 16:36 ` Pavel Begunkov 2020-05-04 16:39 ` Jens Axboe 2020-05-02 12:09 ` [PATCH 2/2] io_uring: add tee(2) support Pavel Begunkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox