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
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
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
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
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
© 2016 - 2026 Red Hat, Inc.