> Generic XDP devmap multi redirect uses skb_clone() for the
> intermediate destinations and sends the last destination with the
> original skb. This can leave multiple destinations sharing the same
> packet data.
> This becomes visible when a devmap egress program mutates packet data.
> One destination can observe changes made for another destination. The
> last-destination path has the same problem: the last destination runs on
> the original skb, so its egress program can modify packet data still
> shared with earlier cloned skbs.
> Native XDP broadcast redirect does not have this issue because
> xdpf_clone() copies the frame data for each destination. Generic XDP
> should provide the same per-destination isolation before running a
> devmap egress program.
> Fix this by making cloned skbs private in dev_map_generic_redirect()
> before running the devmap egress program. Use skb_copy() instead of
> skb_unshare() so that allocation failure does not consume the skb and
> the existing caller error paths keep their ownership semantics.
> Add a selftest that covers the last-destination case where earlier
> destinations do not have a devmap egress program, while the final
> destination does.
> Fixes: e624d4ed4aa8 ("xdp: Extend xdp_redirect_map with broadcast
> support")
> Suggested-by: Jiayuan Chen <[email protected]>
> Signed-off-by: Sun Jian <[email protected]>
Is the Fixes: tag pointing at the right commit?
The tag references e624d4ed4aa8 ("xdp: Extend xdp_redirect_map with
broadcast support"), which introduced the generic-XDP multi/broadcast
redirect that clones skbs and sends the last destination on the original
skb. At that commit, though, dev_map_generic_redirect() only called
generic_xdp_tx() and ran no per-destination program, so sharing skb data
between clones was harmless.
The corruption being fixed only appears once a devmap egress program
runs on the shared skb data. That egress-program execution in the
generic path (dev_map_bpf_prog_run_skb(), gated on dst->xdp_prog) was
added later by 2ea5eabaf04a ("bpf: devmap: Implement devmap prog
execution for generic XDP"). The fix here is itself gated on
dst->xdp_prog && skb_cloned(skb), matching the commit message statement
that the problem "becomes visible when a devmap egress program mutates
packet data".
Would this be more accurate?
Fixes: 2ea5eabaf04a ("bpf: devmap: Implement devmap prog execution for
generic XDP")
e624d4ed4aa8 provides the clone-sharing infrastructure and could be kept
as an additional reference, but 2ea5eabaf04a is the commit that
introduced the writer into the shared-data path.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/27270693851
