Message ID | 20230414060835.74975-1-xuanzhuo@linux.alibaba.com (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net,v1] virtio_net: bugfix overflow inside xdp_linearize_page() | expand |
On Fri, Apr 14, 2023 at 02:08:35PM +0800, Xuan Zhuo wrote: > Here we copy the data from the original buf to the new page. But we > not check that it may be overflow. > > As long as the size received(including vnethdr) is greater than 3840 > (PAGE_SIZE -VIRTIO_XDP_HEADROOM). Then the memcpy will overflow. > > And this is completely possible, as long as the MTU is large, such > as 4096. In our test environment, this will cause crash. Since crash is > caused by the written memory, it is meaningless, so I do not include it. > > Fixes: 72979a6c3590 ("virtio_net: xdp, add slowpath case for non contiguous buffers") > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > Acked-by: Jason Wang <jasowang@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > v1: add Fixes tag > > drivers/net/virtio_net.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 2396c28c0122..ea1bd4bb326d 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -814,8 +814,13 @@ static struct page *xdp_linearize_page(struct receive_queue *rq, > int page_off, > unsigned int *len) > { > - struct page *page = alloc_page(GFP_ATOMIC); > + int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > + struct page *page; > + > + if (page_off + *len + tailroom > PAGE_SIZE) > + return NULL; > > + page = alloc_page(GFP_ATOMIC); > if (!page) > return NULL; > > @@ -823,7 +828,6 @@ static struct page *xdp_linearize_page(struct receive_queue *rq, > page_off += *len; > > while (--*num_buf) { > - int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > unsigned int buflen; > void *buf; > int off; > -- > 2.32.0.3.g01195cf9f
On Fri, 14 Apr 2023 14:08:35 +0800 Xuan Zhuo wrote: > Here we copy the data from the original buf to the new page. But we > not check that it may be overflow. > > As long as the size received(including vnethdr) is greater than 3840 > (PAGE_SIZE -VIRTIO_XDP_HEADROOM). Then the memcpy will overflow. > > And this is completely possible, as long as the MTU is large, such > as 4096. In our test environment, this will cause crash. Since crash is > caused by the written memory, it is meaningless, so I do not include it. > > Fixes: 72979a6c3590 ("virtio_net: xdp, add slowpath case for non contiguous buffers") > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > Acked-by: Jason Wang <jasowang@redhat.com> Applied, thanks! Commit 853618d5886b in net.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 2396c28c0122..ea1bd4bb326d 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -814,8 +814,13 @@ static struct page *xdp_linearize_page(struct receive_queue *rq, int page_off, unsigned int *len) { - struct page *page = alloc_page(GFP_ATOMIC); + int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); + struct page *page; + + if (page_off + *len + tailroom > PAGE_SIZE) + return NULL; + page = alloc_page(GFP_ATOMIC); if (!page) return NULL; @@ -823,7 +828,6 @@ static struct page *xdp_linearize_page(struct receive_queue *rq, page_off += *len; while (--*num_buf) { - int tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); unsigned int buflen; void *buf; int off;