Skip to content

Commit

Permalink
Merge branch 'virtio_net-XDP-fixes'
Browse files Browse the repository at this point in the history
Jesper Dangaard Brouer says:

====================
virtio_net: several bugs in XDP code for driver virtio_net

The virtio_net driver actually violates the original memory model of
XDP causing hard to debug crashes.  Per request of John Fastabend,
instead of removing the XDP feature I'm fixing as much as possible.
While testing virtio_net with XDP_REDIRECT I found 4 different bugs.

Patch-1: not enough tail-room for build_skb in receive_mergeable()
 only option is to disable XDP_REDIRECT in receive_mergeable()

Patch-2: XDP in receive_small() basically never worked (check wrong flag)

Patch-3: fix memory leak for XDP_REDIRECT in error cases

Patch-4: avoid crash when ndo_xdp_xmit is called on dev not ready for XDP

In the longer run, we should consider introducing a separate receive
function when attaching an XDP program, and also change the memory
model to be compatible with XDP when attaching an XDP prog.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Feb 21, 2018
2 parents 9c4ff2a + 8dcc5b0 commit 6c4df17
Showing 1 changed file with 34 additions and 24 deletions.
58 changes: 34 additions & 24 deletions drivers/net/virtio_net.c
Original file line number Diff line number Diff line change
Expand Up @@ -443,21 +443,27 @@ 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;
}

static int virtnet_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
{
struct virtnet_info *vi = netdev_priv(dev);
bool sent = __virtnet_xdp_xmit(vi, xdp);
struct receive_queue *rq = vi->rq;
struct bpf_prog *xdp_prog;
bool sent;

/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
* indicate XDP resources have been successfully allocated.
*/
xdp_prog = rcu_dereference(rq->xdp_prog);
if (!xdp_prog)
return -ENXIO;

sent = __virtnet_xdp_xmit(vi, xdp);
if (!sent)
return -ENOSPC;
return 0;
Expand Down Expand Up @@ -546,8 +552,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();
Expand All @@ -558,7 +567,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
void *orig_data;
u32 act;

if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags))
if (unlikely(hdr->hdr.gso_type))
goto err_xdp;

if (unlikely(xdp_headroom < virtnet_get_headroom(vi))) {
Expand Down Expand Up @@ -596,16 +605,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:
Expand Down Expand Up @@ -677,7 +689,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);
int err;
bool sent;

head_skb = NULL;

Expand Down Expand Up @@ -746,20 +758,18 @@ 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();
goto xdp_xmit;
case XDP_REDIRECT:
err = xdp_do_redirect(dev, &xdp, xdp_prog);
if (!err)
*xdp_xmit = true;
rcu_read_unlock();
goto xdp_xmit;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
Expand Down

0 comments on commit 6c4df17

Please sign in to comment.