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

[ Upstream commit 6f41e595234ed314f36163cc8ad59a1a92e96047 ]

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: 79fbae77cecd ("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>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 25129e723b5751078eaa77806318b378e71e6b19..2dc8d215a591896d175ef00847e0c94d320b54a0 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);