net: atlantic: Fix crash when XDP is enabled but no program is loaded

The aq_xdp_run_prog() function falls back to the XDP_ABORTED action
handler (using a goto) if the operations for any of the other actions fail.
The XDP_ABORTED handler in turn calls the bpf_warn_invalid_xdp_action()
tracepoint. However, the function also jumps into the XDP_PASS helper if no
XDP program is loaded on the device, which means the XDP_ABORTED handler
can be run with a NULL program pointer. This results in a NULL pointer
deref because the tracepoint dereferences the 'prog' pointer passed to it.

This situation can happen in multiple ways:
- If a packet arrives between the removal of the program from the interface
  and the static_branch_dec() in aq_xdp_setup()
- If there are multiple devices using the same driver in the system and
  one of them has an XDP program loaded and the other does not.

Fix this by refactoring the aq_xdp_run_prog() function to remove the 'goto
pass' handling if there is no XDP program loaded. Instead, factor out the
skb building in a separate small helper function.

Fixes: 26efaef759a1 ("net: atlantic: Implement xdp data plane")
Reported-by: Freysteinn Alfredsson <Freysteinn.Alfredsson@kau.se>
Tested-by: Freysteinn Alfredsson <Freysteinn.Alfredsson@kau.se>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/r/20230315125539.103319-1-toke@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 1e8d902..7f93317 100644 (file)
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -412,6 +412,25 @@ int aq_xdp_xmit(struct net_device *dev, int num_frames,
        return num_frames - drop;
 }
 
+static struct sk_buff *aq_xdp_build_skb(struct xdp_buff *xdp,
+                                       struct net_device *dev,
+                                       struct aq_ring_buff_s *buff)
+{
+       struct xdp_frame *xdpf;
+       struct sk_buff *skb;
+
+       xdpf = xdp_convert_buff_to_frame(xdp);
+       if (unlikely(!xdpf))
+               return NULL;
+
+       skb = xdp_build_skb_from_frame(xdpf, dev);
+       if (!skb)
+               return NULL;
+
+       aq_get_rxpages_xdp(buff, xdp);
+       return skb;
+}
+
 static struct sk_buff *aq_xdp_run_prog(struct aq_nic_s *aq_nic,
                                       struct xdp_buff *xdp,
                                       struct aq_ring_s *rx_ring,
@@ -431,7 +450,7 @@ static struct sk_buff *aq_xdp_run_prog(struct aq_nic_s *aq_nic,
 
        prog = READ_ONCE(rx_ring->xdp_prog);
        if (!prog)
-               goto pass;
+               return aq_xdp_build_skb(xdp, aq_nic->ndev, buff);
 
        prefetchw(xdp->data_hard_start); /* xdp_frame write */
 
@@ -442,17 +461,12 @@ static struct sk_buff *aq_xdp_run_prog(struct aq_nic_s *aq_nic,
        act = bpf_prog_run_xdp(prog, xdp);
        switch (act) {
        case XDP_PASS:
-pass:
-               xdpf = xdp_convert_buff_to_frame(xdp);
-               if (unlikely(!xdpf))
-                       goto out_aborted;
-               skb = xdp_build_skb_from_frame(xdpf, aq_nic->ndev);
+               skb = aq_xdp_build_skb(xdp, aq_nic->ndev, buff);
                if (!skb)
                        goto out_aborted;
                u64_stats_update_begin(&rx_ring->stats.rx.syncp);
                ++rx_ring->stats.rx.xdp_pass;
                u64_stats_update_end(&rx_ring->stats.rx.syncp);
-               aq_get_rxpages_xdp(buff, xdp);
                return skb;
        case XDP_TX:
                xdpf = xdp_convert_buff_to_frame(xdp);