* [PATCH net] tun: fix recovery from gup errors
@ 2013-06-23 14:19 Michael S. Tsirkin
2013-06-23 15:36 ` Sergei Shtylyov
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2013-06-23 14:19 UTC (permalink / raw)
To: linux-kernel
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
Brad Hubbard
get user pages might fail partially in tun zero copy
mode. To recover we need to put all pages that we got,
but code used a wrong index resulting in double-free
errors.
Reported-by: Brad Hubbard <bhubbard@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
I haven't figured out why do we get failures,
but recovery is clearly wrong.
This is also -stable material.
drivers/net/tun.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index bfa9bb4..c098b1e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1010,8 +1010,9 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
return -EMSGSIZE;
num_pages = get_user_pages_fast(base, size, 0, &page[i]);
if (num_pages != size) {
- for (i = 0; i < num_pages; i++)
- put_page(page[i]);
+ int j;
+ for (j = 0; j < num_pages; j++)
+ put_page(page[i + j]);
return -EFAULT;
}
truesize = size * PAGE_SIZE;
--
MST
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] tun: fix recovery from gup errors
2013-06-23 14:19 [PATCH net] tun: fix recovery from gup errors Michael S. Tsirkin
@ 2013-06-23 15:36 ` Sergei Shtylyov
2013-06-24 12:54 ` Michael S. Tsirkin
2013-06-24 3:22 ` Jason Wang
2013-06-24 11:31 ` Neil Horman
2 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2013-06-23 15:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David S. Miller, Jason Wang, Eric Dumazet,
Neil Horman, netdev, Brad Hubbard
Hello.
On 23-06-2013 18:19, Michael S. Tsirkin wrote:
> get user pages might fail partially in tun zero copy
> mode. To recover we need to put all pages that we got,
> but code used a wrong index resulting in double-free
> errors.
> Reported-by: Brad Hubbard <bhubbard@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> I haven't figured out why do we get failures,
> but recovery is clearly wrong.
> This is also -stable material.
> drivers/net/tun.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index bfa9bb4..c098b1e 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1010,8 +1010,9 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
> return -EMSGSIZE;
> num_pages = get_user_pages_fast(base, size, 0, &page[i]);
> if (num_pages != size) {
> - for (i = 0; i < num_pages; i++)
> - put_page(page[i]);
> + int j;
Empty line wouldn't hurt here, after declaration.
> + for (j = 0; j < num_pages; j++)
> + put_page(page[i + j]);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tun: fix recovery from gup errors
2013-06-23 14:19 [PATCH net] tun: fix recovery from gup errors Michael S. Tsirkin
2013-06-23 15:36 ` Sergei Shtylyov
@ 2013-06-24 3:22 ` Jason Wang
2013-06-25 23:24 ` David Miller
2013-06-24 11:31 ` Neil Horman
2 siblings, 1 reply; 7+ messages in thread
From: Jason Wang @ 2013-06-24 3:22 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David S. Miller, Eric Dumazet, Neil Horman, netdev,
Brad Hubbard
On 06/23/2013 10:19 PM, Michael S. Tsirkin wrote:
> get user pages might fail partially in tun zero copy
> mode. To recover we need to put all pages that we got,
> but code used a wrong index resulting in double-free
> errors.
>
> Reported-by: Brad Hubbard <bhubbard@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> I haven't figured out why do we get failures,
> but recovery is clearly wrong.
>
> This is also -stable material.
Acked-by: Jason Wang <jasowang@redhat.com>
> drivers/net/tun.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index bfa9bb4..c098b1e 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1010,8 +1010,9 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
> return -EMSGSIZE;
> num_pages = get_user_pages_fast(base, size, 0, &page[i]);
> if (num_pages != size) {
> - for (i = 0; i < num_pages; i++)
> - put_page(page[i]);
> + int j;
> + for (j = 0; j < num_pages; j++)
> + put_page(page[i + j]);
> return -EFAULT;
> }
> truesize = size * PAGE_SIZE;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tun: fix recovery from gup errors
2013-06-23 14:19 [PATCH net] tun: fix recovery from gup errors Michael S. Tsirkin
2013-06-23 15:36 ` Sergei Shtylyov
2013-06-24 3:22 ` Jason Wang
@ 2013-06-24 11:31 ` Neil Horman
2 siblings, 0 replies; 7+ messages in thread
From: Neil Horman @ 2013-06-24 11:31 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David S. Miller, Jason Wang, Eric Dumazet, netdev,
Brad Hubbard
On Sun, Jun 23, 2013 at 05:19:03PM +0300, Michael S. Tsirkin wrote:
> get user pages might fail partially in tun zero copy
> mode. To recover we need to put all pages that we got,
> but code used a wrong index resulting in double-free
> errors.
>
> Reported-by: Brad Hubbard <bhubbard@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> I haven't figured out why do we get failures,
> but recovery is clearly wrong.
>
> This is also -stable material.
>
> drivers/net/tun.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tun: fix recovery from gup errors
2013-06-23 15:36 ` Sergei Shtylyov
@ 2013-06-24 12:54 ` Michael S. Tsirkin
2013-06-24 18:34 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2013-06-24 12:54 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linux-kernel, David S. Miller, Jason Wang, Eric Dumazet,
Neil Horman, netdev, Brad Hubbard
On Sun, Jun 23, 2013 at 07:36:21PM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 23-06-2013 18:19, Michael S. Tsirkin wrote:
>
> >get user pages might fail partially in tun zero copy
> >mode. To recover we need to put all pages that we got,
> >but code used a wrong index resulting in double-free
> >errors.
>
> >Reported-by: Brad Hubbard <bhubbard@redhat.com>
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >---
>
> >I haven't figured out why do we get failures,
> >but recovery is clearly wrong.
>
> >This is also -stable material.
>
> > drivers/net/tun.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
>
> >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >index bfa9bb4..c098b1e 100644
> >--- a/drivers/net/tun.c
> >+++ b/drivers/net/tun.c
> >@@ -1010,8 +1010,9 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
> > return -EMSGSIZE;
> > num_pages = get_user_pages_fast(base, size, 0, &page[i]);
> > if (num_pages != size) {
> >- for (i = 0; i < num_pages; i++)
> >- put_page(page[i]);
> >+ int j;
>
> Empty line wouldn't hurt here, after declaration.
>
> >+ for (j = 0; j < num_pages; j++)
> >+ put_page(page[i + j]);
I think it's clearer without: this is the only code
within this block, declaration is really part of
the loop that comes after it.
An empty line would break it up visually.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tun: fix recovery from gup errors
2013-06-24 12:54 ` Michael S. Tsirkin
@ 2013-06-24 18:34 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2013-06-24 18:34 UTC (permalink / raw)
To: mst
Cc: sergei.shtylyov, linux-kernel, jasowang, edumazet, nhorman,
netdev, bhubbard
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 24 Jun 2013 15:54:12 +0300
> On Sun, Jun 23, 2013 at 07:36:21PM +0400, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 23-06-2013 18:19, Michael S. Tsirkin wrote:
>>
>> >get user pages might fail partially in tun zero copy
>> >mode. To recover we need to put all pages that we got,
>> >but code used a wrong index resulting in double-free
>> >errors.
>>
>> >Reported-by: Brad Hubbard <bhubbard@redhat.com>
>> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> >---
>>
>> >I haven't figured out why do we get failures,
>> >but recovery is clearly wrong.
>>
>> >This is also -stable material.
>>
>> > drivers/net/tun.c | 5 +++--
>> > 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> >index bfa9bb4..c098b1e 100644
>> >--- a/drivers/net/tun.c
>> >+++ b/drivers/net/tun.c
>> >@@ -1010,8 +1010,9 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
>> > return -EMSGSIZE;
>> > num_pages = get_user_pages_fast(base, size, 0, &page[i]);
>> > if (num_pages != size) {
>> >- for (i = 0; i < num_pages; i++)
>> >- put_page(page[i]);
>> >+ int j;
>>
>> Empty line wouldn't hurt here, after declaration.
>>
>> >+ for (j = 0; j < num_pages; j++)
>> >+ put_page(page[i + j]);
>
> I think it's clearer without: this is the only code
> within this block, declaration is really part of
> the loop that comes after it.
> An empty line would break it up visually.
No, really, an empty line after local variable declarations please.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] tun: fix recovery from gup errors
2013-06-24 3:22 ` Jason Wang
@ 2013-06-25 23:24 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2013-06-25 23:24 UTC (permalink / raw)
To: jasowang; +Cc: mst, linux-kernel, edumazet, nhorman, netdev, bhubbard
From: Jason Wang <jasowang@redhat.com>
Date: Mon, 24 Jun 2013 11:22:52 +0800
> On 06/23/2013 10:19 PM, Michael S. Tsirkin wrote:
>> get user pages might fail partially in tun zero copy
>> mode. To recover we need to put all pages that we got,
>> but code used a wrong index resulting in double-free
>> errors.
>>
>> Reported-by: Brad Hubbard <bhubbard@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>
>> I haven't figured out why do we get failures,
>> but recovery is clearly wrong.
>>
>> This is also -stable material.
>
> Acked-by: Jason Wang <jasowang@redhat.com>
Applied with the missing empty line added, I was tired of waiting for
Michael to take care of this himself.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-06-25 23:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-23 14:19 [PATCH net] tun: fix recovery from gup errors Michael S. Tsirkin
2013-06-23 15:36 ` Sergei Shtylyov
2013-06-24 12:54 ` Michael S. Tsirkin
2013-06-24 18:34 ` David Miller
2013-06-24 3:22 ` Jason Wang
2013-06-25 23:24 ` David Miller
2013-06-24 11:31 ` Neil Horman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).