2010-06-29 11:08:21

by Miklos Szeredi

[permalink] [raw]
Subject: [PATCH 0/3] splice fixes

These patches fix bugs that affect real-world users of splice(2) and
sendfile(2) interfaces.

Not sure if some or all of them are appropriate for 2.6.35 and/or
-stable?

Thanks,
Miklos


2010-06-29 11:09:23

by Miklos Szeredi

[permalink] [raw]
Subject: [PATCH 1/3] splice: direct_splice_actor() should not use pos in sd

From: Changli Gao <[email protected]>

direct_splice_actor() shouldn't use sd->pos, as sd->pos is for file reading,
file->f_pos should be used instead.

Signed-off-by: Changli Gao <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
----
fs/splice.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c 2010-06-29 12:46:27.000000000 +0200
+++ linux-2.6/fs/splice.c 2010-06-29 12:46:28.000000000 +0200
@@ -1282,7 +1282,8 @@ static int direct_splice_actor(struct pi
{
struct file *file = sd->u.file;

- return do_splice_from(pipe, file, &sd->pos, sd->total_len, sd->flags);
+ return do_splice_from(pipe, file, &file->f_pos, sd->total_len,
+ sd->flags);
}

/**

2010-06-29 11:10:41

by Miklos Szeredi

[permalink] [raw]
Subject: [PATCH 2/3] splice: check f_mode for seekable file

From: Changli Gao <[email protected]>

check f_mode for seekable file

As a seekable file is allowed without a llseek function, so the old way isn't
work any more.

Signed-off-by: Changli Gao <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
----
fs/splice.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c 2010-06-29 12:46:28.000000000 +0200
+++ linux-2.6/fs/splice.c 2010-06-29 12:46:30.000000000 +0200
@@ -1372,8 +1372,7 @@ static long do_splice(struct file *in, l
if (off_in)
return -ESPIPE;
if (off_out) {
- if (!out->f_op || !out->f_op->llseek ||
- out->f_op->llseek == no_llseek)
+ if (!(out->f_mode & FMODE_PWRITE))
return -EINVAL;
if (copy_from_user(&offset, off_out, sizeof(loff_t)))
return -EFAULT;
@@ -1393,8 +1392,7 @@ static long do_splice(struct file *in, l
if (off_out)
return -ESPIPE;
if (off_in) {
- if (!in->f_op || !in->f_op->llseek ||
- in->f_op->llseek == no_llseek)
+ if (!(in->f_mode & FMODE_PREAD))
return -EINVAL;
if (copy_from_user(&offset, off_in, sizeof(loff_t)))
return -EFAULT;

2010-06-29 11:13:10

by Miklos Szeredi

[permalink] [raw]
Subject: [PATCH 3/3] splice: fix misuse of SPLICE_F_NONBLOCK

From: Miklos Szeredi <[email protected]>

SPLICE_F_NONBLOCK is clearly documented to only affect blocking on the
destination pipe and not on the source file.

In __generic_file_splice_read(), however, it returns EAGAIN if the
page is currently being read.

This makes it impossible to write an application that only wants
failure if the pipe buffer is full. For example if the same process
is handling both ends of a pipe and isn't otherwise able to determine
whether a splice to the pipe will fit or not.

We could make the read non-blocking on O_NONBLOCK or some other splice
flag, but for now this is the simplest fix.

Signed-off-by: Miklos Szeredi <[email protected]>
---
fs/splice.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)

Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c 2010-06-29 12:46:30.000000000 +0200
+++ linux-2.6/fs/splice.c 2010-06-29 12:46:32.000000000 +0200
@@ -399,17 +399,7 @@ __generic_file_splice_read(struct file *
* If the page isn't uptodate, we may need to start io on it
*/
if (!PageUptodate(page)) {
- /*
- * If in nonblock mode then dont block on waiting
- * for an in-flight io page
- */
- if (flags & SPLICE_F_NONBLOCK) {
- if (!trylock_page(page)) {
- error = -EAGAIN;
- break;
- }
- } else
- lock_page(page);
+ lock_page(page);

/*
* Page was truncated, or invalidated by the

2010-06-29 11:15:08

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/3] splice fixes

On 2010-06-29 13:08, Miklos Szeredi wrote:
> These patches fix bugs that affect real-world users of splice(2) and
> sendfile(2) interfaces.
>
> Not sure if some or all of them are appropriate for 2.6.35 and/or
> -stable?

I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
a stable backport as well. Agree?

--
Jens Axboe

2010-06-29 12:05:12

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 0/3] splice fixes

On Tue, 29 Jun 2010, Jens Axboe wrote:
> On 2010-06-29 13:08, Miklos Szeredi wrote:
> > These patches fix bugs that affect real-world users of splice(2) and
> > sendfile(2) interfaces.
> >
> > Not sure if some or all of them are appropriate for 2.6.35 and/or
> > -stable?
>
> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
> a stable backport as well. Agree?

Yes, sounds good.

Thanks,
Miklos

2010-06-29 12:05:53

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/3] splice fixes

On 2010-06-29 14:05, Miklos Szeredi wrote:
> On Tue, 29 Jun 2010, Jens Axboe wrote:
>> On 2010-06-29 13:08, Miklos Szeredi wrote:
>>> These patches fix bugs that affect real-world users of splice(2) and
>>> sendfile(2) interfaces.
>>>
>>> Not sure if some or all of them are appropriate for 2.6.35 and/or
>>> -stable?
>>
>> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
>> a stable backport as well. Agree?
>
> Yes, sounds good.

Good, that is what I queued up. I haven't pushed out the updated
for-linus yet, since I asked Linus to pull it this morning.

--
Jens Axboe

2010-07-07 20:39:55

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 0/3] splice fixes

On Tue, 29 Jun 2010 14:05:52 +0200
Jens Axboe <[email protected]> wrote:

> On 2010-06-29 14:05, Miklos Szeredi wrote:
> > On Tue, 29 Jun 2010, Jens Axboe wrote:
> >> On 2010-06-29 13:08, Miklos Szeredi wrote:
> >>> These patches fix bugs that affect real-world users of splice(2) and
> >>> sendfile(2) interfaces.
> >>>
> >>> Not sure if some or all of them are appropriate for 2.6.35 and/or
> >>> -stable?
> >>
> >> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
> >> a stable backport as well. Agree?
> >
> > Yes, sounds good.
>
> Good, that is what I queued up. I haven't pushed out the updated
> for-linus yet, since I asked Linus to pull it this morning.
>

According to Tim's report on lkml
(http://lkml.org/lkml/2010/6/25/280),
cc56f7de7f00d188c7c4da1e9861581853b9e92f caused a regression.
cc56f7de7f00d188c7c4da1e9861581853b9e92f is present in 2.6.33 and in
2.6.34 hence the fix should be backported into -stable.

According to http://bugs.launchpad.net/bugs/588861, the patch "splice:
direct_splice_actor() should not use pos in sd" fixes the regression.

However this:

commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
Author: Changli Gao <[email protected]>
Date: Tue Jun 29 13:09:18 2010 +0200

splice: direct_splice_actor() should not use pos in sd

went into mainline without a cc:stable tag.

2010-07-08 13:36:42

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/3] splice fixes

On 2010-07-07 22:38, Andrew Morton wrote:
> On Tue, 29 Jun 2010 14:05:52 +0200
> Jens Axboe <[email protected]> wrote:
>
>> On 2010-06-29 14:05, Miklos Szeredi wrote:
>>> On Tue, 29 Jun 2010, Jens Axboe wrote:
>>>> On 2010-06-29 13:08, Miklos Szeredi wrote:
>>>>> These patches fix bugs that affect real-world users of splice(2) and
>>>>> sendfile(2) interfaces.
>>>>>
>>>>> Not sure if some or all of them are appropriate for 2.6.35 and/or
>>>>> -stable?
>>>>
>>>> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
>>>> a stable backport as well. Agree?
>>>
>>> Yes, sounds good.
>>
>> Good, that is what I queued up. I haven't pushed out the updated
>> for-linus yet, since I asked Linus to pull it this morning.
>>
>
> According to Tim's report on lkml
> (http://lkml.org/lkml/2010/6/25/280),
> cc56f7de7f00d188c7c4da1e9861581853b9e92f caused a regression.
> cc56f7de7f00d188c7c4da1e9861581853b9e92f is present in 2.6.33 and in
> 2.6.34 hence the fix should be backported into -stable.
>
> According to http://bugs.launchpad.net/bugs/588861, the patch "splice:
> direct_splice_actor() should not use pos in sd" fixes the regression.
>
> However this:
>
> commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
> Author: Changli Gao <[email protected]>
> Date: Tue Jun 29 13:09:18 2010 +0200
>
> splice: direct_splice_actor() should not use pos in sd
>
> went into mainline without a cc:stable tag.

I will send an email to stable@.

--
Jens Axboe

2010-07-08 13:38:25

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/3] splice fixes

On 2010-07-08 15:36, Jens Axboe wrote:
> On 2010-07-07 22:38, Andrew Morton wrote:
>> On Tue, 29 Jun 2010 14:05:52 +0200
>> Jens Axboe <[email protected]> wrote:
>>
>>> On 2010-06-29 14:05, Miklos Szeredi wrote:
>>>> On Tue, 29 Jun 2010, Jens Axboe wrote:
>>>>> On 2010-06-29 13:08, Miklos Szeredi wrote:
>>>>>> These patches fix bugs that affect real-world users of splice(2) and
>>>>>> sendfile(2) interfaces.
>>>>>>
>>>>>> Not sure if some or all of them are appropriate for 2.6.35 and/or
>>>>>> -stable?
>>>>>
>>>>> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
>>>>> a stable backport as well. Agree?
>>>>
>>>> Yes, sounds good.
>>>
>>> Good, that is what I queued up. I haven't pushed out the updated
>>> for-linus yet, since I asked Linus to pull it this morning.
>>>
>>
>> According to Tim's report on lkml
>> (http://lkml.org/lkml/2010/6/25/280),
>> cc56f7de7f00d188c7c4da1e9861581853b9e92f caused a regression.
>> cc56f7de7f00d188c7c4da1e9861581853b9e92f is present in 2.6.33 and in
>> 2.6.34 hence the fix should be backported into -stable.
>>
>> According to http://bugs.launchpad.net/bugs/588861, the patch "splice:
>> direct_splice_actor() should not use pos in sd" fixes the regression.
>>
>> However this:
>>
>> commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
>> Author: Changli Gao <[email protected]>
>> Date: Tue Jun 29 13:09:18 2010 +0200
>>
>> splice: direct_splice_actor() should not use pos in sd
>>
>> went into mainline without a cc:stable tag.
>
> I will send an email to stable@.

BTW, it's not in mainline yet. I know I forgot to mark it as stable,
but I can't tell them to include it before it's in mainline.


--
Jens Axboe


--
Jens Axboe

2010-07-08 17:26:59

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 0/3] splice fixes

On Thu, 08 Jul 2010 15:38:22 +0200 Jens Axboe <[email protected]> wrote:

> >> However this:
> >>
> >> commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
> >> Author: Changli Gao <[email protected]>
> >> Date: Tue Jun 29 13:09:18 2010 +0200
> >>
> >> splice: direct_splice_actor() should not use pos in sd
> >>
> >> went into mainline without a cc:stable tag.
> >
> > I will send an email to stable@.
>
> BTW, it's not in mainline yet. I know I forgot to mark it as stable,
> but I can't tell them to include it before it's in mainline.

You could redo the changelog then. But [email protected] is cc'ed on
these emails and they're splendid chaps ;)

2010-07-08 17:30:44

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/3] splice fixes

On 08/07/10 19.25, Andrew Morton wrote:
> On Thu, 08 Jul 2010 15:38:22 +0200 Jens Axboe <[email protected]> wrote:
>
>>>> However this:
>>>>
>>>> commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
>>>> Author: Changli Gao <[email protected]>
>>>> Date: Tue Jun 29 13:09:18 2010 +0200
>>>>
>>>> splice: direct_splice_actor() should not use pos in sd
>>>>
>>>> went into mainline without a cc:stable tag.
>>>
>>> I will send an email to stable@.
>>
>> BTW, it's not in mainline yet. I know I forgot to mark it as stable,
>> but I can't tell them to include it before it's in mainline.
>
> You could redo the changelog then. But [email protected] is cc'ed on
> these emails and they're splendid chaps ;)

But then the Linus monster ends up shouting at me, so I'll rather
take a little stable abuse :-)

What I've done in the past is just send stable@ a list of commits
once they are upstream. Less work for them. Best is remembering
the stable CC in the commit, but...

--
Jens Axboe