Patch "xdp: use flags field to disambiguate broadcast redirect" has been added to the 6.8-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    xdp: use flags field to disambiguate broadcast redirect

to the 6.8-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     xdp-use-flags-field-to-disambiguate-broadcast-redire.patch
and it can be found in the queue-6.8 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 71c8ed68f8aaa2afe622fc31ba448563e05479ed
Author: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
Date:   Thu Apr 18 09:18:39 2024 +0200

    xdp: use flags field to disambiguate broadcast redirect
    
    [ Upstream commit 5bcf0dcbf9066348058b88a510c57f70f384c92c ]
    
    When redirecting a packet using XDP, the bpf_redirect_map() helper will set
    up the redirect destination information in struct bpf_redirect_info (using
    the __bpf_xdp_redirect_map() helper function), and the xdp_do_redirect()
    function will read this information after the XDP program returns and pass
    the frame on to the right redirect destination.
    
    When using the BPF_F_BROADCAST flag to do multicast redirect to a whole
    map, __bpf_xdp_redirect_map() sets the 'map' pointer in struct
    bpf_redirect_info to point to the destination map to be broadcast. And
    xdp_do_redirect() reacts to the value of this map pointer to decide whether
    it's dealing with a broadcast or a single-value redirect. However, if the
    destination map is being destroyed before xdp_do_redirect() is called, the
    map pointer will be cleared out (by bpf_clear_redirect_map()) without
    waiting for any XDP programs to stop running. This causes xdp_do_redirect()
    to think that the redirect was to a single target, but the target pointer
    is also NULL (since broadcast redirects don't have a single target), so
    this causes a crash when a NULL pointer is passed to dev_map_enqueue().
    
    To fix this, change xdp_do_redirect() to react directly to the presence of
    the BPF_F_BROADCAST flag in the 'flags' value in struct bpf_redirect_info
    to disambiguate between a single-target and a broadcast redirect. And only
    read the 'map' pointer if the broadcast flag is set, aborting if that has
    been cleared out in the meantime. This prevents the crash, while keeping
    the atomic (cmpxchg-based) clearing of the map pointer itself, and without
    adding any more checks in the non-broadcast fast path.
    
    Fixes: e624d4ed4aa8 ("xdp: Extend xdp_redirect_map with broadcast support")
    Reported-and-tested-by: syzbot+af9492708df9797198d6@xxxxxxxxxxxxxxxxxxxxxxxxx
    Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
    Acked-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
    Reviewed-by: Hangbin Liu <liuhangbin@xxxxxxxxx>
    Acked-by: Jesper Dangaard Brouer <hawk@xxxxxxxxxx>
    Link: https://lore.kernel.org/r/20240418071840.156411-1-toke@xxxxxxxxxx
    Signed-off-by: Martin KaFai Lau <martin.lau@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/net/core/filter.c b/net/core/filter.c
index ef3e78b6a39c4..75cdaa16046bb 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4360,10 +4360,12 @@ static __always_inline int __xdp_do_redirect_frame(struct bpf_redirect_info *ri,
 	enum bpf_map_type map_type = ri->map_type;
 	void *fwd = ri->tgt_value;
 	u32 map_id = ri->map_id;
+	u32 flags = ri->flags;
 	struct bpf_map *map;
 	int err;
 
 	ri->map_id = 0; /* Valid map id idr range: [1,INT_MAX[ */
+	ri->flags = 0;
 	ri->map_type = BPF_MAP_TYPE_UNSPEC;
 
 	if (unlikely(!xdpf)) {
@@ -4375,11 +4377,20 @@ static __always_inline int __xdp_do_redirect_frame(struct bpf_redirect_info *ri,
 	case BPF_MAP_TYPE_DEVMAP:
 		fallthrough;
 	case BPF_MAP_TYPE_DEVMAP_HASH:
-		map = READ_ONCE(ri->map);
-		if (unlikely(map)) {
+		if (unlikely(flags & BPF_F_BROADCAST)) {
+			map = READ_ONCE(ri->map);
+
+			/* The map pointer is cleared when the map is being torn
+			 * down by bpf_clear_redirect_map()
+			 */
+			if (unlikely(!map)) {
+				err = -ENOENT;
+				break;
+			}
+
 			WRITE_ONCE(ri->map, NULL);
 			err = dev_map_enqueue_multi(xdpf, dev, map,
-						    ri->flags & BPF_F_EXCLUDE_INGRESS);
+						    flags & BPF_F_EXCLUDE_INGRESS);
 		} else {
 			err = dev_map_enqueue(fwd, xdpf, dev);
 		}
@@ -4442,9 +4453,9 @@ EXPORT_SYMBOL_GPL(xdp_do_redirect_frame);
 static int xdp_do_generic_redirect_map(struct net_device *dev,
 				       struct sk_buff *skb,
 				       struct xdp_buff *xdp,
-				       struct bpf_prog *xdp_prog,
-				       void *fwd,
-				       enum bpf_map_type map_type, u32 map_id)
+				       struct bpf_prog *xdp_prog, void *fwd,
+				       enum bpf_map_type map_type, u32 map_id,
+				       u32 flags)
 {
 	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 	struct bpf_map *map;
@@ -4454,11 +4465,20 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
 	case BPF_MAP_TYPE_DEVMAP:
 		fallthrough;
 	case BPF_MAP_TYPE_DEVMAP_HASH:
-		map = READ_ONCE(ri->map);
-		if (unlikely(map)) {
+		if (unlikely(flags & BPF_F_BROADCAST)) {
+			map = READ_ONCE(ri->map);
+
+			/* The map pointer is cleared when the map is being torn
+			 * down by bpf_clear_redirect_map()
+			 */
+			if (unlikely(!map)) {
+				err = -ENOENT;
+				break;
+			}
+
 			WRITE_ONCE(ri->map, NULL);
 			err = dev_map_redirect_multi(dev, skb, xdp_prog, map,
-						     ri->flags & BPF_F_EXCLUDE_INGRESS);
+						     flags & BPF_F_EXCLUDE_INGRESS);
 		} else {
 			err = dev_map_generic_redirect(fwd, skb, xdp_prog);
 		}
@@ -4495,9 +4515,11 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 	enum bpf_map_type map_type = ri->map_type;
 	void *fwd = ri->tgt_value;
 	u32 map_id = ri->map_id;
+	u32 flags = ri->flags;
 	int err;
 
 	ri->map_id = 0; /* Valid map id idr range: [1,INT_MAX[ */
+	ri->flags = 0;
 	ri->map_type = BPF_MAP_TYPE_UNSPEC;
 
 	if (map_type == BPF_MAP_TYPE_UNSPEC && map_id == INT_MAX) {
@@ -4517,7 +4539,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 		return 0;
 	}
 
-	return xdp_do_generic_redirect_map(dev, skb, xdp, xdp_prog, fwd, map_type, map_id);
+	return xdp_do_generic_redirect_map(dev, skb, xdp, xdp_prog, fwd, map_type, map_id, flags);
 err:
 	_trace_xdp_redirect_err(dev, xdp_prog, ri->tgt_index, err);
 	return err;




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux