Commit 11b7d897 authored by Jesper Dangaard Brouer's avatar Jesper Dangaard Brouer Committed by David S. Miller
Browse files

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: 186b3c99 ("virtio-net: support XDP_REDIRECT")
Signed-off-by: default avatarJesper Dangaard Brouer <brouer@redhat.com>
Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 95dbe9e7

drivers/net/virtio_net.c

+22 −15
Original line number Diff line number Diff line
@@ -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,15 +595,18 @@ 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

				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)

			if (err)

				goto err_xdp;

			*xdp_xmit = true;

			rcu_read_unlock();

			goto xdp_xmit;
@@ -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,9 +748,13 @@ 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

				if (unlikely(xdp_page != page))

					put_page(xdp_page);

				goto err_xdp;

			}

			*xdp_xmit = true;

			if (unlikely(xdp_page != page))

				goto err_xdp;

CRA Git | Maintained and supported by SUSTech CRA and CCSE