[PATCH bpf-next 1/2] bpf, test_run: Fix user-memory-access vulnerability for LIVE_FRAMES

KaFai Wan posted 2 patches 1 month ago
[PATCH bpf-next 1/2] bpf, test_run: Fix user-memory-access vulnerability for LIVE_FRAMES
Posted by KaFai Wan 1 month ago
When testing XDP programs with LIVE_FRAMES mode, if the metalen is set
to >= (XDP_PACKET_HEADROOM - sizeof(struct xdp_frame)), there won't be
enough space for the xdp_frame conversion in xdp_update_frame_from_buff().
Additionally, the xdp_frame structure may be filled with user-provided data,
which can lead to a memory access vulnerability when converting to skb.

This fix reverts to the original version and ensures data_hard_start
correctly points to the xdp_frame structure, eliminating the security risk.

Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
Fixes: 294635a8165a ("bpf, test_run: fix &xdp_frame misplacement for LIVE_FRAMES")
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
---
 net/bpf/test_run.c                            | 23 +++++++++----------
 .../bpf/prog_tests/xdp_do_redirect.c          |  6 ++---
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 655efac6f133..00234eba7c76 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -90,11 +90,9 @@ static bool bpf_test_timer_continue(struct bpf_test_timer *t, int iterations,
 struct xdp_page_head {
 	struct xdp_buff orig_ctx;
 	struct xdp_buff ctx;
-	union {
-		/* ::data_hard_start starts here */
-		DECLARE_FLEX_ARRAY(struct xdp_frame, frame);
-		DECLARE_FLEX_ARRAY(u8, data);
-	};
+	/* ::data_hard_start starts here */
+	struct xdp_frame frame;
+	DECLARE_FLEX_ARRAY(u8, data);
 };
 
 struct xdp_test_data {
@@ -131,10 +129,11 @@ static void xdp_test_run_init_page(netmem_ref netmem, void *arg)
 	frm_len = orig_ctx->data_end - orig_ctx->data_meta;
 	meta_len = orig_ctx->data - orig_ctx->data_meta;
 	headroom -= meta_len;
+	headroom += sizeof(head->frame);
 
 	new_ctx = &head->ctx;
-	frm = head->frame;
-	data = head->data;
+	frm = &head->frame;
+	data = frm;
 	memcpy(data + headroom, orig_ctx->data_meta, frm_len);
 
 	xdp_init_buff(new_ctx, TEST_XDP_FRAME_SIZE, &xdp->rxq);
@@ -215,8 +214,8 @@ static bool frame_was_changed(const struct xdp_page_head *head)
 	 * i.e. has the highest chances to be overwritten. If those two are
 	 * untouched, it's most likely safe to skip the context reset.
 	 */
-	return head->frame->data != head->orig_ctx.data ||
-	       head->frame->flags != head->orig_ctx.flags;
+	return head->frame.data != head->orig_ctx.data ||
+	       head->frame.flags != head->orig_ctx.flags;
 }
 
 static bool ctx_was_changed(struct xdp_page_head *head)
@@ -234,8 +233,8 @@ static void reset_ctx(struct xdp_page_head *head)
 	head->ctx.data = head->orig_ctx.data;
 	head->ctx.data_meta = head->orig_ctx.data_meta;
 	head->ctx.data_end = head->orig_ctx.data_end;
-	xdp_update_frame_from_buff(&head->ctx, head->frame);
-	head->frame->mem_type = head->orig_ctx.rxq->mem.type;
+	xdp_update_frame_from_buff(&head->ctx, &head->frame);
+	head->frame.mem_type = head->orig_ctx.rxq->mem.type;
 }
 
 static int xdp_recv_frames(struct xdp_frame **frames, int nframes,
@@ -301,7 +300,7 @@ static int xdp_test_run_batch(struct xdp_test_data *xdp, struct bpf_prog *prog,
 		head = phys_to_virt(page_to_phys(page));
 		reset_ctx(head);
 		ctx = &head->ctx;
-		frm = head->frame;
+		frm = &head->frame;
 		xdp->frame_cnt++;
 
 		act = bpf_prog_run_xdp(prog, ctx);
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c b/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
index dd34b0cc4b4e..f7615c265e6e 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
@@ -59,12 +59,12 @@ static int attach_tc_prog(struct bpf_tc_hook *hook, int fd)
 
 /* The maximum permissible size is: PAGE_SIZE - sizeof(struct xdp_page_head) -
  * SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - XDP_PACKET_HEADROOM =
- * 3408 bytes for 64-byte cacheline and 3216 for 256-byte one.
+ * 3368 bytes for 64-byte cacheline and 3216 for 256-byte one.
  */
 #if defined(__s390x__)
-#define MAX_PKT_SIZE 3216
+#define MAX_PKT_SIZE 3176
 #else
-#define MAX_PKT_SIZE 3408
+#define MAX_PKT_SIZE 3368
 #endif
 
 #define PAGE_SIZE_4K  4096
-- 
2.43.0
Re: [PATCH bpf-next 1/2] bpf, test_run: Fix user-memory-access vulnerability for LIVE_FRAMES
Posted by Toke Høiland-Jørgensen 1 month ago
KaFai Wan <kafai.wan@linux.dev> writes:

> This fix reverts to the original version and ensures data_hard_start
> correctly points to the xdp_frame structure, eliminating the security
> risk.

This is wrong. We should just be checking the meta_len on input to
account for the size of xdp_frame. I'll send a patch.

-Toke
Re: [PATCH bpf-next 1/2] bpf, test_run: Fix user-memory-access vulnerability for LIVE_FRAMES
Posted by KaFai Wan 1 month ago
On Mon, 2026-01-05 at 11:46 +0100, Toke Høiland-Jørgensen wrote:
> KaFai Wan <kafai.wan@linux.dev> writes:
> 
> > This fix reverts to the original version and ensures data_hard_start
> > correctly points to the xdp_frame structure, eliminating the security
> > risk.
> 
> This is wrong. We should just be checking the meta_len on input to
> account for the size of xdp_frame. I'll send a patch.

Current version the actual limit of the max input meta_len for live frames is 
XDP_PACKET_HEADROOM - sizeof(struct xdp_frame), not XDP_PACKET_HEADROOM.

The original version not set xdp_buff->data_hard_start with xdp_frame, 
I set it with the correct position by adding the headroom, so there is no need 
for user to reduce the max input meta_len.

This patch is failed with the xdp_do_redirect test, I'll fix and send v2 if 
you're ok with that.
 
> 
> -Toke
> 

-- 
Thanks,
KaFai
Re: [PATCH bpf-next 1/2] bpf, test_run: Fix user-memory-access vulnerability for LIVE_FRAMES
Posted by Toke Høiland-Jørgensen 1 month ago
KaFai Wan <kafai.wan@linux.dev> writes:

> On Mon, 2026-01-05 at 11:46 +0100, Toke Høiland-Jørgensen wrote:
>> KaFai Wan <kafai.wan@linux.dev> writes:
>> 
>> > This fix reverts to the original version and ensures data_hard_start
>> > correctly points to the xdp_frame structure, eliminating the security
>> > risk.
>> 
>> This is wrong. We should just be checking the meta_len on input to
>> account for the size of xdp_frame. I'll send a patch.
>
> Current version the actual limit of the max input meta_len for live frames is 
> XDP_PACKET_HEADROOM - sizeof(struct xdp_frame), not
> XDP_PACKET_HEADROOM.

By "current version", you mean the patch I sent[0], right?

If so, that was deliberate: the stack limits the maximum data_meta size
to XDP_PACKET_HEADROOM - sizeof(struct xdp_frame), so there's no reason
not to do the same for bpf_prog_run(). And some chance that diverging
here will end up surfacing other bugs down the line.

-Toke

[0] https://lore.kernel.org/r/20260105114747.1358750-1-toke@redhat.com
Re: [PATCH bpf-next 1/2] bpf, test_run: Fix user-memory-access vulnerability for LIVE_FRAMES
Posted by KaFai Wan 1 month ago
On Mon, 2026-01-05 at 17:43 +0100, Toke Høiland-Jørgensen wrote:
> KaFai Wan <kafai.wan@linux.dev> writes:
> 
> > On Mon, 2026-01-05 at 11:46 +0100, Toke Høiland-Jørgensen wrote:
> > > KaFai Wan <kafai.wan@linux.dev> writes:
> > > 
> > > > This fix reverts to the original version and ensures data_hard_start
> > > > correctly points to the xdp_frame structure, eliminating the security
> > > > risk.
> > > 
> > > This is wrong. We should just be checking the meta_len on input to
> > > account for the size of xdp_frame. I'll send a patch.
> > 
> > Current version the actual limit of the max input meta_len for live frames is 
> > XDP_PACKET_HEADROOM - sizeof(struct xdp_frame), not
> > XDP_PACKET_HEADROOM.
> 
> By "current version", you mean the patch I sent[0], right?
> 
> If so, that was deliberate: the stack limits the maximum data_meta size
> to XDP_PACKET_HEADROOM - sizeof(struct xdp_frame), so there's no reason
> not to do the same for bpf_prog_run(). And some chance that diverging
> here will end up surfacing other bugs down the line.
> 
Oh, I see. Thank you for your explanation.
> -Toke
> 
> [0] https://lore.kernel.org/r/20260105114747.1358750-1-toke@redhat.com
> 

-- 
Thanks,
KaFai