diff options
author | Jesper Dangaard Brouer <brouer@redhat.com> | 2018-02-20 14:32:15 +0100 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2018-02-21 15:09:29 -0500 |
commit | 11b7d897ccc1fb5a3d3f9eb1e6b4574671e5dd7d (patch) | |
tree | 6827c4f93fc4deecb7bdeee6837a7e58b0bb5050 /drivers/net/virtio_net.c | |
parent | 95dbe9e7b3720efa5cf83d21f44f6d953f7cf4a2 (diff) | |
download | op-kernel-dev-11b7d897ccc1fb5a3d3f9eb1e6b4574671e5dd7d.zip op-kernel-dev-11b7d897ccc1fb5a3d3f9eb1e6b4574671e5dd7d.tar.gz |
virtio_net: fix memory leak in XDP_REDIRECT
XDP_REDIRECT calling xdp_do_redirect() can fail for multiple reasons
(which can be inspected by tracepoints). The current semantics is that
on failure the driver calling xdp_do_redirect() must handle freeing or
recycling the page associated with this frame. This can be seen as an
optimization, as drivers usually have an optimized XDP_DROP code path
for frame recycling in place already.
The virtio_net driver didn't handle when xdp_do_redirect() failed.
This caused a memory leak as the page refcnt wasn't decremented on
failures.
The function __virtnet_xdp_xmit() did handle one type of failure,
when the xmit queue virtqueue_add_outbuf() is full, which "hides"
releasing a refcnt on the page. Instead the function __virtnet_xdp_xmit()
must follow API of xdp_do_redirect(), which on errors leave it up to
the caller to free the page, of the failed send operation.
Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/virtio_net.c')
-rw-r--r-- | drivers/net/virtio_net.c | 37 |
1 files changed, 22 insertions, 15 deletions
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 10c8fc4..1e0e0fc 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -443,12 +443,8 @@ static bool __virtnet_xdp_xmit(struct virtnet_info *vi, sg_init_one(sq->sg, xdp->data, xdp->data_end - xdp->data); err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp->data, GFP_ATOMIC); - if (unlikely(err)) { - struct page *page = virt_to_head_page(xdp->data); - - put_page(page); - return false; - } + if (unlikely(err)) + return false; /* Caller handle free/refcnt */ return true; } @@ -546,8 +542,11 @@ static struct sk_buff *receive_small(struct net_device *dev, unsigned int buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); struct page *page = virt_to_head_page(buf); - unsigned int delta = 0, err; + unsigned int delta = 0; struct page *xdp_page; + bool sent; + int err; + len -= vi->hdr_len; rcu_read_lock(); @@ -596,16 +595,19 @@ static struct sk_buff *receive_small(struct net_device *dev, delta = orig_data - xdp.data; break; case XDP_TX: - if (unlikely(!__virtnet_xdp_xmit(vi, &xdp))) + sent = __virtnet_xdp_xmit(vi, &xdp); + if (unlikely(!sent)) { trace_xdp_exception(vi->dev, xdp_prog, act); - else - *xdp_xmit = true; + goto err_xdp; + } + *xdp_xmit = true; rcu_read_unlock(); goto xdp_xmit; case XDP_REDIRECT: err = xdp_do_redirect(dev, &xdp, xdp_prog); - if (!err) - *xdp_xmit = true; + if (err) + goto err_xdp; + *xdp_xmit = true; rcu_read_unlock(); goto xdp_xmit; default: @@ -677,6 +679,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, struct bpf_prog *xdp_prog; unsigned int truesize; unsigned int headroom = mergeable_ctx_to_headroom(ctx); + bool sent; head_skb = NULL; @@ -745,10 +748,14 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, } break; case XDP_TX: - if (unlikely(!__virtnet_xdp_xmit(vi, &xdp))) + sent = __virtnet_xdp_xmit(vi, &xdp); + if (unlikely(!sent)) { trace_xdp_exception(vi->dev, xdp_prog, act); - else - *xdp_xmit = true; + if (unlikely(xdp_page != page)) + put_page(xdp_page); + goto err_xdp; + } + *xdp_xmit = true; if (unlikely(xdp_page != page)) goto err_xdp; rcu_read_unlock(); |