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 <redacted>
---
  drivers/net/virtio_net.c | 33 ++++++++++++++++++++++++++-------
  1 file changed, 26 insertions(+), 7 deletions(-)

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

Thanks.