[PATCH bpf 1/1] bpf: devmap: reject cloning fragmented xdp frames

[PATCH bpf 1/1] bpf: devmap: reject cloning fragmented xdp frames

[Ren Wei]

From: Zhao Zhang <zzhan461@ucr.edu>

Devmap broadcast redirects clone xdp_frame instances for all but the
last destination. That clone path only copies the linear frame data,
while fragmented XDP frames store skb_shared_info in tailroom outside
the linear area.

As a result, broadcasting a fragmented xdp_frame leaves the clone with
XDP_FLAGS_HAS_FRAGS set but without valid frag metadata. The later free
path then treats uninitialized tail data as skb_shared_info, leading to
an out-of-bounds access during frame return.

Reject fragmented frames in dev_map_enqueue_clone(). Devmap already uses
-EOPNOTSUPP for unsupported XDP frag forwarding cases, and the current
xdp_frame clone model cannot safely represent a cloned fragmented
frame.

Fixes: e624d4ed4aa8 ("xdp: Extend xdp_redirect_map with broadcast support")
Cc: stable@kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Assisted-by: Codex:GPT-5.4
Signed-off-by: Zhao Zhang <zzhan461@ucr.edu>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
 kernel/bpf/devmap.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index cc0a43ebab6b..2bfebded5329 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -581,6 +581,10 @@ static int dev_map_enqueue_clone(struct bpf_dtab_netdev *obj,
 {
 	struct xdp_frame *nxdpf;
 
+	/* Frags live outside the linear frame and cannot be cloned safely. */
+	if (unlikely(xdp_frame_has_frags(xdpf)))
+		return -EOPNOTSUPP;
+
 	nxdpf = xdpf_clone(xdpf);
 	if (!nxdpf)
 		return -ENOMEM;
-- 
2.47.3

Re: [PATCH bpf 1/1] bpf: devmap: reject cloning fragmented xdp frames

[Toke Høiland-Jørgensen]

Ren Wei <n05ec@lzu.edu.cn> writes:

> From: Zhao Zhang <zzhan461@ucr.edu>
>
> Devmap broadcast redirects clone xdp_frame instances for all but the
> last destination. That clone path only copies the linear frame data,
> while fragmented XDP frames store skb_shared_info in tailroom outside
> the linear area.
>
> As a result, broadcasting a fragmented xdp_frame leaves the clone with
> XDP_FLAGS_HAS_FRAGS set but without valid frag metadata. The later free
> path then treats uninitialized tail data as skb_shared_info, leading to
> an out-of-bounds access during frame return.
>
> Reject fragmented frames in dev_map_enqueue_clone(). Devmap already uses
> -EOPNOTSUPP for unsupported XDP frag forwarding cases, and the current
> xdp_frame clone model cannot safely represent a cloned fragmented
> frame.
>
> Fixes: e624d4ed4aa8 ("xdp: Extend xdp_redirect_map with broadcast support")
> Cc: stable@kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Assisted-by: Codex:GPT-5.4
> Signed-off-by: Zhao Zhang <zzhan461@ucr.edu>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
>  kernel/bpf/devmap.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index cc0a43ebab6b..2bfebded5329 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -581,6 +581,10 @@ static int dev_map_enqueue_clone(struct bpf_dtab_netdev *obj,
>  {
>  	struct xdp_frame *nxdpf;
>  
> +	/* Frags live outside the linear frame and cannot be cloned safely. */
> +	if (unlikely(xdp_frame_has_frags(xdpf)))
> +		return -EOPNOTSUPP;
> +

I am OK with restricting this (the support can be restored when/if
someone has a need for it), but we should add the same restriction to
generic XDP (i.e., in dev_map_redirect_clone()), so the feature set
doesn't diverge.

-Toke