[PATCH] bpf: Unshare cloned skb before devmap egress XDP program

[Sun Jian]

dev_map_redirect_clone() uses skb_clone() when redirecting a generic XDP
skb to multiple devmap destinations. The cloned skb can share packet data
with other clones.

If the destination devmap entry has an egress XDP program, that program
can modify packet data. Such modifications can then be observed by other
clones sharing the same packet data.

This can be reproduced by strengthening xdp_veth_egress to configure a
different source MAC for each egress device and checking that store_mac_1/2
observe the MAC configured for their own egress devices. Without the fix,
the SKB_MODE subtest observes store_mac_1 receiving the MAC configured for
the next egress device.

Fix this by unsharing the cloned skb before running the devmap egress XDP
program. Limit the extra copy to destinations with an attached egress
program.

Tested with:
  ./test_progs -t xdp_veth_egress
  ./test_progs -t xdp_veth
  ./test_progs -t xdp

Fixes: e624d4ed4aa8 ("xdp: Extend xdp_redirect_map with broadcast support")
Signed-off-by: Sun Jian <sun.jian.k...@gmail.com>
---
 kernel/bpf/devmap.c                                 |  6 ++++++
 .../selftests/bpf/prog_tests/test_xdp_veth.c        | 13 ++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index cc0a43ebab6b..4ae65d44f9d6 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -730,6 +730,12 @@ static int dev_map_redirect_clone(struct bpf_dtab_netdev 
*dst,
        if (!nskb)
                return -ENOMEM;
 
+       if (dst->xdp_prog) {
+               nskb = skb_unshare(nskb, GFP_ATOMIC);
+               if (!nskb)
+                       return -ENOMEM;
+       }
+
        err = dev_map_generic_redirect(dst, nskb, xdp_prog);
        if (unlikely(err)) {
                consume_skb(nskb);
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xdp_veth.c 
b/tools/testing/selftests/bpf/prog_tests/test_xdp_veth.c
index 3e98a1665936..52d79d5c5629 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xdp_veth.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xdp_veth.c
@@ -456,7 +456,11 @@ static void xdp_veth_egress(u32 flags)
                        .remote_flags = flags,
                }
        };
-       const char magic_mac[6] = { 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF};
+       const unsigned char egress_macs[VETH_PAIRS_COUNT][ETH_ALEN] = {
+               { 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x01 },
+               { 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x02 },
+               { 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x03 },
+       };
        struct xdp_redirect_multi_kern *xdp_redirect_multi_kern;
        struct bpf_object *bpf_objs[VETH_EGRESS_SKEL_NB];
        struct xdp_redirect_map *xdp_redirect_map;
@@ -512,7 +516,7 @@ static void xdp_veth_egress(u32 flags)
                                                 &net_config, prog_cfg, i))
                        goto destroy_xdp_redirect_map;
 
-               err = bpf_map_update_elem(mac_map, &ifindex, magic_mac, 0);
+               err = bpf_map_update_elem(mac_map, &ifindex, egress_macs[i], 0);
                if (!ASSERT_OK(err, "bpf_map_update_elem"))
                        goto destroy_xdp_redirect_map;
 
@@ -531,13 +535,16 @@ static void xdp_veth_egress(u32 flags)
 
        for (i = 0; i < 2; i++) {
                u32 key = i;
+               __be64 expected = 0;
                u64 res;
 
                err = bpf_map_lookup_elem(res_map, &key, &res);
                if (!ASSERT_OK(err, "get MAC res"))
                        goto destroy_xdp_redirect_map;
 
-               ASSERT_STRNEQ((const char *)&res, magic_mac, ETH_ALEN, "compare 
mac");
+               /* store_mac_1/2 run on the second/third remote veths. */
+               memcpy(&expected, egress_macs[i + 1], ETH_ALEN);
+               ASSERT_EQ(res, expected, "compare mac");
        }
 
 destroy_xdp_redirect_map:
-- 
2.43.0