Martin KaFai Lau <kafai@xxxxxx> writes: > On Thu, Mar 10, 2022 at 11:56:20PM +0100, Toke Høiland-Jørgensen wrote: >> The live packet mode uses some extra space at the start of each page to >> cache data structures so they don't have to be rebuilt at every repetition. >> This space wasn't correctly accounted for in the size checking of the >> arguments supplied to userspace. In addition, the definition of the frame >> size should include the size of the skb_shared_info (as there is other >> logic that subtracts the size of this). >> >> Together, these mistakes resulted in userspace being able to trip the >> XDP_WARN() in xdp_update_frame_from_buff(), which syzbot discovered in >> short order. Fix this by changing the frame size define and adding the >> extra headroom to the bpf_prog_test_run_xdp() function. Also drop the >> max_len parameter to the page_pool init, since this is related to DMA which >> is not used for the page pool instance in PROG_TEST_RUN. >> >> Reported-by: syzbot+0e91362d99386dc5de99@xxxxxxxxxxxxxxxxxxxxxxxxx >> Fixes: b530e9e1063e ("bpf: Add "live packet" mode for XDP in BPF_PROG_RUN") >> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx> >> --- >> net/bpf/test_run.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c >> index 24405a280a9b..e7b9c2636d10 100644 >> --- a/net/bpf/test_run.c >> +++ b/net/bpf/test_run.c >> @@ -112,8 +112,7 @@ struct xdp_test_data { >> u32 frame_cnt; >> }; >> >> -#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head) \ >> - - sizeof(struct skb_shared_info)) >> +#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head)) >> #define TEST_XDP_MAX_BATCH 256 >> >> static void xdp_test_run_init_page(struct page *page, void *arg) >> @@ -156,7 +155,6 @@ static int xdp_test_run_setup(struct xdp_test_data *xdp, struct xdp_buff *orig_c >> .flags = 0, >> .pool_size = xdp->batch_size, >> .nid = NUMA_NO_NODE, >> - .max_len = TEST_XDP_FRAME_SIZE, >> .init_callback = xdp_test_run_init_page, >> .init_arg = xdp, >> }; >> @@ -1230,6 +1228,8 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, >> batch_size = NAPI_POLL_WEIGHT; >> else if (batch_size > TEST_XDP_MAX_BATCH) >> return -E2BIG; >> + >> + headroom += sizeof(struct xdp_page_head); > The orig_ctx->data_end will ensure there is a sizeof(struct skb_shared_info) > tailroom ? It is quite tricky to read but I don't have a better idea > either. Yeah, the length checks are all done for the non-live data case (in bpf_test_init()), so seemed simpler to just account the extra headroom to those checks instead of adding an extra check to the live-packet code. > Acked-by: Martin KaFai Lau <kafai@xxxxxx> Thanks! -Toke