From: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
[ Upstream commit 37d010399f7552add2b68e2b347901c83562dab8 ]
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@xxxxxx>
Tested-by: Freysteinn Alfredsson <Freysteinn.Alfredsson@xxxxxx>
Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20230315125539.103319-1-toke@xxxxxxxxxx
Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
.../net/ethernet/aquantia/atlantic/aq_ring.c | 28 ++++++++++++++-----
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 25129e723b575..2dc8d215a5918 100644
--- 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);
--
2.39.2
