[PATCH v2 net 0/7] virtio_net: Fix problems around XDP tx and napi_tx

[PATCH v2 net 0/7] virtio_net: Fix problems around XDP tx and napi_tx

[Toshiaki Makita]

While I'm looking into how to account standard tx counters on XDP tx
processing, I found several bugs around XDP tx and napi_tx.

Patch1: Fix oops on error path. Patch2 depends on this.
Patch2: Fix memory corruption on freeing xdp_frames with napi_tx enabled.
Patch3: Minor fix patch5 depends on.
Patch4: Fix memory corruption on processing xdp_frames when XDP is disabled.
  Also patch5 depends on this.
Patch5: Fix memory corruption on processing xdp_frames while XDP is being
  disabled.
Patch6: Minor fix patch7 depends on.
Patch7: Fix memory corruption on freeing sk_buff or xdp_frames when a normal
  queue is reused for XDP and vise versa.

v2:
- patch5: Make rcu_assign_pointer/synchronize_net conditional instead of
          _virtnet_set_queues.
- patch7: Use napi_consume_skb() instead of dev_consume_skb_any()

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Toshiaki Makita (7):
  virtio_net: Don't enable NAPI when interface is down
  virtio_net: Don't call free_old_xmit_skbs for xdp_frames
  virtio_net: Fix not restoring real_num_rx_queues
  virtio_net: Fix out of bounds access of sq
  virtio_net: Don't process redirected XDP frames when XDP is disabled
  virtio_net: Use xdp_return_frame to free xdp_frames on destroying vqs
  virtio_net: Differentiate sk_buff and xdp_frame on freeing

 drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 112 insertions(+), 47 deletions(-)

-- 
1.8.3.1

[PATCH v2 net 1/7] virtio_net: Don't enable NAPI when interface is down

[Toshiaki Makita]

Commit 4e09ff536284 ("virtio-net: disable NAPI only when enabled during
XDP set") tried to fix inappropriate NAPI enabling/disabling when
!netif_running(), but was not complete.

On error path virtio_net could enable NAPI even when !netif_running().
This can cause enabling NAPI twice on virtnet_open(), which would
trigger BUG_ON() in napi_enable().

Fixes: 4941d472bf95b ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8fadd8e..8e4c5d4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2430,8 +2430,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return 0;
 
 err:
-	for (i = 0; i < vi->max_queue_pairs; i++)
-		virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+	if (netif_running(dev)) {
+		for (i = 0; i < vi->max_queue_pairs; i++)
+			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+	}
 	if (prog)
 		bpf_prog_sub(prog, vi->max_queue_pairs - 1);
 	return err;
-- 
1.8.3.1

[PATCH v2 net 1/7] virtio_net: Don't enable NAPI when interface is down

[Toshiaki Makita]

Commit 4e09ff536284 ("virtio-net: disable NAPI only when enabled during
XDP set") tried to fix inappropriate NAPI enabling/disabling when
!netif_running(), but was not complete.

On error path virtio_net could enable NAPI even when !netif_running().
This can cause enabling NAPI twice on virtnet_open(), which would
trigger BUG_ON() in napi_enable().

Fixes: 4941d472bf95b ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8fadd8e..8e4c5d4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2430,8 +2430,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return 0;
 
 err:
-	for (i = 0; i < vi->max_queue_pairs; i++)
-		virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+	if (netif_running(dev)) {
+		for (i = 0; i < vi->max_queue_pairs; i++)
+			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+	}
 	if (prog)
 		bpf_prog_sub(prog, vi->max_queue_pairs - 1);
 	return err;
-- 
1.8.3.1

[PATCH v2 net 2/7] virtio_net: Don't call free_old_xmit_skbs for xdp_frames

[Toshiaki Makita]

When napi_tx is enabled, virtnet_poll_cleantx() called
free_old_xmit_skbs() even for xdp send queue.
This is bogus since the queue has xdp_frames, not sk_buffs, thus mangled
device tx bytes counters because skb->len is meaningless value, and even
triggered oops due to general protection fault on freeing them.

Since xdp send queues do not aquire locks, old xdp_frames should be
freed only in virtnet_xdp_xmit(), so just skip free_old_xmit_skbs() for
xdp send queues.

Similarly virtnet_poll_tx() called free_old_xmit_skbs(). This NAPI
handler is called even without calling start_xmit() because cb for tx is
by default enabled. Once the handler is called, it enabled the cb again,
and then the handler would be called again. We don't need this handler
for XDP, so don't enable cb as well as not calling free_old_xmit_skbs().

Also, we need to disable tx NAPI when disabling XDP, so
virtnet_poll_tx() can safely access curr_queue_pairs and
xdp_queue_pairs, which are not atomically updated while disabling XDP.

Fixes: b92f1e6751a6 ("virtio-net: transmit napi")
Fixes: 7b0411ef4aa6 ("virtio-net: clean tx descriptors from rx napi")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 49 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8e4c5d4..046f955 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1358,6 +1358,16 @@ static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
 	u64_stats_update_end(&sq->stats.syncp);
 }
 
+static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
+{
+	if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
+		return false;
+	else if (q < vi->curr_queue_pairs)
+		return true;
+	else
+		return false;
+}
+
 static void virtnet_poll_cleantx(struct receive_queue *rq)
 {
 	struct virtnet_info *vi = rq->vq->vdev->priv;
@@ -1365,7 +1375,7 @@ static void virtnet_poll_cleantx(struct receive_queue *rq)
 	struct send_queue *sq = &vi->sq[index];
 	struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, index);
 
-	if (!sq->napi.weight)
+	if (!sq->napi.weight || is_xdp_raw_buffer_queue(vi, index))
 		return;
 
 	if (__netif_tx_trylock(txq)) {
@@ -1442,8 +1452,16 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget)
 {
 	struct send_queue *sq = container_of(napi, struct send_queue, napi);
 	struct virtnet_info *vi = sq->vq->vdev->priv;
-	struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq));
+	unsigned int index = vq2txq(sq->vq);
+	struct netdev_queue *txq;
 
+	if (unlikely(is_xdp_raw_buffer_queue(vi, index))) {
+		/* We don't need to enable cb for XDP */
+		napi_complete_done(napi, 0);
+		return 0;
+	}
+
+	txq = netdev_get_tx_queue(vi->dev, index);
 	__netif_tx_lock(txq, raw_smp_processor_id());
 	free_old_xmit_skbs(sq, true);
 	__netif_tx_unlock(txq);
@@ -2402,9 +2420,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	}
 
 	/* Make sure NAPI is not using any XDP TX queues for RX. */
-	if (netif_running(dev))
-		for (i = 0; i < vi->max_queue_pairs; i++)
+	if (netif_running(dev)) {
+		for (i = 0; i < vi->max_queue_pairs; i++) {
 			napi_disable(&vi->rq[i].napi);
+			virtnet_napi_tx_disable(&vi->sq[i].napi);
+		}
+	}
 
 	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
@@ -2423,16 +2444,22 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		}
 		if (old_prog)
 			bpf_prog_put(old_prog);
-		if (netif_running(dev))
+		if (netif_running(dev)) {
 			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+			virtnet_napi_tx_enable(vi, vi->sq[i].vq,
+					       &vi->sq[i].napi);
+		}
 	}
 
 	return 0;
 
 err:
 	if (netif_running(dev)) {
-		for (i = 0; i < vi->max_queue_pairs; i++)
+		for (i = 0; i < vi->max_queue_pairs; i++) {
 			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+			virtnet_napi_tx_enable(vi, vi->sq[i].vq,
+					       &vi->sq[i].napi);
+		}
 	}
 	if (prog)
 		bpf_prog_sub(prog, vi->max_queue_pairs - 1);
@@ -2615,16 +2642,6 @@ static void free_receive_page_frags(struct virtnet_info *vi)
 			put_page(vi->rq[i].alloc_frag.page);
 }
 
-static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
-{
-	if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
-		return false;
-	else if (q < vi->curr_queue_pairs)
-		return true;
-	else
-		return false;
-}
-
 static void free_unused_bufs(struct virtnet_info *vi)
 {
 	void *buf;
-- 
1.8.3.1

[PATCH v2 net 2/7] virtio_net: Don't call free_old_xmit_skbs for xdp_frames

[Toshiaki Makita]

When napi_tx is enabled, virtnet_poll_cleantx() called
free_old_xmit_skbs() even for xdp send queue.
This is bogus since the queue has xdp_frames, not sk_buffs, thus mangled
device tx bytes counters because skb->len is meaningless value, and even
triggered oops due to general protection fault on freeing them.

Since xdp send queues do not aquire locks, old xdp_frames should be
freed only in virtnet_xdp_xmit(), so just skip free_old_xmit_skbs() for
xdp send queues.

Similarly virtnet_poll_tx() called free_old_xmit_skbs(). This NAPI
handler is called even without calling start_xmit() because cb for tx is
by default enabled. Once the handler is called, it enabled the cb again,
and then the handler would be called again. We don't need this handler
for XDP, so don't enable cb as well as not calling free_old_xmit_skbs().

Also, we need to disable tx NAPI when disabling XDP, so
virtnet_poll_tx() can safely access curr_queue_pairs and
xdp_queue_pairs, which are not atomically updated while disabling XDP.

Fixes: b92f1e6751a6 ("virtio-net: transmit napi")
Fixes: 7b0411ef4aa6 ("virtio-net: clean tx descriptors from rx napi")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 49 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8e4c5d4..046f955 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1358,6 +1358,16 @@ static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
 	u64_stats_update_end(&sq->stats.syncp);
 }
 
+static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
+{
+	if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
+		return false;
+	else if (q < vi->curr_queue_pairs)
+		return true;
+	else
+		return false;
+}
+
 static void virtnet_poll_cleantx(struct receive_queue *rq)
 {
 	struct virtnet_info *vi = rq->vq->vdev->priv;
@@ -1365,7 +1375,7 @@ static void virtnet_poll_cleantx(struct receive_queue *rq)
 	struct send_queue *sq = &vi->sq[index];
 	struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, index);
 
-	if (!sq->napi.weight)
+	if (!sq->napi.weight || is_xdp_raw_buffer_queue(vi, index))
 		return;
 
 	if (__netif_tx_trylock(txq)) {
@@ -1442,8 +1452,16 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget)
 {
 	struct send_queue *sq = container_of(napi, struct send_queue, napi);
 	struct virtnet_info *vi = sq->vq->vdev->priv;
-	struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq));
+	unsigned int index = vq2txq(sq->vq);
+	struct netdev_queue *txq;
 
+	if (unlikely(is_xdp_raw_buffer_queue(vi, index))) {
+		/* We don't need to enable cb for XDP */
+		napi_complete_done(napi, 0);
+		return 0;
+	}
+
+	txq = netdev_get_tx_queue(vi->dev, index);
 	__netif_tx_lock(txq, raw_smp_processor_id());
 	free_old_xmit_skbs(sq, true);
 	__netif_tx_unlock(txq);
@@ -2402,9 +2420,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	}
 
 	/* Make sure NAPI is not using any XDP TX queues for RX. */
-	if (netif_running(dev))
-		for (i = 0; i < vi->max_queue_pairs; i++)
+	if (netif_running(dev)) {
+		for (i = 0; i < vi->max_queue_pairs; i++) {
 			napi_disable(&vi->rq[i].napi);
+			virtnet_napi_tx_disable(&vi->sq[i].napi);
+		}
+	}
 
 	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
@@ -2423,16 +2444,22 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		}
 		if (old_prog)
 			bpf_prog_put(old_prog);
-		if (netif_running(dev))
+		if (netif_running(dev)) {
 			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+			virtnet_napi_tx_enable(vi, vi->sq[i].vq,
+					       &vi->sq[i].napi);
+		}
 	}
 
 	return 0;
 
 err:
 	if (netif_running(dev)) {
-		for (i = 0; i < vi->max_queue_pairs; i++)
+		for (i = 0; i < vi->max_queue_pairs; i++) {
 			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
+			virtnet_napi_tx_enable(vi, vi->sq[i].vq,
+					       &vi->sq[i].napi);
+		}
 	}
 	if (prog)
 		bpf_prog_sub(prog, vi->max_queue_pairs - 1);
@@ -2615,16 +2642,6 @@ static void free_receive_page_frags(struct virtnet_info *vi)
 			put_page(vi->rq[i].alloc_frag.page);
 }
 
-static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
-{
-	if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
-		return false;
-	else if (q < vi->curr_queue_pairs)
-		return true;
-	else
-		return false;
-}
-
 static void free_unused_bufs(struct virtnet_info *vi)
 {
 	void *buf;
-- 
1.8.3.1

[PATCH v2 net 3/7] virtio_net: Fix not restoring real_num_rx_queues

[Toshiaki Makita]

When _virtnet_set_queues() failed we did not restore real_num_rx_queues.
Fix this by placing the change of real_num_rx_queues after
_virtnet_set_queues().
This order is also in line with virtnet_set_channels().

Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 046f955..0e1a369 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2427,10 +2427,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		}
 	}
 
-	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
 	if (err)
 		goto err;
+	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 	vi->xdp_queue_pairs = xdp_qp;
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
-- 
1.8.3.1

[PATCH v2 net 3/7] virtio_net: Fix not restoring real_num_rx_queues

[Toshiaki Makita]

When _virtnet_set_queues() failed we did not restore real_num_rx_queues.
Fix this by placing the change of real_num_rx_queues after
_virtnet_set_queues().
This order is also in line with virtnet_set_channels().

Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 046f955..0e1a369 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2427,10 +2427,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		}
 	}
 
-	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
 	if (err)
 		goto err;
+	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 	vi->xdp_queue_pairs = xdp_qp;
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
-- 
1.8.3.1

[PATCH v2 net 4/7] virtio_net: Fix out of bounds access of sq

[Toshiaki Makita]

When XDP is disabled, curr_queue_pairs + smp_processor_id() can be
larger than max_queue_pairs.
There is no guarantee that we have enough XDP send queues dedicated for
each cpu when XDP is disabled, so do not count drops on sq in that case.

Fixes: 5b8f3c8d30a6 ("virtio_net: Add XDP related stats")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0e1a369..669b65c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -491,20 +491,17 @@ static int virtnet_xdp_xmit(struct net_device *dev,
 	int ret, err;
 	int i;
 
-	sq = virtnet_xdp_sq(vi);
-
-	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
-		ret = -EINVAL;
-		drops = n;
-		goto out;
-	}
-
 	/* 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) {
-		ret = -ENXIO;
+	if (!xdp_prog)
+		return -ENXIO;
+
+	sq = virtnet_xdp_sq(vi);
+
+	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
+		ret = -EINVAL;
 		drops = n;
 		goto out;
 	}
-- 
1.8.3.1

[PATCH v2 net 4/7] virtio_net: Fix out of bounds access of sq

[Toshiaki Makita]

When XDP is disabled, curr_queue_pairs + smp_processor_id() can be
larger than max_queue_pairs.
There is no guarantee that we have enough XDP send queues dedicated for
each cpu when XDP is disabled, so do not count drops on sq in that case.

Fixes: 5b8f3c8d30a6 ("virtio_net: Add XDP related stats")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0e1a369..669b65c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -491,20 +491,17 @@ static int virtnet_xdp_xmit(struct net_device *dev,
 	int ret, err;
 	int i;
 
-	sq = virtnet_xdp_sq(vi);
-
-	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
-		ret = -EINVAL;
-		drops = n;
-		goto out;
-	}
-
 	/* 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) {
-		ret = -ENXIO;
+	if (!xdp_prog)
+		return -ENXIO;
+
+	sq = virtnet_xdp_sq(vi);
+
+	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK)) {
+		ret = -EINVAL;
 		drops = n;
 		goto out;
 	}
-- 
1.8.3.1

[PATCH v2 net 5/7] virtio_net: Don't process redirected XDP frames when XDP is disabled

[Toshiaki Makita]

Commit 8dcc5b0ab0ec ("virtio_net: fix ndo_xdp_xmit crash towards dev not
ready for XDP") tried to avoid access to unexpected sq while XDP is
disabled, but was not complete.

There was a small window which causes out of bounds sq access in
virtnet_xdp_xmit() while disabling XDP.

An example case of
 - curr_queue_pairs = 6 (2 for SKB and 4 for XDP)
 - online_cpu_num = xdp_queue_paris = 4
when XDP is enabled:

CPU 0                         CPU 1
(Disabling XDP)               (Processing redirected XDP frames)

                              virtnet_xdp_xmit()
virtnet_xdp_set()
 _virtnet_set_queues()
  set curr_queue_pairs (2)
                               check if rq->xdp_prog is not NULL
                               virtnet_xdp_sq(vi)
                                qp = curr_queue_pairs -
                                     xdp_queue_pairs +
                                     smp_processor_id()
                                   = 2 - 4 + 1 = -1
                                sq = &vi->sq[qp] // out of bounds access
  set xdp_queue_pairs (0)
  rq->xdp_prog = NULL

Basically we should not change curr_queue_pairs and xdp_queue_pairs
while someone can read the values. Thus, when disabling XDP, assign NULL
to rq->xdp_prog first, and wait for RCU grace period, then change
xxx_queue_pairs.
Note that we need to keep the current order when enabling XDP though.

- v2: Make rcu_assign_pointer/synchronize_net conditional instead of
      _virtnet_set_queues.

Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 669b65c..cea52e4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2410,6 +2410,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		return -ENOMEM;
 	}
 
+	old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
+	if (!prog && !old_prog)
+		return 0;
+
 	if (prog) {
 		prog = bpf_prog_add(prog, vi->max_queue_pairs - 1);
 		if (IS_ERR(prog))
@@ -2424,21 +2428,30 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		}
 	}
 
+	if (!prog) {
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
+			if (i == 0)
+				virtnet_restore_guest_offloads(vi);
+		}
+		synchronize_net();
+	}
+
 	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
 	if (err)
 		goto err;
 	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 	vi->xdp_queue_pairs = xdp_qp;
 
-	for (i = 0; i < vi->max_queue_pairs; i++) {
-		old_prog = rtnl_dereference(vi->rq[i].xdp_prog);
-		rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
-		if (i == 0) {
-			if (!old_prog)
+	if (prog) {
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
+			if (i == 0 && !old_prog)
 				virtnet_clear_guest_offloads(vi);
-			if (!prog)
-				virtnet_restore_guest_offloads(vi);
 		}
+	}
+
+	for (i = 0; i < vi->max_queue_pairs; i++) {
 		if (old_prog)
 			bpf_prog_put(old_prog);
 		if (netif_running(dev)) {
@@ -2451,6 +2464,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return 0;
 
 err:
+	if (!prog) {
+		virtnet_clear_guest_offloads(vi);
+		for (i = 0; i < vi->max_queue_pairs; i++)
+			rcu_assign_pointer(vi->rq[i].xdp_prog, old_prog);
+	}
+
 	if (netif_running(dev)) {
 		for (i = 0; i < vi->max_queue_pairs; i++) {
 			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
-- 
1.8.3.1

[PATCH v2 net 5/7] virtio_net: Don't process redirected XDP frames when XDP is disabled

[Toshiaki Makita]

Commit 8dcc5b0ab0ec ("virtio_net: fix ndo_xdp_xmit crash towards dev not
ready for XDP") tried to avoid access to unexpected sq while XDP is
disabled, but was not complete.

There was a small window which causes out of bounds sq access in
virtnet_xdp_xmit() while disabling XDP.

An example case of
 - curr_queue_pairs = 6 (2 for SKB and 4 for XDP)
 - online_cpu_num = xdp_queue_paris = 4
when XDP is enabled:

CPU 0                         CPU 1
(Disabling XDP)               (Processing redirected XDP frames)

                              virtnet_xdp_xmit()
virtnet_xdp_set()
 _virtnet_set_queues()
  set curr_queue_pairs (2)
                               check if rq->xdp_prog is not NULL
                               virtnet_xdp_sq(vi)
                                qp = curr_queue_pairs -
                                     xdp_queue_pairs +
                                     smp_processor_id()
                                   = 2 - 4 + 1 = -1
                                sq = &vi->sq[qp] // out of bounds access
  set xdp_queue_pairs (0)
  rq->xdp_prog = NULL

Basically we should not change curr_queue_pairs and xdp_queue_pairs
while someone can read the values. Thus, when disabling XDP, assign NULL
to rq->xdp_prog first, and wait for RCU grace period, then change
xxx_queue_pairs.
Note that we need to keep the current order when enabling XDP though.

- v2: Make rcu_assign_pointer/synchronize_net conditional instead of
      _virtnet_set_queues.

Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 669b65c..cea52e4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2410,6 +2410,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		return -ENOMEM;
 	}
 
+	old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
+	if (!prog && !old_prog)
+		return 0;
+
 	if (prog) {
 		prog = bpf_prog_add(prog, vi->max_queue_pairs - 1);
 		if (IS_ERR(prog))
@@ -2424,21 +2428,30 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		}
 	}
 
+	if (!prog) {
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
+			if (i == 0)
+				virtnet_restore_guest_offloads(vi);
+		}
+		synchronize_net();
+	}
+
 	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
 	if (err)
 		goto err;
 	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 	vi->xdp_queue_pairs = xdp_qp;
 
-	for (i = 0; i < vi->max_queue_pairs; i++) {
-		old_prog = rtnl_dereference(vi->rq[i].xdp_prog);
-		rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
-		if (i == 0) {
-			if (!old_prog)
+	if (prog) {
+		for (i = 0; i < vi->max_queue_pairs; i++) {
+			rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
+			if (i == 0 && !old_prog)
 				virtnet_clear_guest_offloads(vi);
-			if (!prog)
-				virtnet_restore_guest_offloads(vi);
 		}
+	}
+
+	for (i = 0; i < vi->max_queue_pairs; i++) {
 		if (old_prog)
 			bpf_prog_put(old_prog);
 		if (netif_running(dev)) {
@@ -2451,6 +2464,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return 0;
 
 err:
+	if (!prog) {
+		virtnet_clear_guest_offloads(vi);
+		for (i = 0; i < vi->max_queue_pairs; i++)
+			rcu_assign_pointer(vi->rq[i].xdp_prog, old_prog);
+	}
+
 	if (netif_running(dev)) {
 		for (i = 0; i < vi->max_queue_pairs; i++) {
 			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
-- 
1.8.3.1

[PATCH v2 net 6/7] virtio_net: Use xdp_return_frame to free xdp_frames on destroying vqs

[Toshiaki Makita]

put_page() can work as a fallback for freeing xdp_frames, but the
appropriate way is to use xdp_return_frame().

Fixes: cac320c850ef ("virtio_net: convert to use generic xdp_frame and xdp_return_frame API")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index cea52e4..1d454ce 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2669,7 +2669,7 @@ static void free_unused_bufs(struct virtnet_info *vi)
 			if (!is_xdp_raw_buffer_queue(vi, i))
 				dev_kfree_skb(buf);
 			else
-				put_page(virt_to_head_page(buf));
+				xdp_return_frame(buf);
 		}
 	}
 
-- 
1.8.3.1

[PATCH v2 net 6/7] virtio_net: Use xdp_return_frame to free xdp_frames on destroying vqs

[Toshiaki Makita]

put_page() can work as a fallback for freeing xdp_frames, but the
appropriate way is to use xdp_return_frame().

Fixes: cac320c850ef ("virtio_net: convert to use generic xdp_frame and xdp_return_frame API")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index cea52e4..1d454ce 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2669,7 +2669,7 @@ static void free_unused_bufs(struct virtnet_info *vi)
 			if (!is_xdp_raw_buffer_queue(vi, i))
 				dev_kfree_skb(buf);
 			else
-				put_page(virt_to_head_page(buf));
+				xdp_return_frame(buf);
 		}
 	}
 
-- 
1.8.3.1

[PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Toshiaki Makita]

We do not reset or free up unused buffers when enabling/disabling XDP,
so it can happen that xdp_frames are freed after disabling XDP or
sk_buffs are freed after enabling XDP on xdp tx queues.
Thus we need to handle both forms (xdp_frames and sk_buffs) regardless
of XDP setting.
One way to trigger this problem is to disable XDP when napi_tx is
enabled. In that case, virtnet_xdp_set() calls virtnet_napi_enable()
which kicks NAPI. The NAPI handler will call virtnet_poll_cleantx()
which invokes free_old_xmit_skbs() for queues which have been used by
XDP.

Note that even with this change we need to keep skipping
free_old_xmit_skbs() from NAPI handlers when XDP is enabled, because XDP
tx queues do not aquire queue locks.

- v2: Use napi_consume_skb() instead of dev_consume_skb_any()

Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
NOTE: Dropped Acked-by because of the v2 change.

 drivers/net/virtio_net.c | 54 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1d454ce..2594481 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -57,6 +57,8 @@
 #define VIRTIO_XDP_TX		BIT(0)
 #define VIRTIO_XDP_REDIR	BIT(1)
 
+#define VIRTIO_XDP_FLAG	BIT(0)
+
 /* RX packet size EWMA. The average packet size is used to determine the packet
  * buffer size when refilling RX rings. As the entire RX ring may be refilled
  * at once, the weight is chosen so that the EWMA will be insensitive to short-
@@ -252,6 +254,21 @@ struct padded_vnet_hdr {
 	char padding[4];
 };
 
+static bool is_xdp_frame(void *ptr)
+{
+	return (unsigned long)ptr & VIRTIO_XDP_FLAG;
+}
+
+static void *xdp_to_ptr(struct xdp_frame *ptr)
+{
+	return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
+}
+
+static struct xdp_frame *ptr_to_xdp(void *ptr)
+{
+	return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
+}
+
 /* Converting between virtqueue no. and kernel tx/rx queue no.
  * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
  */
@@ -462,7 +479,8 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
 
 	sg_init_one(sq->sg, xdpf->data, xdpf->len);
 
-	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
+	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf),
+				   GFP_ATOMIC);
 	if (unlikely(err))
 		return -ENOSPC; /* Caller handle free/refcnt */
 
@@ -482,13 +500,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
 {
 	struct virtnet_info *vi = netdev_priv(dev);
 	struct receive_queue *rq = vi->rq;
-	struct xdp_frame *xdpf_sent;
 	struct bpf_prog *xdp_prog;
 	struct send_queue *sq;
 	unsigned int len;
 	int drops = 0;
 	int kicks = 0;
 	int ret, err;
+	void *ptr;
 	int i;
 
 	/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
@@ -507,8 +525,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
 	}
 
 	/* Free up any pending old buffers before queueing new ones. */
-	while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
-		xdp_return_frame(xdpf_sent);
+	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
+		if (likely(is_xdp_frame(ptr)))
+			xdp_return_frame(ptr_to_xdp(ptr));
+		else
+			napi_consume_skb(ptr, false);
+	}
 
 	for (i = 0; i < n; i++) {
 		struct xdp_frame *xdpf = frames[i];
@@ -1329,18 +1351,26 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
 
 static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
 {
-	struct sk_buff *skb;
 	unsigned int len;
 	unsigned int packets = 0;
 	unsigned int bytes = 0;
+	void *ptr;
 
-	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
-		pr_debug("Sent skb %p\n", skb);
+	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
+		if (likely(!is_xdp_frame(ptr))) {
+			struct sk_buff *skb = ptr;
 
-		bytes += skb->len;
-		packets++;
+			pr_debug("Sent skb %p\n", skb);
 
-		napi_consume_skb(skb, in_napi);
+			bytes += skb->len;
+			napi_consume_skb(skb, in_napi);
+		} else {
+			struct xdp_frame *frame = ptr_to_xdp(ptr);
+
+			bytes += frame->len;
+			xdp_return_frame(frame);
+		}
+		packets++;
 	}
 
 	/* Avoid overhead when no packets have been processed
@@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		struct virtqueue *vq = vi->sq[i].vq;
 		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
-			if (!is_xdp_raw_buffer_queue(vi, i))
+			if (!is_xdp_frame(buf))
 				dev_kfree_skb(buf);
 			else
-				xdp_return_frame(buf);
+				xdp_return_frame(ptr_to_xdp(buf));
 		}
 	}
 
-- 
1.8.3.1

[PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Toshiaki Makita]

We do not reset or free up unused buffers when enabling/disabling XDP,
so it can happen that xdp_frames are freed after disabling XDP or
sk_buffs are freed after enabling XDP on xdp tx queues.
Thus we need to handle both forms (xdp_frames and sk_buffs) regardless
of XDP setting.
One way to trigger this problem is to disable XDP when napi_tx is
enabled. In that case, virtnet_xdp_set() calls virtnet_napi_enable()
which kicks NAPI. The NAPI handler will call virtnet_poll_cleantx()
which invokes free_old_xmit_skbs() for queues which have been used by
XDP.

Note that even with this change we need to keep skipping
free_old_xmit_skbs() from NAPI handlers when XDP is enabled, because XDP
tx queues do not aquire queue locks.

- v2: Use napi_consume_skb() instead of dev_consume_skb_any()

Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
NOTE: Dropped Acked-by because of the v2 change.

 drivers/net/virtio_net.c | 54 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1d454ce..2594481 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -57,6 +57,8 @@
 #define VIRTIO_XDP_TX		BIT(0)
 #define VIRTIO_XDP_REDIR	BIT(1)
 
+#define VIRTIO_XDP_FLAG	BIT(0)
+
 /* RX packet size EWMA. The average packet size is used to determine the packet
  * buffer size when refilling RX rings. As the entire RX ring may be refilled
  * at once, the weight is chosen so that the EWMA will be insensitive to short-
@@ -252,6 +254,21 @@ struct padded_vnet_hdr {
 	char padding[4];
 };
 
+static bool is_xdp_frame(void *ptr)
+{
+	return (unsigned long)ptr & VIRTIO_XDP_FLAG;
+}
+
+static void *xdp_to_ptr(struct xdp_frame *ptr)
+{
+	return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
+}
+
+static struct xdp_frame *ptr_to_xdp(void *ptr)
+{
+	return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
+}
+
 /* Converting between virtqueue no. and kernel tx/rx queue no.
  * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
  */
@@ -462,7 +479,8 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
 
 	sg_init_one(sq->sg, xdpf->data, xdpf->len);
 
-	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
+	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf),
+				   GFP_ATOMIC);
 	if (unlikely(err))
 		return -ENOSPC; /* Caller handle free/refcnt */
 
@@ -482,13 +500,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
 {
 	struct virtnet_info *vi = netdev_priv(dev);
 	struct receive_queue *rq = vi->rq;
-	struct xdp_frame *xdpf_sent;
 	struct bpf_prog *xdp_prog;
 	struct send_queue *sq;
 	unsigned int len;
 	int drops = 0;
 	int kicks = 0;
 	int ret, err;
+	void *ptr;
 	int i;
 
 	/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
@@ -507,8 +525,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
 	}
 
 	/* Free up any pending old buffers before queueing new ones. */
-	while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
-		xdp_return_frame(xdpf_sent);
+	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
+		if (likely(is_xdp_frame(ptr)))
+			xdp_return_frame(ptr_to_xdp(ptr));
+		else
+			napi_consume_skb(ptr, false);
+	}
 
 	for (i = 0; i < n; i++) {
 		struct xdp_frame *xdpf = frames[i];
@@ -1329,18 +1351,26 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
 
 static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
 {
-	struct sk_buff *skb;
 	unsigned int len;
 	unsigned int packets = 0;
 	unsigned int bytes = 0;
+	void *ptr;
 
-	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
-		pr_debug("Sent skb %p\n", skb);
+	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
+		if (likely(!is_xdp_frame(ptr))) {
+			struct sk_buff *skb = ptr;
 
-		bytes += skb->len;
-		packets++;
+			pr_debug("Sent skb %p\n", skb);
 
-		napi_consume_skb(skb, in_napi);
+			bytes += skb->len;
+			napi_consume_skb(skb, in_napi);
+		} else {
+			struct xdp_frame *frame = ptr_to_xdp(ptr);
+
+			bytes += frame->len;
+			xdp_return_frame(frame);
+		}
+		packets++;
 	}
 
 	/* Avoid overhead when no packets have been processed
@@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		struct virtqueue *vq = vi->sq[i].vq;
 		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
-			if (!is_xdp_raw_buffer_queue(vi, i))
+			if (!is_xdp_frame(buf))
 				dev_kfree_skb(buf);
 			else
-				xdp_return_frame(buf);
+				xdp_return_frame(ptr_to_xdp(buf));
 		}
 	}
 
-- 
1.8.3.1

Re: [PATCH v2 net 5/7] virtio_net: Don't process redirected XDP frames when XDP is disabled

[Jason Wang]

On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> Commit 8dcc5b0ab0ec ("virtio_net: fix ndo_xdp_xmit crash towards dev not
> ready for XDP") tried to avoid access to unexpected sq while XDP is
> disabled, but was not complete.
>
> There was a small window which causes out of bounds sq access in
> virtnet_xdp_xmit() while disabling XDP.
>
> An example case of
>   - curr_queue_pairs = 6 (2 for SKB and 4 for XDP)
>   - online_cpu_num = xdp_queue_paris = 4
> when XDP is enabled:
>
> CPU 0                         CPU 1
> (Disabling XDP)               (Processing redirected XDP frames)
>
>                                virtnet_xdp_xmit()
> virtnet_xdp_set()
>   _virtnet_set_queues()
>    set curr_queue_pairs (2)
>                                 check if rq->xdp_prog is not NULL
>                                 virtnet_xdp_sq(vi)
>                                  qp = curr_queue_pairs -
>                                       xdp_queue_pairs +
>                                       smp_processor_id()
>                                     = 2 - 4 + 1 = -1
>                                  sq = &vi->sq[qp] // out of bounds access
>    set xdp_queue_pairs (0)
>    rq->xdp_prog = NULL
>
> Basically we should not change curr_queue_pairs and xdp_queue_pairs
> while someone can read the values. Thus, when disabling XDP, assign NULL
> to rq->xdp_prog first, and wait for RCU grace period, then change
> xxx_queue_pairs.
> Note that we need to keep the current order when enabling XDP though.
>
> - v2: Make rcu_assign_pointer/synchronize_net conditional instead of
>        _virtnet_set_queues.
>
> Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>   drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
>   1 file changed, 26 insertions(+), 7 deletions(-)


Acked-by: Jason Wang <jasowang@redhat.com>

Thanks.

Re: [PATCH v2 net 5/7] virtio_net: Don't process redirected XDP frames when XDP is disabled

[Jason Wang]

On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> Commit 8dcc5b0ab0ec ("virtio_net: fix ndo_xdp_xmit crash towards dev not
> ready for XDP") tried to avoid access to unexpected sq while XDP is
> disabled, but was not complete.
>
> There was a small window which causes out of bounds sq access in
> virtnet_xdp_xmit() while disabling XDP.
>
> An example case of
>   - curr_queue_pairs = 6 (2 for SKB and 4 for XDP)
>   - online_cpu_num = xdp_queue_paris = 4
> when XDP is enabled:
>
> CPU 0                         CPU 1
> (Disabling XDP)               (Processing redirected XDP frames)
>
>                                virtnet_xdp_xmit()
> virtnet_xdp_set()
>   _virtnet_set_queues()
>    set curr_queue_pairs (2)
>                                 check if rq->xdp_prog is not NULL
>                                 virtnet_xdp_sq(vi)
>                                  qp = curr_queue_pairs -
>                                       xdp_queue_pairs +
>                                       smp_processor_id()
>                                     = 2 - 4 + 1 = -1
>                                  sq = &vi->sq[qp] // out of bounds access
>    set xdp_queue_pairs (0)
>    rq->xdp_prog = NULL
>
> Basically we should not change curr_queue_pairs and xdp_queue_pairs
> while someone can read the values. Thus, when disabling XDP, assign NULL
> to rq->xdp_prog first, and wait for RCU grace period, then change
> xxx_queue_pairs.
> Note that we need to keep the current order when enabling XDP though.
>
> - v2: Make rcu_assign_pointer/synchronize_net conditional instead of
>        _virtnet_set_queues.
>
> Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>   drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
>   1 file changed, 26 insertions(+), 7 deletions(-)


Acked-by: Jason Wang <jasowang@redhat.com>

Thanks.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Jason Wang]

On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> We do not reset or free up unused buffers when enabling/disabling XDP,
> so it can happen that xdp_frames are freed after disabling XDP or
> sk_buffs are freed after enabling XDP on xdp tx queues.
> Thus we need to handle both forms (xdp_frames and sk_buffs) regardless
> of XDP setting.
> One way to trigger this problem is to disable XDP when napi_tx is
> enabled. In that case, virtnet_xdp_set() calls virtnet_napi_enable()
> which kicks NAPI. The NAPI handler will call virtnet_poll_cleantx()
> which invokes free_old_xmit_skbs() for queues which have been used by
> XDP.
>
> Note that even with this change we need to keep skipping
> free_old_xmit_skbs() from NAPI handlers when XDP is enabled, because XDP
> tx queues do not aquire queue locks.
>
> - v2: Use napi_consume_skb() instead of dev_consume_skb_any()
>
> Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> NOTE: Dropped Acked-by because of the v2 change.
>
>   drivers/net/virtio_net.c | 54 +++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1d454ce..2594481 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -57,6 +57,8 @@
>   #define VIRTIO_XDP_TX		BIT(0)
>   #define VIRTIO_XDP_REDIR	BIT(1)
>   
> +#define VIRTIO_XDP_FLAG	BIT(0)
> +
>   /* RX packet size EWMA. The average packet size is used to determine the packet
>    * buffer size when refilling RX rings. As the entire RX ring may be refilled
>    * at once, the weight is chosen so that the EWMA will be insensitive to short-
> @@ -252,6 +254,21 @@ struct padded_vnet_hdr {
>   	char padding[4];
>   };
>   
> +static bool is_xdp_frame(void *ptr)
> +{
> +	return (unsigned long)ptr & VIRTIO_XDP_FLAG;
> +}
> +
> +static void *xdp_to_ptr(struct xdp_frame *ptr)
> +{
> +	return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
> +}
> +
> +static struct xdp_frame *ptr_to_xdp(void *ptr)
> +{
> +	return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
> +}
> +
>   /* Converting between virtqueue no. and kernel tx/rx queue no.
>    * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
>    */
> @@ -462,7 +479,8 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
>   
>   	sg_init_one(sq->sg, xdpf->data, xdpf->len);
>   
> -	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
> +	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf),
> +				   GFP_ATOMIC);
>   	if (unlikely(err))
>   		return -ENOSPC; /* Caller handle free/refcnt */
>   
> @@ -482,13 +500,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>   {
>   	struct virtnet_info *vi = netdev_priv(dev);
>   	struct receive_queue *rq = vi->rq;
> -	struct xdp_frame *xdpf_sent;
>   	struct bpf_prog *xdp_prog;
>   	struct send_queue *sq;
>   	unsigned int len;
>   	int drops = 0;
>   	int kicks = 0;
>   	int ret, err;
> +	void *ptr;
>   	int i;
>   
>   	/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
> @@ -507,8 +525,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>   	}
>   
>   	/* Free up any pending old buffers before queueing new ones. */
> -	while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
> -		xdp_return_frame(xdpf_sent);
> +	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		if (likely(is_xdp_frame(ptr)))
> +			xdp_return_frame(ptr_to_xdp(ptr));
> +		else
> +			napi_consume_skb(ptr, false);
> +	}
>   
>   	for (i = 0; i < n; i++) {
>   		struct xdp_frame *xdpf = frames[i];
> @@ -1329,18 +1351,26 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>   
>   static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
>   {
> -	struct sk_buff *skb;
>   	unsigned int len;
>   	unsigned int packets = 0;
>   	unsigned int bytes = 0;
> +	void *ptr;
>   
> -	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> -		pr_debug("Sent skb %p\n", skb);
> +	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		if (likely(!is_xdp_frame(ptr))) {
> +			struct sk_buff *skb = ptr;
>   
> -		bytes += skb->len;
> -		packets++;
> +			pr_debug("Sent skb %p\n", skb);
>   
> -		napi_consume_skb(skb, in_napi);
> +			bytes += skb->len;
> +			napi_consume_skb(skb, in_napi);
> +		} else {
> +			struct xdp_frame *frame = ptr_to_xdp(ptr);
> +
> +			bytes += frame->len;
> +			xdp_return_frame(frame);
> +		}
> +		packets++;
>   	}
>   
>   	/* Avoid overhead when no packets have been processed
> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
>   	for (i = 0; i < vi->max_queue_pairs; i++) {
>   		struct virtqueue *vq = vi->sq[i].vq;
>   		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> -			if (!is_xdp_raw_buffer_queue(vi, i))
> +			if (!is_xdp_frame(buf))


I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you 
can sent a patch on top to remove it.


>   				dev_kfree_skb(buf);
>   			else
> -				xdp_return_frame(buf);
> +				xdp_return_frame(ptr_to_xdp(buf));
>   		}
>   	}
>   


Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Jason Wang]

On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> We do not reset or free up unused buffers when enabling/disabling XDP,
> so it can happen that xdp_frames are freed after disabling XDP or
> sk_buffs are freed after enabling XDP on xdp tx queues.
> Thus we need to handle both forms (xdp_frames and sk_buffs) regardless
> of XDP setting.
> One way to trigger this problem is to disable XDP when napi_tx is
> enabled. In that case, virtnet_xdp_set() calls virtnet_napi_enable()
> which kicks NAPI. The NAPI handler will call virtnet_poll_cleantx()
> which invokes free_old_xmit_skbs() for queues which have been used by
> XDP.
>
> Note that even with this change we need to keep skipping
> free_old_xmit_skbs() from NAPI handlers when XDP is enabled, because XDP
> tx queues do not aquire queue locks.
>
> - v2: Use napi_consume_skb() instead of dev_consume_skb_any()
>
> Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> NOTE: Dropped Acked-by because of the v2 change.
>
>   drivers/net/virtio_net.c | 54 +++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1d454ce..2594481 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -57,6 +57,8 @@
>   #define VIRTIO_XDP_TX		BIT(0)
>   #define VIRTIO_XDP_REDIR	BIT(1)
>   
> +#define VIRTIO_XDP_FLAG	BIT(0)
> +
>   /* RX packet size EWMA. The average packet size is used to determine the packet
>    * buffer size when refilling RX rings. As the entire RX ring may be refilled
>    * at once, the weight is chosen so that the EWMA will be insensitive to short-
> @@ -252,6 +254,21 @@ struct padded_vnet_hdr {
>   	char padding[4];
>   };
>   
> +static bool is_xdp_frame(void *ptr)
> +{
> +	return (unsigned long)ptr & VIRTIO_XDP_FLAG;
> +}
> +
> +static void *xdp_to_ptr(struct xdp_frame *ptr)
> +{
> +	return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
> +}
> +
> +static struct xdp_frame *ptr_to_xdp(void *ptr)
> +{
> +	return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
> +}
> +
>   /* Converting between virtqueue no. and kernel tx/rx queue no.
>    * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
>    */
> @@ -462,7 +479,8 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
>   
>   	sg_init_one(sq->sg, xdpf->data, xdpf->len);
>   
> -	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
> +	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf),
> +				   GFP_ATOMIC);
>   	if (unlikely(err))
>   		return -ENOSPC; /* Caller handle free/refcnt */
>   
> @@ -482,13 +500,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>   {
>   	struct virtnet_info *vi = netdev_priv(dev);
>   	struct receive_queue *rq = vi->rq;
> -	struct xdp_frame *xdpf_sent;
>   	struct bpf_prog *xdp_prog;
>   	struct send_queue *sq;
>   	unsigned int len;
>   	int drops = 0;
>   	int kicks = 0;
>   	int ret, err;
> +	void *ptr;
>   	int i;
>   
>   	/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
> @@ -507,8 +525,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>   	}
>   
>   	/* Free up any pending old buffers before queueing new ones. */
> -	while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
> -		xdp_return_frame(xdpf_sent);
> +	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		if (likely(is_xdp_frame(ptr)))
> +			xdp_return_frame(ptr_to_xdp(ptr));
> +		else
> +			napi_consume_skb(ptr, false);
> +	}
>   
>   	for (i = 0; i < n; i++) {
>   		struct xdp_frame *xdpf = frames[i];
> @@ -1329,18 +1351,26 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>   
>   static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
>   {
> -	struct sk_buff *skb;
>   	unsigned int len;
>   	unsigned int packets = 0;
>   	unsigned int bytes = 0;
> +	void *ptr;
>   
> -	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> -		pr_debug("Sent skb %p\n", skb);
> +	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		if (likely(!is_xdp_frame(ptr))) {
> +			struct sk_buff *skb = ptr;
>   
> -		bytes += skb->len;
> -		packets++;
> +			pr_debug("Sent skb %p\n", skb);
>   
> -		napi_consume_skb(skb, in_napi);
> +			bytes += skb->len;
> +			napi_consume_skb(skb, in_napi);
> +		} else {
> +			struct xdp_frame *frame = ptr_to_xdp(ptr);
> +
> +			bytes += frame->len;
> +			xdp_return_frame(frame);
> +		}
> +		packets++;
>   	}
>   
>   	/* Avoid overhead when no packets have been processed
> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
>   	for (i = 0; i < vi->max_queue_pairs; i++) {
>   		struct virtqueue *vq = vi->sq[i].vq;
>   		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> -			if (!is_xdp_raw_buffer_queue(vi, i))
> +			if (!is_xdp_frame(buf))


I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you 
can sent a patch on top to remove it.


>   				dev_kfree_skb(buf);
>   			else
> -				xdp_return_frame(buf);
> +				xdp_return_frame(ptr_to_xdp(buf));
>   		}
>   	}
>   


Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Toshiaki Makita]

On 2019/01/29 11:23, Jason Wang wrote:
> On 2019/1/29 上午8:45, Toshiaki Makita wrote:
...
>> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct
>> virtnet_info *vi)
>>       for (i = 0; i < vi->max_queue_pairs; i++) {
>>           struct virtqueue *vq = vi->sq[i].vq;
>>           while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
>> -            if (!is_xdp_raw_buffer_queue(vi, i))
>> +            if (!is_xdp_frame(buf))
> 
> 
> I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you
> can sent a patch on top to remove it.

Actually patch2 added new users of it ;)

> 
> 
>>                   dev_kfree_skb(buf);
>>               else
>> -                xdp_return_frame(buf);
>> +                xdp_return_frame(ptr_to_xdp(buf));
>>           }
>>       }
>>   
> 
> 
> Acked-by: Jason Wang <jasowang@redhat.com>
> 

Thanks!

-- 
Toshiaki Makita

Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Toshiaki Makita]

On 2019/01/29 11:23, Jason Wang wrote:
> On 2019/1/29 上午8:45, Toshiaki Makita wrote:
...
>> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct
>> virtnet_info *vi)
>>       for (i = 0; i < vi->max_queue_pairs; i++) {
>>           struct virtqueue *vq = vi->sq[i].vq;
>>           while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
>> -            if (!is_xdp_raw_buffer_queue(vi, i))
>> +            if (!is_xdp_frame(buf))
> 
> 
> I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you
> can sent a patch on top to remove it.

Actually patch2 added new users of it ;)

> 
> 
>>                   dev_kfree_skb(buf);
>>               else
>> -                xdp_return_frame(buf);
>> +                xdp_return_frame(ptr_to_xdp(buf));
>>           }
>>       }
>>   
> 
> 
> Acked-by: Jason Wang <jasowang@redhat.com>
> 

Thanks!

-- 
Toshiaki Makita

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Jason Wang]

On 2019/1/29 上午10:35, Toshiaki Makita wrote:
> On 2019/01/29 11:23, Jason Wang wrote:
>> On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> ...
>>> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct
>>> virtnet_info *vi)
>>>        for (i = 0; i < vi->max_queue_pairs; i++) {
>>>            struct virtqueue *vq = vi->sq[i].vq;
>>>            while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
>>> -            if (!is_xdp_raw_buffer_queue(vi, i))
>>> +            if (!is_xdp_frame(buf))
>>
>> I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you
>> can sent a patch on top to remove it.
> Actually patch2 added new users of it ;)


Right, I missed that.

Thanks


>
>>
>>>                    dev_kfree_skb(buf);
>>>                else
>>> -                xdp_return_frame(buf);
>>> +                xdp_return_frame(ptr_to_xdp(buf));
>>>            }
>>>        }
>>>    
>>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>>
> Thanks!
>

Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Jason Wang]

On 2019/1/29 上午10:35, Toshiaki Makita wrote:
> On 2019/01/29 11:23, Jason Wang wrote:
>> On 2019/1/29 上午8:45, Toshiaki Makita wrote:
> ...
>>> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct
>>> virtnet_info *vi)
>>>        for (i = 0; i < vi->max_queue_pairs; i++) {
>>>            struct virtqueue *vq = vi->sq[i].vq;
>>>            while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
>>> -            if (!is_xdp_raw_buffer_queue(vi, i))
>>> +            if (!is_xdp_frame(buf))
>>
>> I believe this is the last user of is_xdp_raw_buffer_queue(), maybe you
>> can sent a patch on top to remove it.
> Actually patch2 added new users of it ;)


Right, I missed that.

Thanks


>
>>
>>>                    dev_kfree_skb(buf);
>>>                else
>>> -                xdp_return_frame(buf);
>>> +                xdp_return_frame(ptr_to_xdp(buf));
>>>            }
>>>        }
>>>    
>>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>>
> Thanks!
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 net 5/7] virtio_net: Don't process redirected XDP frames when XDP is disabled

[Michael S. Tsirkin]

On Tue, Jan 29, 2019 at 09:45:57AM +0900, Toshiaki Makita wrote:
> Commit 8dcc5b0ab0ec ("virtio_net: fix ndo_xdp_xmit crash towards dev not
> ready for XDP") tried to avoid access to unexpected sq while XDP is
> disabled, but was not complete.
> 
> There was a small window which causes out of bounds sq access in
> virtnet_xdp_xmit() while disabling XDP.
> 
> An example case of
>  - curr_queue_pairs = 6 (2 for SKB and 4 for XDP)
>  - online_cpu_num = xdp_queue_paris = 4
> when XDP is enabled:
> 
> CPU 0                         CPU 1
> (Disabling XDP)               (Processing redirected XDP frames)
> 
>                               virtnet_xdp_xmit()
> virtnet_xdp_set()
>  _virtnet_set_queues()
>   set curr_queue_pairs (2)
>                                check if rq->xdp_prog is not NULL
>                                virtnet_xdp_sq(vi)
>                                 qp = curr_queue_pairs -
>                                      xdp_queue_pairs +
>                                      smp_processor_id()
>                                    = 2 - 4 + 1 = -1
>                                 sq = &vi->sq[qp] // out of bounds access
>   set xdp_queue_pairs (0)
>   rq->xdp_prog = NULL
> 
> Basically we should not change curr_queue_pairs and xdp_queue_pairs
> while someone can read the values. Thus, when disabling XDP, assign NULL
> to rq->xdp_prog first, and wait for RCU grace period, then change
> xxx_queue_pairs.
> Note that we need to keep the current order when enabling XDP though.
> 
> - v2: Make rcu_assign_pointer/synchronize_net conditional instead of
>       _virtnet_set_queues.
> 
> Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 669b65c..cea52e4 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2410,6 +2410,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>  		return -ENOMEM;
>  	}
>  
> +	old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
> +	if (!prog && !old_prog)
> +		return 0;
> +
>  	if (prog) {
>  		prog = bpf_prog_add(prog, vi->max_queue_pairs - 1);
>  		if (IS_ERR(prog))
> @@ -2424,21 +2428,30 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>  		}
>  	}
>  
> +	if (!prog) {
> +		for (i = 0; i < vi->max_queue_pairs; i++) {
> +			rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
> +			if (i == 0)
> +				virtnet_restore_guest_offloads(vi);
> +		}
> +		synchronize_net();
> +	}
> +
>  	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
>  	if (err)
>  		goto err;
>  	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
>  	vi->xdp_queue_pairs = xdp_qp;
>  
> -	for (i = 0; i < vi->max_queue_pairs; i++) {
> -		old_prog = rtnl_dereference(vi->rq[i].xdp_prog);
> -		rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
> -		if (i == 0) {
> -			if (!old_prog)
> +	if (prog) {
> +		for (i = 0; i < vi->max_queue_pairs; i++) {
> +			rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
> +			if (i == 0 && !old_prog)
>  				virtnet_clear_guest_offloads(vi);
> -			if (!prog)
> -				virtnet_restore_guest_offloads(vi);
>  		}
> +	}
> +
> +	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		if (old_prog)
>  			bpf_prog_put(old_prog);
>  		if (netif_running(dev)) {
> @@ -2451,6 +2464,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>  	return 0;
>  
>  err:
> +	if (!prog) {
> +		virtnet_clear_guest_offloads(vi);
> +		for (i = 0; i < vi->max_queue_pairs; i++)
> +			rcu_assign_pointer(vi->rq[i].xdp_prog, old_prog);
> +	}
> +
>  	if (netif_running(dev)) {
>  		for (i = 0; i < vi->max_queue_pairs; i++) {
>  			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
> -- 
> 1.8.3.1
> 

Re: [PATCH v2 net 5/7] virtio_net: Don't process redirected XDP frames when XDP is disabled

[Michael S. Tsirkin]

On Tue, Jan 29, 2019 at 09:45:57AM +0900, Toshiaki Makita wrote:
> Commit 8dcc5b0ab0ec ("virtio_net: fix ndo_xdp_xmit crash towards dev not
> ready for XDP") tried to avoid access to unexpected sq while XDP is
> disabled, but was not complete.
> 
> There was a small window which causes out of bounds sq access in
> virtnet_xdp_xmit() while disabling XDP.
> 
> An example case of
>  - curr_queue_pairs = 6 (2 for SKB and 4 for XDP)
>  - online_cpu_num = xdp_queue_paris = 4
> when XDP is enabled:
> 
> CPU 0                         CPU 1
> (Disabling XDP)               (Processing redirected XDP frames)
> 
>                               virtnet_xdp_xmit()
> virtnet_xdp_set()
>  _virtnet_set_queues()
>   set curr_queue_pairs (2)
>                                check if rq->xdp_prog is not NULL
>                                virtnet_xdp_sq(vi)
>                                 qp = curr_queue_pairs -
>                                      xdp_queue_pairs +
>                                      smp_processor_id()
>                                    = 2 - 4 + 1 = -1
>                                 sq = &vi->sq[qp] // out of bounds access
>   set xdp_queue_pairs (0)
>   rq->xdp_prog = NULL
> 
> Basically we should not change curr_queue_pairs and xdp_queue_pairs
> while someone can read the values. Thus, when disabling XDP, assign NULL
> to rq->xdp_prog first, and wait for RCU grace period, then change
> xxx_queue_pairs.
> Note that we need to keep the current order when enabling XDP though.
> 
> - v2: Make rcu_assign_pointer/synchronize_net conditional instead of
>       _virtnet_set_queues.
> 
> Fixes: 186b3c998c50 ("virtio-net: support XDP_REDIRECT")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 669b65c..cea52e4 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2410,6 +2410,10 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>  		return -ENOMEM;
>  	}
>  
> +	old_prog = rtnl_dereference(vi->rq[0].xdp_prog);
> +	if (!prog && !old_prog)
> +		return 0;
> +
>  	if (prog) {
>  		prog = bpf_prog_add(prog, vi->max_queue_pairs - 1);
>  		if (IS_ERR(prog))
> @@ -2424,21 +2428,30 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>  		}
>  	}
>  
> +	if (!prog) {
> +		for (i = 0; i < vi->max_queue_pairs; i++) {
> +			rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
> +			if (i == 0)
> +				virtnet_restore_guest_offloads(vi);
> +		}
> +		synchronize_net();
> +	}
> +
>  	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
>  	if (err)
>  		goto err;
>  	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
>  	vi->xdp_queue_pairs = xdp_qp;
>  
> -	for (i = 0; i < vi->max_queue_pairs; i++) {
> -		old_prog = rtnl_dereference(vi->rq[i].xdp_prog);
> -		rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
> -		if (i == 0) {
> -			if (!old_prog)
> +	if (prog) {
> +		for (i = 0; i < vi->max_queue_pairs; i++) {
> +			rcu_assign_pointer(vi->rq[i].xdp_prog, prog);
> +			if (i == 0 && !old_prog)
>  				virtnet_clear_guest_offloads(vi);
> -			if (!prog)
> -				virtnet_restore_guest_offloads(vi);
>  		}
> +	}
> +
> +	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		if (old_prog)
>  			bpf_prog_put(old_prog);
>  		if (netif_running(dev)) {
> @@ -2451,6 +2464,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>  	return 0;
>  
>  err:
> +	if (!prog) {
> +		virtnet_clear_guest_offloads(vi);
> +		for (i = 0; i < vi->max_queue_pairs; i++)
> +			rcu_assign_pointer(vi->rq[i].xdp_prog, old_prog);
> +	}
> +
>  	if (netif_running(dev)) {
>  		for (i = 0; i < vi->max_queue_pairs; i++) {
>  			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
> -- 
> 1.8.3.1
> 

Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Michael S. Tsirkin]

On Tue, Jan 29, 2019 at 09:45:59AM +0900, Toshiaki Makita wrote:
> We do not reset or free up unused buffers when enabling/disabling XDP,
> so it can happen that xdp_frames are freed after disabling XDP or
> sk_buffs are freed after enabling XDP on xdp tx queues.
> Thus we need to handle both forms (xdp_frames and sk_buffs) regardless
> of XDP setting.
> One way to trigger this problem is to disable XDP when napi_tx is
> enabled. In that case, virtnet_xdp_set() calls virtnet_napi_enable()
> which kicks NAPI. The NAPI handler will call virtnet_poll_cleantx()
> which invokes free_old_xmit_skbs() for queues which have been used by
> XDP.
> 
> Note that even with this change we need to keep skipping
> free_old_xmit_skbs() from NAPI handlers when XDP is enabled, because XDP
> tx queues do not aquire queue locks.
> 
> - v2: Use napi_consume_skb() instead of dev_consume_skb_any()
> 
> Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> NOTE: Dropped Acked-by because of the v2 change.
> 
>  drivers/net/virtio_net.c | 54 +++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1d454ce..2594481 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -57,6 +57,8 @@
>  #define VIRTIO_XDP_TX		BIT(0)
>  #define VIRTIO_XDP_REDIR	BIT(1)
>  
> +#define VIRTIO_XDP_FLAG	BIT(0)
> +
>  /* RX packet size EWMA. The average packet size is used to determine the packet
>   * buffer size when refilling RX rings. As the entire RX ring may be refilled
>   * at once, the weight is chosen so that the EWMA will be insensitive to short-
> @@ -252,6 +254,21 @@ struct padded_vnet_hdr {
>  	char padding[4];
>  };
>  
> +static bool is_xdp_frame(void *ptr)
> +{
> +	return (unsigned long)ptr & VIRTIO_XDP_FLAG;
> +}
> +
> +static void *xdp_to_ptr(struct xdp_frame *ptr)
> +{
> +	return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
> +}
> +
> +static struct xdp_frame *ptr_to_xdp(void *ptr)
> +{
> +	return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
> +}
> +
>  /* Converting between virtqueue no. and kernel tx/rx queue no.
>   * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
>   */
> @@ -462,7 +479,8 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
>  
>  	sg_init_one(sq->sg, xdpf->data, xdpf->len);
>  
> -	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
> +	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf),
> +				   GFP_ATOMIC);
>  	if (unlikely(err))
>  		return -ENOSPC; /* Caller handle free/refcnt */
>  
> @@ -482,13 +500,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
>  	struct receive_queue *rq = vi->rq;
> -	struct xdp_frame *xdpf_sent;
>  	struct bpf_prog *xdp_prog;
>  	struct send_queue *sq;
>  	unsigned int len;
>  	int drops = 0;
>  	int kicks = 0;
>  	int ret, err;
> +	void *ptr;
>  	int i;
>  
>  	/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
> @@ -507,8 +525,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>  	}
>  
>  	/* Free up any pending old buffers before queueing new ones. */
> -	while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
> -		xdp_return_frame(xdpf_sent);
> +	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		if (likely(is_xdp_frame(ptr)))
> +			xdp_return_frame(ptr_to_xdp(ptr));
> +		else
> +			napi_consume_skb(ptr, false);
> +	}
>  
>  	for (i = 0; i < n; i++) {
>  		struct xdp_frame *xdpf = frames[i];
> @@ -1329,18 +1351,26 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  
>  static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
>  {
> -	struct sk_buff *skb;
>  	unsigned int len;
>  	unsigned int packets = 0;
>  	unsigned int bytes = 0;
> +	void *ptr;
>  
> -	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> -		pr_debug("Sent skb %p\n", skb);
> +	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		if (likely(!is_xdp_frame(ptr))) {
> +			struct sk_buff *skb = ptr;
>  
> -		bytes += skb->len;
> -		packets++;
> +			pr_debug("Sent skb %p\n", skb);
>  
> -		napi_consume_skb(skb, in_napi);
> +			bytes += skb->len;
> +			napi_consume_skb(skb, in_napi);
> +		} else {
> +			struct xdp_frame *frame = ptr_to_xdp(ptr);
> +
> +			bytes += frame->len;
> +			xdp_return_frame(frame);
> +		}
> +		packets++;
>  	}
>  
>  	/* Avoid overhead when no packets have been processed
> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
>  	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		struct virtqueue *vq = vi->sq[i].vq;
>  		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> -			if (!is_xdp_raw_buffer_queue(vi, i))
> +			if (!is_xdp_frame(buf))
>  				dev_kfree_skb(buf);
>  			else
> -				xdp_return_frame(buf);
> +				xdp_return_frame(ptr_to_xdp(buf));
>  		}
>  	}
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH v2 net 7/7] virtio_net: Differentiate sk_buff and xdp_frame on freeing

[Michael S. Tsirkin]

On Tue, Jan 29, 2019 at 09:45:59AM +0900, Toshiaki Makita wrote:
> We do not reset or free up unused buffers when enabling/disabling XDP,
> so it can happen that xdp_frames are freed after disabling XDP or
> sk_buffs are freed after enabling XDP on xdp tx queues.
> Thus we need to handle both forms (xdp_frames and sk_buffs) regardless
> of XDP setting.
> One way to trigger this problem is to disable XDP when napi_tx is
> enabled. In that case, virtnet_xdp_set() calls virtnet_napi_enable()
> which kicks NAPI. The NAPI handler will call virtnet_poll_cleantx()
> which invokes free_old_xmit_skbs() for queues which have been used by
> XDP.
> 
> Note that even with this change we need to keep skipping
> free_old_xmit_skbs() from NAPI handlers when XDP is enabled, because XDP
> tx queues do not aquire queue locks.
> 
> - v2: Use napi_consume_skb() instead of dev_consume_skb_any()
> 
> Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> NOTE: Dropped Acked-by because of the v2 change.
> 
>  drivers/net/virtio_net.c | 54 +++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1d454ce..2594481 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -57,6 +57,8 @@
>  #define VIRTIO_XDP_TX		BIT(0)
>  #define VIRTIO_XDP_REDIR	BIT(1)
>  
> +#define VIRTIO_XDP_FLAG	BIT(0)
> +
>  /* RX packet size EWMA. The average packet size is used to determine the packet
>   * buffer size when refilling RX rings. As the entire RX ring may be refilled
>   * at once, the weight is chosen so that the EWMA will be insensitive to short-
> @@ -252,6 +254,21 @@ struct padded_vnet_hdr {
>  	char padding[4];
>  };
>  
> +static bool is_xdp_frame(void *ptr)
> +{
> +	return (unsigned long)ptr & VIRTIO_XDP_FLAG;
> +}
> +
> +static void *xdp_to_ptr(struct xdp_frame *ptr)
> +{
> +	return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
> +}
> +
> +static struct xdp_frame *ptr_to_xdp(void *ptr)
> +{
> +	return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
> +}
> +
>  /* Converting between virtqueue no. and kernel tx/rx queue no.
>   * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
>   */
> @@ -462,7 +479,8 @@ static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
>  
>  	sg_init_one(sq->sg, xdpf->data, xdpf->len);
>  
> -	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdpf, GFP_ATOMIC);
> +	err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf),
> +				   GFP_ATOMIC);
>  	if (unlikely(err))
>  		return -ENOSPC; /* Caller handle free/refcnt */
>  
> @@ -482,13 +500,13 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
>  	struct receive_queue *rq = vi->rq;
> -	struct xdp_frame *xdpf_sent;
>  	struct bpf_prog *xdp_prog;
>  	struct send_queue *sq;
>  	unsigned int len;
>  	int drops = 0;
>  	int kicks = 0;
>  	int ret, err;
> +	void *ptr;
>  	int i;
>  
>  	/* Only allow ndo_xdp_xmit if XDP is loaded on dev, as this
> @@ -507,8 +525,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>  	}
>  
>  	/* Free up any pending old buffers before queueing new ones. */
> -	while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
> -		xdp_return_frame(xdpf_sent);
> +	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		if (likely(is_xdp_frame(ptr)))
> +			xdp_return_frame(ptr_to_xdp(ptr));
> +		else
> +			napi_consume_skb(ptr, false);
> +	}
>  
>  	for (i = 0; i < n; i++) {
>  		struct xdp_frame *xdpf = frames[i];
> @@ -1329,18 +1351,26 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  
>  static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
>  {
> -	struct sk_buff *skb;
>  	unsigned int len;
>  	unsigned int packets = 0;
>  	unsigned int bytes = 0;
> +	void *ptr;
>  
> -	while ((skb = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> -		pr_debug("Sent skb %p\n", skb);
> +	while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> +		if (likely(!is_xdp_frame(ptr))) {
> +			struct sk_buff *skb = ptr;
>  
> -		bytes += skb->len;
> -		packets++;
> +			pr_debug("Sent skb %p\n", skb);
>  
> -		napi_consume_skb(skb, in_napi);
> +			bytes += skb->len;
> +			napi_consume_skb(skb, in_napi);
> +		} else {
> +			struct xdp_frame *frame = ptr_to_xdp(ptr);
> +
> +			bytes += frame->len;
> +			xdp_return_frame(frame);
> +		}
> +		packets++;
>  	}
>  
>  	/* Avoid overhead when no packets have been processed
> @@ -2666,10 +2696,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
>  	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		struct virtqueue *vq = vi->sq[i].vq;
>  		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> -			if (!is_xdp_raw_buffer_queue(vi, i))
> +			if (!is_xdp_frame(buf))
>  				dev_kfree_skb(buf);
>  			else
> -				xdp_return_frame(buf);
> +				xdp_return_frame(ptr_to_xdp(buf));
>  		}
>  	}
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH v2 net 0/7] virtio_net: Fix problems around XDP tx and napi_tx

[David Miller]

From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Tue, 29 Jan 2019 09:45:52 +0900

> While I'm looking into how to account standard tx counters on XDP tx
> processing, I found several bugs around XDP tx and napi_tx.
> 
> Patch1: Fix oops on error path. Patch2 depends on this.
> Patch2: Fix memory corruption on freeing xdp_frames with napi_tx enabled.
> Patch3: Minor fix patch5 depends on.
> Patch4: Fix memory corruption on processing xdp_frames when XDP is disabled.
>   Also patch5 depends on this.
> Patch5: Fix memory corruption on processing xdp_frames while XDP is being
>   disabled.
> Patch6: Minor fix patch7 depends on.
> Patch7: Fix memory corruption on freeing sk_buff or xdp_frames when a normal
>   queue is reused for XDP and vise versa.
> 
> v2:
> - patch5: Make rcu_assign_pointer/synchronize_net conditional instead of
>           _virtnet_set_queues.
> - patch7: Use napi_consume_skb() instead of dev_consume_skb_any()
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Series applied.

Re: [PATCH v2 net 0/7] virtio_net: Fix problems around XDP tx and napi_tx

[David Miller]

From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Tue, 29 Jan 2019 09:45:52 +0900

> While I'm looking into how to account standard tx counters on XDP tx
> processing, I found several bugs around XDP tx and napi_tx.
> 
> Patch1: Fix oops on error path. Patch2 depends on this.
> Patch2: Fix memory corruption on freeing xdp_frames with napi_tx enabled.
> Patch3: Minor fix patch5 depends on.
> Patch4: Fix memory corruption on processing xdp_frames when XDP is disabled.
>   Also patch5 depends on this.
> Patch5: Fix memory corruption on processing xdp_frames while XDP is being
>   disabled.
> Patch6: Minor fix patch7 depends on.
> Patch7: Fix memory corruption on freeing sk_buff or xdp_frames when a normal
>   queue is reused for XDP and vise versa.
> 
> v2:
> - patch5: Make rcu_assign_pointer/synchronize_net conditional instead of
>           _virtnet_set_queues.
> - patch7: Use napi_consume_skb() instead of dev_consume_skb_any()
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>

Series applied.