Skip to content

Commit 94bbbdf

Browse files
author
Alexei Starovoitov
committed
Merge branch 'double-fix bpf_test_run + XDP_PASS recycling'
Alexander Lobakin says: ==================== Enabling skb PP recycling revealed a couple issues in the bpf_test_run code. Recycling broke the assumption that the headroom won't ever be touched during the test_run execution: xdp_scrub_frame() invalidates the XDP frame at the headroom start, while neigh xmit code overwrites 2 bytes to the left of the Ethernet header. The first makes the kernel panic in certain cases, while the second breaks xdp_do_redirect selftest on BE. test_run is a limited-scope entity, so let's hope no more corner cases will happen here or at least they will be as easy and pleasant to fix as those two. ==================== Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents 082cdc6 + 5640b6d commit 94bbbdf

File tree

3 files changed

+16
-5
lines changed

3 files changed

+16
-5
lines changed

net/bpf/test_run.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,16 @@ static void xdp_test_run_teardown(struct xdp_test_data *xdp)
208208
kfree(xdp->skbs);
209209
}
210210

211+
static bool frame_was_changed(const struct xdp_page_head *head)
212+
{
213+
/* xdp_scrub_frame() zeroes the data pointer, flags is the last field,
214+
* i.e. has the highest chances to be overwritten. If those two are
215+
* untouched, it's most likely safe to skip the context reset.
216+
*/
217+
return head->frm.data != head->orig_ctx.data ||
218+
head->frm.flags != head->orig_ctx.flags;
219+
}
220+
211221
static bool ctx_was_changed(struct xdp_page_head *head)
212222
{
213223
return head->orig_ctx.data != head->ctx.data ||
@@ -217,7 +227,7 @@ static bool ctx_was_changed(struct xdp_page_head *head)
217227

218228
static void reset_ctx(struct xdp_page_head *head)
219229
{
220-
if (likely(!ctx_was_changed(head)))
230+
if (likely(!frame_was_changed(head) && !ctx_was_changed(head)))
221231
return;
222232

223233
head->ctx.data = head->orig_ctx.data;

tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ static void test_max_pkt_size(int fd)
8686
void test_xdp_do_redirect(void)
8787
{
8888
int err, xdp_prog_fd, tc_prog_fd, ifindex_src, ifindex_dst;
89-
char data[sizeof(pkt_udp) + sizeof(__u32)];
89+
char data[sizeof(pkt_udp) + sizeof(__u64)];
9090
struct test_xdp_do_redirect *skel = NULL;
9191
struct nstoken *nstoken = NULL;
9292
struct bpf_link *link;
9393
LIBBPF_OPTS(bpf_xdp_query_opts, query_opts);
94-
struct xdp_md ctx_in = { .data = sizeof(__u32),
94+
struct xdp_md ctx_in = { .data = sizeof(__u64),
9595
.data_end = sizeof(data) };
9696
DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts,
9797
.data_in = &data,
@@ -105,8 +105,9 @@ void test_xdp_do_redirect(void)
105105
DECLARE_LIBBPF_OPTS(bpf_tc_hook, tc_hook,
106106
.attach_point = BPF_TC_INGRESS);
107107

108-
memcpy(&data[sizeof(__u32)], &pkt_udp, sizeof(pkt_udp));
108+
memcpy(&data[sizeof(__u64)], &pkt_udp, sizeof(pkt_udp));
109109
*((__u32 *)data) = 0x42; /* metadata test value */
110+
*((__u32 *)data + 4) = 0;
110111

111112
skel = test_xdp_do_redirect__open();
112113
if (!ASSERT_OK_PTR(skel, "skel"))

tools/testing/selftests/bpf/progs/test_xdp_do_redirect.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ int xdp_redirect(struct xdp_md *xdp)
5252

5353
*payload = MARK_IN;
5454

55-
if (bpf_xdp_adjust_meta(xdp, 4))
55+
if (bpf_xdp_adjust_meta(xdp, sizeof(__u64)))
5656
return XDP_ABORTED;
5757

5858
if (retcode > XDP_PASS)

0 commit comments

Comments
 (0)