[PATCH net-next 0/2] net: drop the stale dst when steering to another device

[PATCH net-next 0/2] net: drop the stale dst when steering to another device

[Jamal Hadi Salim]

This patchset is created to address constant whining by both sashikos
about this being an issue when fixing a UAF on a teql issue (entering
v4 now), see the v3 here fixing the teql consumer:
https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/

Two producers steer a packet to a network device of their choosing and
then hand it to the neighbour layer for link-layer resolution:

  1/2 netfilter: the neighbour variant of nft `fwd`, on the netdev egress
      hook
  2/2 bpf: `bpf_redirect_neigh()` when the program supplies the nexthop

Both rewrite `skb->dev` to the target device and leave the skb's dst
pointing at the route the packet arrived through. When the target
device's neighbour for the nexthop is incomplete the packet is parked on
that neighbour's arp_queue, and draining the queue re-looks the neighbour
up from the stale dst. The frame is therefore built with the original
device's link-layer header and then transmitted on the target device: the
packet leaves with another device's addresses, and the sender never
learns that its nexthop was unreachable.

This is explicit: the steer-to-another-device path never worked in this
respect, so there is nothing here to regress. Both producers shipped with
`skb->dev` swapped to the target device and the route's dst still
attached, and the arp_queue drain has re-derived the neighbour from that
dst for as long as either producer has existed; the invariant "the skb's
dst describes where the skb is going", which the routing path keeps when
it steers, was never established for these two entry points. With no
commit to anchor with a `Fixes:` tag and no crash, UAF or lockup
involved, the series targets net-next and does not nominate `Cc: stable`:
a backport would change long-standing redirect behaviour for no
crash-class benefit.

More context: this came out of the review of the fix for a parked skb
being resolved against the wrong device in sch_teql, posted separately and
not part of this series. The neighbour code's queue drain is doing the
right thing there, it re-derives the neighbour from the skb, so the repair
belongs with the producers rather than with the drain path. Both
producers are fixed in the same series so neither is left behind; each
patch stands on its own and carries its own reproduction recipe.

The reproduction for both arms is a pair of veth devices, one of them
with an unresolved neighbour, plus a capture on the target's peer: the
captured frame's source and destination addresses name the device whose
neighbour was used to build it.

Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com
Link: https://lore.kernel.org/netdev/20261026160747.283702-1-horms@kernel.org/
Link: https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/

Jamal Hadi Salim (2):
  netfilter: nft_fwd_netdev: drop the stale dst when steering to a
    device
  bpf: drop the stale dst when redirecting to an explicit nexthop

 net/core/filter.c              | 4 ++++
 net/netfilter/nft_fwd_netdev.c | 1 +
 2 files changed, 5 insertions(+)

-- 
2.43.0

[PATCH net-next 2/2] bpf: drop the stale dst when redirecting to an explicit nexthop

[Jamal Hadi Salim]

This is a followup of the review of the teql arp-queue fix as reported
by Sashiko.

The issue: __bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6()
install a dst for the redirect target only when the program lets the
helper do the FIB lookup. With BPF_F_NEXTHOP the program supplies the
nexthop, the lookup is skipped, and whatever dst the packet arrived
with stays attached while bpf_out_neigh_v{4,6}() switch skb->dev to the
target device. If that device's neighbour is incomplete the packet is
parked on its arp_queue with the original route's dst; draining the
queue re-looks the neighbour up from that dst and builds the frame with
the original device's link-layer header, then transmits it on the target
device.

The Fix: drop the dst when the nexthop is supplied. That branch never
consults the route dst, so nothing is lost, and the queue drain keeps
the target device's parking neighbour.

Conditions to recreate the bug: two network namespaces, A (10.10.0.1/24)
and B (10.1.0.1/24) in one, their veth peers in the other with only A's
peer addressed (10.10.0.2/24) so B's nexthop cannot be resolved by ARP.
Resolve A's neighbour for 10.10.0.2 as permanent and attach a cls_bpf
egress program to A, with vb the index of B:

    struct bpf_redir_neigh nh = { .nh_family = AF_INET,
                                  .ipv4_nh = htonl(0x0a010002) };

    return bpf_redirect_neigh(vb, &nh, sizeof(nh), 0);

Send one packet to 10.10.0.2. It is parked on B's incomplete neighbour
for 10.1.0.2; once that neighbour becomes valid the frame leaves B
carrying A's source address and A's neighbour as the destination.

Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com
Link: https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/core/filter.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index aca8160b26a1..2af24f371648 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2318,6 +2318,8 @@ static int __bpf_redirect_neigh_v6(struct sk_buff *skb, struct net_device *dev,
 		skb_dst_set(skb, dst);
 	} else if (nh->nh_family != AF_INET6) {
 		goto out_drop;
+	} else {
+		skb_dst_drop(skb);
 	}
 
 	err = bpf_out_neigh_v6(net, skb, dev, nh);
@@ -2433,6 +2435,8 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
 
 		skb_dst_drop(skb);
 		skb_dst_set(skb, &rt->dst);
+	} else {
+		skb_dst_drop(skb);
 	}
 
 	err = bpf_out_neigh_v4(net, skb, dev, nh);
-- 
2.43.0

Re: [PATCH net-next 2/2] bpf: drop the stale dst when redirecting to an explicit nexthop

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] bpf: missing Fixes trailer for commit ba452c9e996d in commit
  message
--

commit 563f2cf949ebea821bc686f9b196aaca4663a1a8
Author: Jamal Hadi Salim <jhs@mojatatu.com>

> bpf: drop the stale dst when redirecting to an explicit nexthop
>
> This is a followup of the review of the teql arp-queue fix as reported
> by Sashiko.
>
> The issue: __bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6()
> install a dst for the redirect target only when the program lets the
> helper do the FIB lookup. With BPF_F_NEXTHOP the program supplies the
> nexthop, the lookup is skipped, and whatever dst the packet arrived
> with stays attached while bpf_out_neigh_v{4,6}() switch skb->dev to the
> target device. If that device's neighbour is incomplete the packet is
> parked on its arp_queue with the original route's dst; draining the
> queue re-looks the neighbour up from that dst and builds the frame with
> the original device's link-layer header, then transmits it on the target
> device.
>
> The Fix: drop the dst when the nexthop is supplied. That branch never
> consults the route dst, so nothing is lost, and the queue drain keeps
> the target device's parking neighbour.
>
> Conditions to recreate the bug: two network namespaces, A (10.10.0.1/24)
> and B (10.1.0.1/24) in one, their veth peers in the other with only A's
> peer addressed (10.10.0.2/24) so B's nexthop cannot be resolved by ARP.
> Resolve A's neighbour for 10.10.0.2 as permanent and attach a cls_bpf
> egress program to A, with vb the index of B:
>
>     struct bpf_redir_neigh nh = { .nh_family = AF_INET,
>                                   .ipv4_nh = htonl(0x0a010002) };
>
>     return bpf_redirect_neigh(vb, &nh, sizeof(nh), 0);
>
> Send one packet to 10.10.0.2. It is parked on B's incomplete neighbour
> for 10.1.0.2; once that neighbour becomes valid the frame leaves B
> carrying A's source address and A's neighbour as the destination.
>
> Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com
> Link: https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

[Severity: Low]
This fixes a bug in the explicit nexthop handling of
__bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6().  Should the
commit message include a Fixes tag?

  Fixes: ba452c9e996d ("bpf: Fix bpf_redirect_neigh helper api to support supplying nexthop")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/CORE-9NOG.v1.r0.20261111073528@mojatatu.com?part=2