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