* [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers [not found] <1387983339-1172-1-git-send-email-mst@redhat.com> @ 2013-12-25 14:56 ` Michael S. Tsirkin 2013-12-25 18:33 ` Michael Dalton 2013-12-26 7:28 ` Jason Wang 2013-12-25 14:56 ` [PATCH stable 2/2] virtio-net: make all RX paths handle erors consistently Michael S. Tsirkin 2013-12-25 20:13 ` [PATCH stable] virtio_net: don't leak memory or block when too many frags Michael S. Tsirkin 2 siblings, 2 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2013-12-25 14:56 UTC (permalink / raw) To: netdev Cc: Michael Dalton, linux-kernel, virtualization, Eric Dumazet, David Miller Eric Dumazet noticed that if we encounter an error when processing a mergeable buffer, we don't dequeue all of the buffers from this packet, the result is almost sure to be loss of networking. Jason Wang noticed that we also leak a page and that we don't decrement the rq buf count, so we won't repost buffers (a resource leak). Fix both issues. Cc: Rusty Russell <rusty@rustcorp.com.au> Cc: Michael Dalton <mwdalton@google.com> Cc: Eric Dumazet <edumazet@google.com> Cc: Jason Wang <jasowang@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: David S. Miller <davem@davemloft.net> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> (cherry picked from commit 8fc3b9e9a229778e5af3aa453c44f1a3857ba769) --- drivers/net/virtio_net.c | 66 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 9fbdfcd..435076f 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -297,26 +297,33 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, return skb; } -static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb) +static struct sk_buff *receive_mergeable(struct net_device *dev, + struct receive_queue *rq, + void *buf, + unsigned int len) { - struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb); - struct page *page; - int num_buf, i, len; + struct skb_vnet_hdr *hdr = page_address(buf); + int num_buf = hdr->mhdr.num_buffers; + struct page *page = buf; + struct sk_buff *skb = page_to_skb(rq, page, len); + int i; + + if (unlikely(!skb)) + goto err_skb; - num_buf = hdr->mhdr.num_buffers; while (--num_buf) { i = skb_shinfo(skb)->nr_frags; if (i >= MAX_SKB_FRAGS) { pr_debug("%s: packet too long\n", skb->dev->name); skb->dev->stats.rx_length_errors++; - return -EINVAL; + return NULL; } page = virtqueue_get_buf(rq->vq, &len); if (!page) { - pr_debug("%s: rx error: %d buffers missing\n", - skb->dev->name, hdr->mhdr.num_buffers); - skb->dev->stats.rx_length_errors++; - return -EINVAL; + pr_debug("%s: rx error: %d buffers %d missing\n", + dev->name, hdr->mhdr.num_buffers, num_buf); + dev->stats.rx_length_errors++; + goto err_buf; } if (len > PAGE_SIZE) @@ -326,7 +333,25 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb) --rq->num; } - return 0; + return skb; +err_skb: + give_pages(rq, page); + while (--num_buf) { + buf = virtqueue_get_buf(rq->vq, &len); + if (unlikely(!buf)) { + pr_debug("%s: rx error: %d buffers missing\n", + dev->name, num_buf); + dev->stats.rx_length_errors++; + break; + } + page = buf; + give_pages(rq, page); + --rq->num; + } +err_buf: + dev->stats.rx_dropped++; + dev_kfree_skb(skb); + return NULL; } static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) @@ -354,17 +379,18 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) skb_trim(skb, len); } else { page = buf; - skb = page_to_skb(rq, page, len); - if (unlikely(!skb)) { - dev->stats.rx_dropped++; - give_pages(rq, page); - return; - } - if (vi->mergeable_rx_bufs) - if (receive_mergeable(rq, skb)) { - dev_kfree_skb(skb); + if (vi->mergeable_rx_bufs) { + skb = receive_mergeable(dev, rq, page, len); + if (unlikely(!skb)) + return; + } else { + skb = page_to_skb(rq, page, len); + if (unlikely(!skb)) { + dev->stats.rx_dropped++; + give_pages(rq, page); return; } + } } hdr = skb_vnet_hdr(skb); -- MST ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers 2013-12-25 14:56 ` [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers Michael S. Tsirkin @ 2013-12-25 18:33 ` Michael Dalton 2013-12-25 19:19 ` Michael S. Tsirkin 2013-12-26 7:28 ` Jason Wang 1 sibling, 1 reply; 10+ messages in thread From: Michael Dalton @ 2013-12-25 18:33 UTC (permalink / raw) To: Michael S. Tsirkin Cc: netdev, linux-kernel, lf-virt, Eric Dumazet, David Miller Hi Michael, quick question below: On Wed, Dec 25, 2013 at 6:56 AM, Michael S. Tsirkin <mst@redhat.com> wrote: > if (i >= MAX_SKB_FRAGS) { > pr_debug("%s: packet too long\n", skb->dev->name); > skb->dev->stats.rx_length_errors++; > - return -EINVAL; > + return NULL; > } Should this error handling path free the SKB before returning NULL? It seems like if we just return NULL we may leak memory. Best, Mike ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers 2013-12-25 18:33 ` Michael Dalton @ 2013-12-25 19:19 ` Michael S. Tsirkin 2013-12-26 6:35 ` Michael Dalton 0 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2013-12-25 19:19 UTC (permalink / raw) To: Michael Dalton; +Cc: netdev, linux-kernel, lf-virt, Eric Dumazet, David Miller On Wed, Dec 25, 2013 at 10:33:37AM -0800, Michael Dalton wrote: > Hi Michael, quick question below: > > On Wed, Dec 25, 2013 at 6:56 AM, Michael S. Tsirkin <mst@redhat.com> wrote: > > if (i >= MAX_SKB_FRAGS) { > > pr_debug("%s: packet too long\n", skb->dev->name); > > skb->dev->stats.rx_length_errors++; > > - return -EINVAL; > > + return NULL; > > } > > Should this error handling path free the SKB before returning NULL? > It seems like if we just return NULL we may leak memory. > > Best, > > Mike It's a device error, but I agree, if we touch this code anyway there's no reason not to handle this consistently and do goto toward end of file. It's not a backport anymore though - this code is gone upstream, so I'll make it a separate patch I think. -- MST ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers 2013-12-25 19:19 ` Michael S. Tsirkin @ 2013-12-26 6:35 ` Michael Dalton 0 siblings, 0 replies; 10+ messages in thread From: Michael Dalton @ 2013-12-26 6:35 UTC (permalink / raw) To: Michael S. Tsirkin Cc: netdev, linux-kernel, lf-virt, Eric Dumazet, David Miller Acked-by: Michael Dalton <mwdalton@google.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers 2013-12-25 14:56 ` [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers Michael S. Tsirkin 2013-12-25 18:33 ` Michael Dalton @ 2013-12-26 7:28 ` Jason Wang 1 sibling, 0 replies; 10+ messages in thread From: Jason Wang @ 2013-12-26 7:28 UTC (permalink / raw) To: Michael S. Tsirkin, netdev Cc: Michael Dalton, linux-kernel, virtualization, Eric Dumazet, David Miller On 12/25/2013 10:56 PM, Michael S. Tsirkin wrote: > Eric Dumazet noticed that if we encounter an error > when processing a mergeable buffer, we don't > dequeue all of the buffers from this packet, > the result is almost sure to be loss of networking. > > Jason Wang noticed that we also leak a page and that we don't decrement > the rq buf count, so we won't repost buffers (a resource leak). This issue does not existed in stable tree. > Fix both issues. > > Cc: Rusty Russell <rusty@rustcorp.com.au> > Cc: Michael Dalton <mwdalton@google.com> > Cc: Eric Dumazet <edumazet@google.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: David S. Miller <davem@davemloft.net> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > (cherry picked from commit 8fc3b9e9a229778e5af3aa453c44f1a3857ba769) > --- > drivers/net/virtio_net.c | 66 +++++++++++++++++++++++++++++++++--------------- > 1 file changed, 46 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 9fbdfcd..435076f 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -297,26 +297,33 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, > return skb; > } > > -static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb) > +static struct sk_buff *receive_mergeable(struct net_device *dev, > + struct receive_queue *rq, > + void *buf, > + unsigned int len) > { > - struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb); > - struct page *page; > - int num_buf, i, len; > + struct skb_vnet_hdr *hdr = page_address(buf); > + int num_buf = hdr->mhdr.num_buffers; > + struct page *page = buf; > + struct sk_buff *skb = page_to_skb(rq, page, len); > + int i; > + > + if (unlikely(!skb)) > + goto err_skb; > > - num_buf = hdr->mhdr.num_buffers; > while (--num_buf) { > i = skb_shinfo(skb)->nr_frags; > if (i >= MAX_SKB_FRAGS) { > pr_debug("%s: packet too long\n", skb->dev->name); > skb->dev->stats.rx_length_errors++; > - return -EINVAL; > + return NULL; > } > page = virtqueue_get_buf(rq->vq, &len); > if (!page) { > - pr_debug("%s: rx error: %d buffers missing\n", > - skb->dev->name, hdr->mhdr.num_buffers); > - skb->dev->stats.rx_length_errors++; > - return -EINVAL; > + pr_debug("%s: rx error: %d buffers %d missing\n", > + dev->name, hdr->mhdr.num_buffers, num_buf); > + dev->stats.rx_length_errors++; > + goto err_buf; > } > > if (len > PAGE_SIZE) > @@ -326,7 +333,25 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb) > > --rq->num; > } > - return 0; > + return skb; > +err_skb: > + give_pages(rq, page); > + while (--num_buf) { > + buf = virtqueue_get_buf(rq->vq, &len); > + if (unlikely(!buf)) { > + pr_debug("%s: rx error: %d buffers missing\n", > + dev->name, num_buf); > + dev->stats.rx_length_errors++; > + break; > + } > + page = buf; > + give_pages(rq, page); > + --rq->num; > + } > +err_buf: > + dev->stats.rx_dropped++; > + dev_kfree_skb(skb); > + return NULL; > } > > static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) > @@ -354,17 +379,18 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) > skb_trim(skb, len); > } else { > page = buf; > - skb = page_to_skb(rq, page, len); > - if (unlikely(!skb)) { > - dev->stats.rx_dropped++; > - give_pages(rq, page); > - return; > - } > - if (vi->mergeable_rx_bufs) > - if (receive_mergeable(rq, skb)) { > - dev_kfree_skb(skb); > + if (vi->mergeable_rx_bufs) { > + skb = receive_mergeable(dev, rq, page, len); > + if (unlikely(!skb)) > + return; > + } else { > + skb = page_to_skb(rq, page, len); > + if (unlikely(!skb)) { > + dev->stats.rx_dropped++; > + give_pages(rq, page); > return; > } > + } > } > > hdr = skb_vnet_hdr(skb); ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH stable 2/2] virtio-net: make all RX paths handle erors consistently [not found] <1387983339-1172-1-git-send-email-mst@redhat.com> 2013-12-25 14:56 ` [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers Michael S. Tsirkin @ 2013-12-25 14:56 ` Michael S. Tsirkin 2013-12-26 7:28 ` Jason Wang 2013-12-26 7:31 ` Zhi Yong Wu 2013-12-25 20:13 ` [PATCH stable] virtio_net: don't leak memory or block when too many frags Michael S. Tsirkin 2 siblings, 2 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2013-12-25 14:56 UTC (permalink / raw) To: netdev; +Cc: linux-kernel, David Miller, virtualization receive mergeable now handles errors internally. Do same for big and small packet paths, otherwise the logic is too hard to follow. Cc: Jason Wang <jasowang@redhat.com> Cc: David S. Miller <davem@davemloft.net> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> (cherry picked from commit f121159d72091f25afb22007c833e60a6845e912) --- drivers/net/virtio_net.c | 56 +++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 435076f..c0ed6d5 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -297,6 +297,34 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, return skb; } +static struct sk_buff *receive_small(void *buf, unsigned int len) +{ + struct sk_buff * skb = buf; + + len -= sizeof(struct virtio_net_hdr); + skb_trim(skb, len); + + return skb; +} + +static struct sk_buff *receive_big(struct net_device *dev, + struct receive_queue *rq, + void *buf) +{ + struct page *page = buf; + struct sk_buff *skb = page_to_skb(rq, page, 0); + + if (unlikely(!skb)) + goto err; + + return skb; + +err: + dev->stats.rx_dropped++; + give_pages(rq, page); + return NULL; +} + static struct sk_buff *receive_mergeable(struct net_device *dev, struct receive_queue *rq, void *buf, @@ -360,7 +388,6 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) struct net_device *dev = vi->dev; struct virtnet_stats *stats = this_cpu_ptr(vi->stats); struct sk_buff *skb; - struct page *page; struct skb_vnet_hdr *hdr; if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) { @@ -372,26 +399,15 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) dev_kfree_skb(buf); return; } + if (vi->mergeable_rx_bufs) + skb = receive_mergeable(dev, rq, buf, len); + else if (vi->big_packets) + skb = receive_big(dev, rq, buf); + else + skb = receive_small(buf, len); - if (!vi->mergeable_rx_bufs && !vi->big_packets) { - skb = buf; - len -= sizeof(struct virtio_net_hdr); - skb_trim(skb, len); - } else { - page = buf; - if (vi->mergeable_rx_bufs) { - skb = receive_mergeable(dev, rq, page, len); - if (unlikely(!skb)) - return; - } else { - skb = page_to_skb(rq, page, len); - if (unlikely(!skb)) { - dev->stats.rx_dropped++; - give_pages(rq, page); - return; - } - } - } + if (unlikely(!skb)) + return; hdr = skb_vnet_hdr(skb); -- MST ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH stable 2/2] virtio-net: make all RX paths handle erors consistently 2013-12-25 14:56 ` [PATCH stable 2/2] virtio-net: make all RX paths handle erors consistently Michael S. Tsirkin @ 2013-12-26 7:28 ` Jason Wang 2013-12-26 7:31 ` Zhi Yong Wu 1 sibling, 0 replies; 10+ messages in thread From: Jason Wang @ 2013-12-26 7:28 UTC (permalink / raw) To: Michael S. Tsirkin, netdev; +Cc: linux-kernel, David Miller, virtualization On 12/25/2013 10:56 PM, Michael S. Tsirkin wrote: > receive mergeable now handles errors internally. > Do same for big and small packet paths, otherwise > the logic is too hard to follow. > > Cc: Jason Wang <jasowang@redhat.com> > Cc: David S. Miller <davem@davemloft.net> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > (cherry picked from commit f121159d72091f25afb22007c833e60a6845e912) > --- > drivers/net/virtio_net.c | 56 +++++++++++++++++++++++++++++++----------------- > 1 file changed, 36 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 435076f..c0ed6d5 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -297,6 +297,34 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, > return skb; > } > > +static struct sk_buff *receive_small(void *buf, unsigned int len) > +{ > + struct sk_buff * skb = buf; > + > + len -= sizeof(struct virtio_net_hdr); > + skb_trim(skb, len); > + > + return skb; > +} > + > +static struct sk_buff *receive_big(struct net_device *dev, > + struct receive_queue *rq, > + void *buf) > +{ > + struct page *page = buf; > + struct sk_buff *skb = page_to_skb(rq, page, 0); > + > + if (unlikely(!skb)) > + goto err; > + > + return skb; > + > +err: > + dev->stats.rx_dropped++; > + give_pages(rq, page); > + return NULL; > +} > + > static struct sk_buff *receive_mergeable(struct net_device *dev, > struct receive_queue *rq, > void *buf, > @@ -360,7 +388,6 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) > struct net_device *dev = vi->dev; > struct virtnet_stats *stats = this_cpu_ptr(vi->stats); > struct sk_buff *skb; > - struct page *page; > struct skb_vnet_hdr *hdr; > > if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) { > @@ -372,26 +399,15 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) > dev_kfree_skb(buf); > return; > } > + if (vi->mergeable_rx_bufs) > + skb = receive_mergeable(dev, rq, buf, len); > + else if (vi->big_packets) > + skb = receive_big(dev, rq, buf); > + else > + skb = receive_small(buf, len); > > - if (!vi->mergeable_rx_bufs && !vi->big_packets) { > - skb = buf; > - len -= sizeof(struct virtio_net_hdr); > - skb_trim(skb, len); > - } else { > - page = buf; > - if (vi->mergeable_rx_bufs) { > - skb = receive_mergeable(dev, rq, page, len); > - if (unlikely(!skb)) > - return; > - } else { > - skb = page_to_skb(rq, page, len); > - if (unlikely(!skb)) { > - dev->stats.rx_dropped++; > - give_pages(rq, page); > - return; > - } > - } > - } > + if (unlikely(!skb)) > + return; > > hdr = skb_vnet_hdr(skb); > Ack-ed by: Jason Wang <jasowang@redhat.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH stable 2/2] virtio-net: make all RX paths handle erors consistently 2013-12-25 14:56 ` [PATCH stable 2/2] virtio-net: make all RX paths handle erors consistently Michael S. Tsirkin 2013-12-26 7:28 ` Jason Wang @ 2013-12-26 7:31 ` Zhi Yong Wu 1 sibling, 0 replies; 10+ messages in thread From: Zhi Yong Wu @ 2013-12-26 7:31 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Linux Netdev List, lf-virt, linux-kernel mlist, David Miller typo in the subject.... s/erors/errors/ On Wed, Dec 25, 2013 at 10:56 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > receive mergeable now handles errors internally. > Do same for big and small packet paths, otherwise > the logic is too hard to follow. > > Cc: Jason Wang <jasowang@redhat.com> > Cc: David S. Miller <davem@davemloft.net> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > (cherry picked from commit f121159d72091f25afb22007c833e60a6845e912) > --- > drivers/net/virtio_net.c | 56 +++++++++++++++++++++++++++++++----------------- > 1 file changed, 36 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 435076f..c0ed6d5 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -297,6 +297,34 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, > return skb; > } > > +static struct sk_buff *receive_small(void *buf, unsigned int len) > +{ > + struct sk_buff * skb = buf; > + > + len -= sizeof(struct virtio_net_hdr); > + skb_trim(skb, len); > + > + return skb; > +} > + > +static struct sk_buff *receive_big(struct net_device *dev, > + struct receive_queue *rq, > + void *buf) > +{ > + struct page *page = buf; > + struct sk_buff *skb = page_to_skb(rq, page, 0); > + > + if (unlikely(!skb)) > + goto err; > + > + return skb; > + > +err: > + dev->stats.rx_dropped++; > + give_pages(rq, page); > + return NULL; > +} > + > static struct sk_buff *receive_mergeable(struct net_device *dev, > struct receive_queue *rq, > void *buf, > @@ -360,7 +388,6 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) > struct net_device *dev = vi->dev; > struct virtnet_stats *stats = this_cpu_ptr(vi->stats); > struct sk_buff *skb; > - struct page *page; > struct skb_vnet_hdr *hdr; > > if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) { > @@ -372,26 +399,15 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) > dev_kfree_skb(buf); > return; > } > + if (vi->mergeable_rx_bufs) > + skb = receive_mergeable(dev, rq, buf, len); > + else if (vi->big_packets) > + skb = receive_big(dev, rq, buf); > + else > + skb = receive_small(buf, len); > > - if (!vi->mergeable_rx_bufs && !vi->big_packets) { > - skb = buf; > - len -= sizeof(struct virtio_net_hdr); > - skb_trim(skb, len); > - } else { > - page = buf; > - if (vi->mergeable_rx_bufs) { > - skb = receive_mergeable(dev, rq, page, len); > - if (unlikely(!skb)) > - return; > - } else { > - skb = page_to_skb(rq, page, len); > - if (unlikely(!skb)) { > - dev->stats.rx_dropped++; > - give_pages(rq, page); > - return; > - } > - } > - } > + if (unlikely(!skb)) > + return; > > hdr = skb_vnet_hdr(skb); > > -- > MST > > _______________________________________________ > Virtualization mailing list > Virtualization@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization -- Regards, Zhi Yong Wu ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH stable] virtio_net: don't leak memory or block when too many frags [not found] <1387983339-1172-1-git-send-email-mst@redhat.com> 2013-12-25 14:56 ` [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers Michael S. Tsirkin 2013-12-25 14:56 ` [PATCH stable 2/2] virtio-net: make all RX paths handle erors consistently Michael S. Tsirkin @ 2013-12-25 20:13 ` Michael S. Tsirkin 2013-12-26 6:35 ` Michael Dalton 2 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2013-12-25 20:13 UTC (permalink / raw) To: netdev; +Cc: linux-kernel, virtualization, David Miller, Michael Dalton We leak an skb when there are too many frags, we also stop processing the packet in the middle, the result is almost sure to be loss of networking. Reported-by: Michael Dalton <mwdalton@google.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- Note: this path is gone upstream, so this patch is for stable kernel only. This is on top of series: virtio-net: backport error handling bugfix that I sent previously (and that this is in reply to). drivers/net/virtio_net.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index c0ed6d5..86fe1e7 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -344,7 +344,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, if (i >= MAX_SKB_FRAGS) { pr_debug("%s: packet too long\n", skb->dev->name); skb->dev->stats.rx_length_errors++; - return NULL; + goto err_frags; } page = virtqueue_get_buf(rq->vq, &len); if (!page) { @@ -364,6 +364,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, return skb; err_skb: give_pages(rq, page); while (--num_buf) { +err_frags: buf = virtqueue_get_buf(rq->vq, &len); if (unlikely(!buf)) { -- MST ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH stable] virtio_net: don't leak memory or block when too many frags 2013-12-25 20:13 ` [PATCH stable] virtio_net: don't leak memory or block when too many frags Michael S. Tsirkin @ 2013-12-26 6:35 ` Michael Dalton 0 siblings, 0 replies; 10+ messages in thread From: Michael Dalton @ 2013-12-26 6:35 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: netdev, David Miller, linux-kernel, lf-virt Acked-by: Michael Dalton <mwdalton@google.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-12-26 7:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1387983339-1172-1-git-send-email-mst@redhat.com>
2013-12-25 14:56 ` [PATCH stable 1/2] virtio_net: fix error handling for mergeable buffers Michael S. Tsirkin
2013-12-25 18:33 ` Michael Dalton
2013-12-25 19:19 ` Michael S. Tsirkin
2013-12-26 6:35 ` Michael Dalton
2013-12-26 7:28 ` Jason Wang
2013-12-25 14:56 ` [PATCH stable 2/2] virtio-net: make all RX paths handle erors consistently Michael S. Tsirkin
2013-12-26 7:28 ` Jason Wang
2013-12-26 7:31 ` Zhi Yong Wu
2013-12-25 20:13 ` [PATCH stable] virtio_net: don't leak memory or block when too many frags Michael S. Tsirkin
2013-12-26 6:35 ` Michael Dalton
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).