* [PATCH v2 2/2] virtio-net: Prevent NULL dereference
2011-09-28 14:40 [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb Sasha Levin
@ 2011-09-28 14:40 ` Sasha Levin
2011-10-03 18:40 ` Michael S. Tsirkin
` (2 more replies)
2011-10-03 18:19 ` [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb David Miller
` (2 subsequent siblings)
3 siblings, 3 replies; 14+ messages in thread
From: Sasha Levin @ 2011-09-28 14:40 UTC (permalink / raw)
To: linux-kernel
Cc: Sasha Levin, Rusty Russell, Michael S. Tsirkin, virtualization,
netdev, kvm
This patch prevents a NULL dereference when the user has passed a length
longer than an actual buffer to virtio-net.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Cc: kvm@vger.kernel.org
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
drivers/net/virtio_net.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index bde0dec..4a53d2a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -208,12 +208,22 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
return NULL;
}
- while (len) {
+ while (len && page) {
set_skb_frag(skb, page, offset, &len);
page = (struct page *)page->private;
offset = 0;
}
+ /*
+ * This is the case where we ran out of pages in our linked list, but
+ * supposedly have more data to read.
+ */
+ if (len > 0) {
+ pr_debug("%s: missing data to assemble skb\n", skb->dev->name);
+ dev_kfree_skb(skb);
+ return NULL;
+ }
+
if (page)
give_pages(vi, page);
--
1.7.6.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
2011-09-28 14:40 ` [PATCH v2 2/2] virtio-net: Prevent NULL dereference Sasha Levin
@ 2011-10-03 18:40 ` Michael S. Tsirkin
2011-10-05 13:50 ` Sasha Levin
2011-10-03 19:05 ` Michael S. Tsirkin
2011-10-06 19:41 ` David Miller
2 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2011-10-03 18:40 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
On Wed, Sep 28, 2011 at 05:40:55PM +0300, Sasha Levin wrote:
> This patch prevents a NULL dereference when the user has passed a length
> longer than an actual buffer to virtio-net.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> drivers/net/virtio_net.c | 12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index bde0dec..4a53d2a 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -208,12 +208,22 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> return NULL;
> }
>
> - while (len) {
> + while (len && page) {
> set_skb_frag(skb, page, offset, &len);
> page = (struct page *)page->private;
> offset = 0;
> }
>
> + /*
> + * This is the case where we ran out of pages in our linked list, but
> + * supposedly have more data to read.
Again, let's clarify that this only happens with broken devices.
> + */
> + if (len > 0) {
> + pr_debug("%s: missing data to assemble skb\n", skb->dev->name);
> + dev_kfree_skb(skb);
> + return NULL;
> + }
> +
> if (page)
> give_pages(vi, page);
>
> --
> 1.7.6.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
2011-10-03 18:40 ` Michael S. Tsirkin
@ 2011-10-05 13:50 ` Sasha Levin
2011-10-05 18:59 ` Michael S. Tsirkin
0 siblings, 1 reply; 14+ messages in thread
From: Sasha Levin @ 2011-10-05 13:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
On Mon, 2011-10-03 at 20:40 +0200, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2011 at 05:40:55PM +0300, Sasha Levin wrote:
> > This patch prevents a NULL dereference when the user has passed a length
> > longer than an actual buffer to virtio-net.
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> > drivers/net/virtio_net.c | 12 +++++++++++-
> > 1 files changed, 11 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index bde0dec..4a53d2a 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -208,12 +208,22 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > return NULL;
> > }
> >
> > - while (len) {
> > + while (len && page) {
> > set_skb_frag(skb, page, offset, &len);
> > page = (struct page *)page->private;
> > offset = 0;
> > }
> >
> > + /*
> > + * This is the case where we ran out of pages in our linked list, but
> > + * supposedly have more data to read.
>
> Again, let's clarify that this only happens with broken devices.
I think that the code within the if() makes it clear that it isn't the
regular path.
--
Sasha.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
2011-10-05 13:50 ` Sasha Levin
@ 2011-10-05 18:59 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2011-10-05 18:59 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
On Wed, Oct 05, 2011 at 03:50:14PM +0200, Sasha Levin wrote:
> On Mon, 2011-10-03 at 20:40 +0200, Michael S. Tsirkin wrote:
> > On Wed, Sep 28, 2011 at 05:40:55PM +0300, Sasha Levin wrote:
> > > This patch prevents a NULL dereference when the user has passed a length
> > > longer than an actual buffer to virtio-net.
> > >
> > > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > Cc: virtualization@lists.linux-foundation.org
> > > Cc: netdev@vger.kernel.org
> > > Cc: kvm@vger.kernel.org
> > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > > ---
> > > drivers/net/virtio_net.c | 12 +++++++++++-
> > > 1 files changed, 11 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index bde0dec..4a53d2a 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -208,12 +208,22 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > > return NULL;
> > > }
> > >
> > > - while (len) {
> > > + while (len && page) {
> > > set_skb_frag(skb, page, offset, &len);
> > > page = (struct page *)page->private;
> > > offset = 0;
> > > }
> > >
> > > + /*
> > > + * This is the case where we ran out of pages in our linked list, but
> > > + * supposedly have more data to read.
> >
> > Again, let's clarify that this only happens with broken devices.
>
> I think that the code within the if() makes it clear that it isn't the
> regular path.
It doesn't make it clear that this never happens in absence of bugs.
> --
>
> Sasha.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
2011-09-28 14:40 ` [PATCH v2 2/2] virtio-net: Prevent NULL dereference Sasha Levin
2011-10-03 18:40 ` Michael S. Tsirkin
@ 2011-10-03 19:05 ` Michael S. Tsirkin
2011-10-08 8:40 ` Sasha Levin
2011-10-06 19:41 ` David Miller
2 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2011-10-03 19:05 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
On Wed, Sep 28, 2011 at 05:40:55PM +0300, Sasha Levin wrote:
> This patch prevents a NULL dereference when the user has passed a length
> longer than an actual buffer to virtio-net.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> drivers/net/virtio_net.c | 12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index bde0dec..4a53d2a 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -208,12 +208,22 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> return NULL;
> }
>
> - while (len) {
> + while (len && page) {
I think if (unlikely(!page))
goto err_page;
would make the logic clearer. But see below
> set_skb_frag(skb, page, offset, &len);
> page = (struct page *)page->private;
> offset = 0;
> }
>
> + /*
> + * This is the case where we ran out of pages in our linked list, but
> + * supposedly have more data to read.
> + */
> + if (len > 0) {
> + pr_debug("%s: missing data to assemble skb\n", skb->dev->name);
> + dev_kfree_skb(skb);
> + return NULL;
> + }
> +
> if (page)
> give_pages(vi, page);
>
I thought about this some more. If length was too large host is
right now writing into pages that we have freed.
That is very bad, and I don't know what do do with it,
really not worth prettifying that IMO, NULL pointer is the least of our
worries.
But, with mergeable buffers at least, and that is the main mode anyway,
there is always a single page per buf, right?
So I think we should change the code,
and for mergeable buffers we shall only verify that
length <= PAGE_SIZE.
IOW copy a bit of code from page_to_skb(vi, page, len)
to receive_mergeable, maybe add a common function
if we need to avoid duplication.
> --
> 1.7.6.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
2011-10-03 19:05 ` Michael S. Tsirkin
@ 2011-10-08 8:40 ` Sasha Levin
0 siblings, 0 replies; 14+ messages in thread
From: Sasha Levin @ 2011-10-08 8:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
On Mon, 2011-10-03 at 21:05 +0200, Michael S. Tsirkin wrote:
> I thought about this some more. If length was too large host is
> right now writing into pages that we have freed.
> That is very bad, and I don't know what do do with it,
> really not worth prettifying that IMO, NULL pointer is the least of our
> worries.
>
> But, with mergeable buffers at least, and that is the main mode anyway,
> there is always a single page per buf, right?
> So I think we should change the code,
> and for mergeable buffers we shall only verify that
> length <= PAGE_SIZE.
In receive_mergeable() we already verify both 'page' being non-null and
length:
if (len > PAGE_SIZE)
len = PAGE_SIZE;
>
> IOW copy a bit of code from page_to_skb(vi, page, len)
> to receive_mergeable, maybe add a common function
> if we need to avoid duplication.
--
Sasha.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
2011-09-28 14:40 ` [PATCH v2 2/2] virtio-net: Prevent NULL dereference Sasha Levin
2011-10-03 18:40 ` Michael S. Tsirkin
2011-10-03 19:05 ` Michael S. Tsirkin
@ 2011-10-06 19:41 ` David Miller
2 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2011-10-06 19:41 UTC (permalink / raw)
To: levinsasha928; +Cc: linux-kernel, rusty, mst, virtualization, netdev, kvm
From: Sasha Levin <levinsasha928@gmail.com>
Date: Wed, 28 Sep 2011 17:40:55 +0300
> This patch prevents a NULL dereference when the user has passed a length
> longer than an actual buffer to virtio-net.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
Waiting for a respin of this patch with clarified comments.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb
2011-09-28 14:40 [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb Sasha Levin
2011-09-28 14:40 ` [PATCH v2 2/2] virtio-net: Prevent NULL dereference Sasha Levin
@ 2011-10-03 18:19 ` David Miller
2011-10-03 18:42 ` Michael S. Tsirkin
2011-10-03 19:04 ` Michael S. Tsirkin
2011-10-06 19:41 ` David Miller
3 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2011-10-03 18:19 UTC (permalink / raw)
To: levinsasha928; +Cc: linux-kernel, rusty, mst, virtualization, netdev, kvm
From: Sasha Levin <levinsasha928@gmail.com>
Date: Wed, 28 Sep 2011 17:40:54 +0300
> This patch verifies that the length of a buffer stored in a linked list
> of pages is small enough to fit into a skb.
>
> If the size is larger than a max size of a skb, it means that we shouldn't
> go ahead building skbs anyway since we won't be able to send the buffer as
> the user requested.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
Michael, tell me what's happening with these two virtio-net bug fixes.
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb
2011-10-03 18:19 ` [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb David Miller
@ 2011-10-03 18:42 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2011-10-03 18:42 UTC (permalink / raw)
To: David Miller
Cc: levinsasha928, linux-kernel, rusty, virtualization, netdev, kvm
On Mon, Oct 03, 2011 at 02:19:51PM -0400, David Miller wrote:
> From: Sasha Levin <levinsasha928@gmail.com>
> Date: Wed, 28 Sep 2011 17:40:54 +0300
>
> > This patch verifies that the length of a buffer stored in a linked list
> > of pages is small enough to fit into a skb.
> >
> > If the size is larger than a max size of a skb, it means that we shouldn't
> > go ahead building skbs anyway since we won't be able to send the buffer as
> > the user requested.
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
>
> Michael, tell me what's happening with these two virtio-net bug fixes.
>
> Thanks.
Thanks for the reminder.
They are more enhancements than bugfixes, in that both help debug buggy
hypervisors. So net-next material.
Patch 1 is ok patch 2 would benefit from a slightly clearer comment.
--
MST
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb
2011-09-28 14:40 [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb Sasha Levin
2011-09-28 14:40 ` [PATCH v2 2/2] virtio-net: Prevent NULL dereference Sasha Levin
2011-10-03 18:19 ` [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb David Miller
@ 2011-10-03 19:04 ` Michael S. Tsirkin
2011-10-05 13:50 ` Sasha Levin
2011-10-06 19:41 ` David Miller
3 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2011-10-03 19:04 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
On Wed, Sep 28, 2011 at 05:40:54PM +0300, Sasha Levin wrote:
> This patch verifies that the length of a buffer stored in a linked list
> of pages is small enough to fit into a skb.
>
> If the size is larger than a max size of a skb, it means that we shouldn't
> go ahead building skbs anyway since we won't be able to send the buffer as
> the user requested.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> drivers/net/virtio_net.c | 13 +++++++++++++
> 1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0c7321c..bde0dec 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -195,6 +195,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> len -= copy;
> offset += copy;
>
> + /*
> + * Verify that we can indeed put this data into a skb.
> + * This is here to handle cases when the device erroneously
> + * tries to receive more than is possible. This is usually
> + * the case of a broken device.
> + */
> + if (unlikely(len > MAX_SKB_FRAGS * PAGE_SIZE)) {
> + if (net_ratelimit())
> + pr_debug("%s: too much data\n", skb->dev->name);
> + dev_kfree_skb(skb);
> + return NULL;
> + }
> +
BTW, receive_mergeable does
pr_debug("%s: packet too long\n", skb->dev->name);
skb->dev->stats.rx_length_errors++;
which makes sense.
> while (len) {
> set_skb_frag(skb, page, offset, &len);
> page = (struct page *)page->private;
> --
> 1.7.6.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb
2011-10-03 19:04 ` Michael S. Tsirkin
@ 2011-10-05 13:50 ` Sasha Levin
2011-10-05 18:59 ` Michael S. Tsirkin
0 siblings, 1 reply; 14+ messages in thread
From: Sasha Levin @ 2011-10-05 13:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
On Mon, 2011-10-03 at 21:04 +0200, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2011 at 05:40:54PM +0300, Sasha Levin wrote:
> > This patch verifies that the length of a buffer stored in a linked list
> > of pages is small enough to fit into a skb.
> >
> > If the size is larger than a max size of a skb, it means that we shouldn't
> > go ahead building skbs anyway since we won't be able to send the buffer as
> > the user requested.
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> > drivers/net/virtio_net.c | 13 +++++++++++++
> > 1 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 0c7321c..bde0dec 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -195,6 +195,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > len -= copy;
> > offset += copy;
> >
> > + /*
> > + * Verify that we can indeed put this data into a skb.
> > + * This is here to handle cases when the device erroneously
> > + * tries to receive more than is possible. This is usually
> > + * the case of a broken device.
> > + */
> > + if (unlikely(len > MAX_SKB_FRAGS * PAGE_SIZE)) {
> > + if (net_ratelimit())
> > + pr_debug("%s: too much data\n", skb->dev->name);
> > + dev_kfree_skb(skb);
> > + return NULL;
> > + }
> > +
>
> BTW, receive_mergeable does
> pr_debug("%s: packet too long\n", skb->dev->name);
> skb->dev->stats.rx_length_errors++;
>
> which makes sense.
Do you think we should increase rx_length_errors here as well?
--
Sasha.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb
2011-10-05 13:50 ` Sasha Levin
@ 2011-10-05 18:59 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2011-10-05 18:59 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
On Wed, Oct 05, 2011 at 03:50:54PM +0200, Sasha Levin wrote:
> On Mon, 2011-10-03 at 21:04 +0200, Michael S. Tsirkin wrote:
> > On Wed, Sep 28, 2011 at 05:40:54PM +0300, Sasha Levin wrote:
> > > This patch verifies that the length of a buffer stored in a linked list
> > > of pages is small enough to fit into a skb.
> > >
> > > If the size is larger than a max size of a skb, it means that we shouldn't
> > > go ahead building skbs anyway since we won't be able to send the buffer as
> > > the user requested.
> > >
> > > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > > Cc: virtualization@lists.linux-foundation.org
> > > Cc: netdev@vger.kernel.org
> > > Cc: kvm@vger.kernel.org
> > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > > ---
> > > drivers/net/virtio_net.c | 13 +++++++++++++
> > > 1 files changed, 13 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 0c7321c..bde0dec 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -195,6 +195,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > > len -= copy;
> > > offset += copy;
> > >
> > > + /*
> > > + * Verify that we can indeed put this data into a skb.
> > > + * This is here to handle cases when the device erroneously
> > > + * tries to receive more than is possible. This is usually
> > > + * the case of a broken device.
> > > + */
> > > + if (unlikely(len > MAX_SKB_FRAGS * PAGE_SIZE)) {
> > > + if (net_ratelimit())
> > > + pr_debug("%s: too much data\n", skb->dev->name);
> > > + dev_kfree_skb(skb);
> > > + return NULL;
> > > + }
> > > +
> >
> > BTW, receive_mergeable does
> > pr_debug("%s: packet too long\n", skb->dev->name);
> > skb->dev->stats.rx_length_errors++;
> >
> > which makes sense.
>
> Do you think we should increase rx_length_errors here as well?
this is all debugging tool for devices/drivers, right?
so maybe not worth the noise.
> --
>
> Sasha.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb
2011-09-28 14:40 [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb Sasha Levin
` (2 preceding siblings ...)
2011-10-03 19:04 ` Michael S. Tsirkin
@ 2011-10-06 19:41 ` David Miller
3 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2011-10-06 19:41 UTC (permalink / raw)
To: levinsasha928; +Cc: linux-kernel, rusty, mst, virtualization, netdev, kvm
From: Sasha Levin <levinsasha928@gmail.com>
Date: Wed, 28 Sep 2011 17:40:54 +0300
> This patch verifies that the length of a buffer stored in a linked list
> of pages is small enough to fit into a skb.
>
> If the size is larger than a max size of a skb, it means that we shouldn't
> go ahead building skbs anyway since we won't be able to send the buffer as
> the user requested.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
Applied to net-next
^ permalink raw reply [flat|nested] 14+ messages in thread