[PATCH bpf v3 1/2] bpf: Run generic devmap egress prog on private skb

[Sun Jian]

Generic XDP devmap multi redirect uses skb_clone() for intermediate
destinations and sends the last destination with the original skb. This
can leave multiple destinations sharing the same packet data.

This becomes visible after generic devmap egress-program support was
added: a devmap egress program may mutate packet data, and another
destination sharing the same data can observe that mutation.

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 before running the generic devmap
egress program. Use skb_copy() instead of skb_unshare() so allocation
failure does not consume the skb and the existing caller error paths keep
their ownership semantics.

Fixes: 2ea5eabaf04a ("bpf: devmap: Implement devmap prog execution for generic 
XDP")
Suggested-by: Jiayuan Chen <jiayuan.c...@linux.dev>
Suggested-by: Jakub Kicinski <k...@kernel.org>
Reviewed-by: Toke Høiland-Jørgensen <t...@redhat.com>
Signed-off-by: Sun Jian <sun.jian.k...@gmail.com>
---
 kernel/bpf/devmap.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index cc0a43ebab6b..14506834345a 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -512,8 +512,10 @@ static inline int __xdp_enqueue(struct net_device *dev, 
struct xdp_frame *xdpf,
        return 0;
 }
 
-static u32 dev_map_bpf_prog_run_skb(struct sk_buff *skb, struct 
bpf_dtab_netdev *dst)
+static int dev_map_bpf_prog_run_skb(struct sk_buff **pskb,
+                                   struct bpf_dtab_netdev *dst)
 {
+       struct sk_buff *skb = *pskb;
        struct xdp_txq_info txq = { .dev = dst->dev };
        struct xdp_buff xdp;
        u32 act;
@@ -521,6 +523,18 @@ static u32 dev_map_bpf_prog_run_skb(struct sk_buff *skb, 
struct bpf_dtab_netdev
        if (!dst->xdp_prog)
                return XDP_PASS;
 
+       if (skb_cloned(skb)) {
+               struct sk_buff *nskb;
+
+               nskb = skb_copy(skb, GFP_ATOMIC);
+               if (!nskb)
+                       return -ENOMEM;
+
+               consume_skb(skb);
+               skb = nskb;
+               *pskb = nskb;
+       }
+
        __skb_pull(skb, skb->mac_len);
        xdp.txq = &txq;
 
@@ -710,7 +724,10 @@ int dev_map_generic_redirect(struct bpf_dtab_netdev *dst, 
struct sk_buff *skb,
         * return 0 even if packet is dropped. Helper below takes care of
         * freeing skb.
         */
-       if (dev_map_bpf_prog_run_skb(skb, dst) != XDP_PASS)
+       err = dev_map_bpf_prog_run_skb(&skb, dst);
+       if (err < 0)
+               return err;
+       if (err != XDP_PASS)
                return 0;
 
        skb->dev = dst->dev;
-- 
2.43.0