From 4676341af35e5087aa78bb65e6b4ad141cc8fe95 Mon Sep 17 00:00:00 2001 From: Paul Chaignon Date: Thu, 18 Sep 2025 18:46:33 +0200 Subject: [PATCH 1/5] bpf: Refactor cleanup of bpf_prog_test_run_skb This bit of refactoring aims to simplify the next patch in this series, in which freeing 'data' is a bit less straightforward. Tested-by: syzbot@syzkaller.appspotmail.com Signed-off-by: Paul Chaignon --- net/bpf/test_run.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 4a862d6053861..a9c81fec3290c 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -990,10 +990,10 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, u32 size = kattr->test.data_size_in; u32 repeat = kattr->test.repeat; struct __sk_buff *ctx = NULL; + struct sk_buff *skb = NULL; + struct sock *sk = NULL; u32 retval, duration; int hh_len = ETH_HLEN; - struct sk_buff *skb; - struct sock *sk; void *data; int ret; @@ -1009,8 +1009,9 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff)); if (IS_ERR(ctx)) { - kfree(data); - return PTR_ERR(ctx); + ret = PTR_ERR(ctx); + ctx = NULL; + goto out; } switch (prog->type) { @@ -1030,24 +1031,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, sk = sk_alloc(net, AF_UNSPEC, GFP_USER, &bpf_dummy_proto, 1); if (!sk) { - kfree(data); - kfree(ctx); - return -ENOMEM; + ret = -ENOMEM; + goto out; } sock_init_data(NULL, sk); skb = slab_build_skb(data); if (!skb) { - kfree(data); - kfree(ctx); - sk_free(sk); - return -ENOMEM; + ret = -ENOMEM; + goto out; } skb->sk = sk; skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); __skb_put(skb, size); + data = NULL; /* data released via kfree_skb */ + if (ctx && ctx->ifindex > 1) { dev = dev_get_by_index(net, ctx->ifindex); if (!dev) { @@ -1139,7 +1139,9 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, if (dev && dev != net->loopback_dev) dev_put(dev); kfree_skb(skb); - sk_free(sk); + kfree(data); + if (sk) + sk_free(sk); kfree(ctx); return ret; } From 6f8a6742906a9ecb3b7de516d2c18d02a7ab253e Mon Sep 17 00:00:00 2001 From: Paul Chaignon Date: Thu, 18 Sep 2025 18:47:05 +0200 Subject: [PATCH 2/5] bpf: Reorder bpf_prog_test_run_skb initialization This patch reorders the initialization of bpf_prog_test_run_skb to simplify the subsequent patch. Program types are checked first, followed by the ctx init, and finally the data init. With the subsequent patch, program types and the ctx init provide information that is used in the data init. Thus, we need the data init to happen last. Signed-off-by: Paul Chaignon --- net/bpf/test_run.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index a9c81fec3290c..00b12d7454793 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -1001,19 +1001,6 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, kattr->test.cpu || kattr->test.batch_size) return -EINVAL; - data = bpf_test_init(kattr, kattr->test.data_size_in, - size, NET_SKB_PAD + NET_IP_ALIGN, - SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); - if (IS_ERR(data)) - return PTR_ERR(data); - - ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff)); - if (IS_ERR(ctx)) { - ret = PTR_ERR(ctx); - ctx = NULL; - goto out; - } - switch (prog->type) { case BPF_PROG_TYPE_SCHED_CLS: case BPF_PROG_TYPE_SCHED_ACT: @@ -1029,6 +1016,19 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, break; } + ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff)); + if (IS_ERR(ctx)) + return PTR_ERR(ctx); + + data = bpf_test_init(kattr, kattr->test.data_size_in, + size, NET_SKB_PAD + NET_IP_ALIGN, + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); + if (IS_ERR(data)) { + ret = PTR_ERR(data); + data = NULL; + goto out; + } + sk = sk_alloc(net, AF_UNSPEC, GFP_USER, &bpf_dummy_proto, 1); if (!sk) { ret = -ENOMEM; From db97c4025418a23b5a9778c36876ff8642629009 Mon Sep 17 00:00:00 2001 From: Paul Chaignon Date: Thu, 18 Sep 2025 18:47:49 +0200 Subject: [PATCH 3/5] bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN This patch adds support for crafting non-linear skbs in BPF test runs for tc programs, via a new flag BPF_F_TEST_SKB_NON_LINEAR. When this flag is set, the size of the linear area is given by ctx->data_end, with a minimum of ETH_HLEN always pulled in the linear area. This is particularly useful to test support for non-linear skbs in large codebases such as Cilium. We've had multiple bugs in the past few years where we were missing calls to bpf_skb_pull_data(). This support in BPF_PROG_TEST_RUN would allow us to automatically cover this case in our BPF tests. In addition to the selftests introduced later in the series, this patch was tested by setting BPF_F_TEST_SKB_NON_LINEAR for all tc selftests programs and checking test failures were expected. Tested-by: syzbot@syzkaller.appspotmail.com Suggested-by: Daniel Borkmann Signed-off-by: Paul Chaignon --- net/bpf/test_run.c | 82 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 65 insertions(+), 17 deletions(-) diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 00b12d7454793..222a54c24c70e 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -660,21 +660,30 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_memb_release, KF_RELEASE) BTF_KFUNCS_END(test_sk_check_kfunc_ids) static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size, - u32 size, u32 headroom, u32 tailroom) + u32 size, u32 headroom, u32 tailroom, bool nonlinear) { void __user *data_in = u64_to_user_ptr(kattr->test.data_in); - void *data; + void *data, *dst; if (user_size < ETH_HLEN || user_size > PAGE_SIZE - headroom - tailroom) return ERR_PTR(-EINVAL); - size = SKB_DATA_ALIGN(size); - data = kzalloc(size + headroom + tailroom, GFP_USER); + /* In non-linear case, data_in is copied to the paged data */ + if (nonlinear) { + data = alloc_page(GFP_USER); + } else { + size = SKB_DATA_ALIGN(size); + data = kzalloc(size + headroom + tailroom, GFP_USER); + } if (!data) return ERR_PTR(-ENOMEM); - if (copy_from_user(data + headroom, data_in, user_size)) { - kfree(data); + if (nonlinear) + dst = page_address(data); + else + dst = data + headroom; + if (copy_from_user(dst, data_in, user_size)) { + nonlinear ? __free_page(data) : kfree(data); return ERR_PTR(-EFAULT); } @@ -910,6 +919,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb) /* cb is allowed */ if (!range_is_zero(__skb, offsetofend(struct __sk_buff, cb), + offsetof(struct __sk_buff, data_end))) + return -EINVAL; + + /* data_end is allowed, but not copied to skb */ + + if (!range_is_zero(__skb, offsetofend(struct __sk_buff, data_end), offsetof(struct __sk_buff, tstamp))) return -EINVAL; @@ -984,7 +999,8 @@ static struct proto bpf_dummy_proto = { int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, union bpf_attr __user *uattr) { - bool is_l2 = false, is_direct_pkt_access = false; + bool is_l2 = false, is_direct_pkt_access = false, is_nonlinear = false; + int hh_len = ETH_HLEN, linear_size = ETH_HLEN; struct net *net = current->nsproxy->net_ns; struct net_device *dev = net->loopback_dev; u32 size = kattr->test.data_size_in; @@ -993,7 +1009,6 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, struct sk_buff *skb = NULL; struct sock *sk = NULL; u32 retval, duration; - int hh_len = ETH_HLEN; void *data; int ret; @@ -1020,9 +1035,19 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, if (IS_ERR(ctx)) return PTR_ERR(ctx); + if (ctx && ctx->data_end && ctx->data_end < kattr->test.data_size_in) { + if (!is_l2) { + ret = -EINVAL; + goto out; + } + linear_size = max(ETH_HLEN, ctx->data_end); + is_nonlinear = true; + } + data = bpf_test_init(kattr, kattr->test.data_size_in, size, NET_SKB_PAD + NET_IP_ALIGN, - SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)), + is_nonlinear); if (IS_ERR(data)) { ret = PTR_ERR(data); data = NULL; @@ -1036,15 +1061,32 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, } sock_init_data(NULL, sk); - skb = slab_build_skb(data); + if (is_nonlinear) + skb = alloc_skb(NET_SKB_PAD + NET_IP_ALIGN + size + + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)), + GFP_USER); + else + skb = slab_build_skb(data); if (!skb) { ret = -ENOMEM; goto out; } + skb->sk = sk; skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); - __skb_put(skb, size); + + if (is_nonlinear) { + skb_fill_page_desc(skb, 0, data, 0, size); + skb->truesize += PAGE_SIZE; + skb->data_len = size; + skb->len = size; + + /* eth_type_trans expects the Ethernet header in the linear area. */ + __pskb_pull_tail(skb, linear_size); + } else { + __skb_put(skb, size); + } data = NULL; /* data released via kfree_skb */ @@ -1127,9 +1169,11 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, convert_skb_to___skb(skb, ctx); size = skb->len; - /* bpf program can never convert linear skb to non-linear */ - if (WARN_ON_ONCE(skb_is_nonlinear(skb))) + if (skb_is_nonlinear(skb)) { + /* bpf program can never convert linear skb to non-linear */ + WARN_ON_ONCE(!is_nonlinear); size = skb_headlen(skb); + } ret = bpf_test_finish(kattr, uattr, skb->data, NULL, size, retval, duration); if (!ret) @@ -1139,7 +1183,10 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, if (dev && dev != net->loopback_dev) dev_put(dev); kfree_skb(skb); - kfree(data); + if (!is_nonlinear) + kfree(data); + else if (data) + __free_page(data); if (sk) sk_free(sk); kfree(ctx); @@ -1265,7 +1312,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, size = max_data_sz; } - data = bpf_test_init(kattr, size, max_data_sz, headroom, tailroom); + data = bpf_test_init(kattr, size, max_data_sz, headroom, tailroom, false); if (IS_ERR(data)) { ret = PTR_ERR(data); goto free_ctx; @@ -1388,7 +1435,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, if (size < ETH_HLEN) return -EINVAL; - data = bpf_test_init(kattr, kattr->test.data_size_in, size, 0, 0); + data = bpf_test_init(kattr, kattr->test.data_size_in, size, 0, 0, false); if (IS_ERR(data)) return PTR_ERR(data); @@ -1661,7 +1708,8 @@ int bpf_prog_test_run_nf(struct bpf_prog *prog, data = bpf_test_init(kattr, kattr->test.data_size_in, size, NET_SKB_PAD + NET_IP_ALIGN, - SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)), + false); if (IS_ERR(data)) return PTR_ERR(data); From d8ebc6efef24f6b54e64bd78b56cbd92569c7b11 Mon Sep 17 00:00:00 2001 From: Paul Chaignon Date: Thu, 18 Sep 2025 18:49:00 +0200 Subject: [PATCH 4/5] selftests/bpf: Support non-linear flag in test loader This patch adds support for a new tag __linear_size in the test loader, to specify the size of the linear area in case of non-linear skbs. If the tag is absent or null, a linear skb is crafted. Signed-off-by: Paul Chaignon --- tools/testing/selftests/bpf/progs/bpf_misc.h | 4 ++++ .../bpf/progs/verifier_direct_packet_access.c | 1 + tools/testing/selftests/bpf/test_loader.c | 19 +++++++++++++++++-- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h index 1004c4a64aafb..570906607c206 100644 --- a/tools/testing/selftests/bpf/progs/bpf_misc.h +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h @@ -119,6 +119,9 @@ * Several __arch_* annotations could be specified at once. * When test case is not run on current arch it is marked as skipped. * __caps_unpriv Specify the capabilities that should be set when running the test. + * + * __linear_size Specify the size of the linear area of non-linear skbs, or + * 0 for linear skbs. */ #define __msg(msg) __attribute__((btf_decl_tag("comment:test_expect_msg=" XSTR(__COUNTER__) "=" msg))) #define __xlated(msg) __attribute__((btf_decl_tag("comment:test_expect_xlated=" XSTR(__COUNTER__) "=" msg))) @@ -150,6 +153,7 @@ #define __stderr_unpriv(msg) __attribute__((btf_decl_tag("comment:test_expect_stderr_unpriv=" XSTR(__COUNTER__) "=" msg))) #define __stdout(msg) __attribute__((btf_decl_tag("comment:test_expect_stdout=" XSTR(__COUNTER__) "=" msg))) #define __stdout_unpriv(msg) __attribute__((btf_decl_tag("comment:test_expect_stdout_unpriv=" XSTR(__COUNTER__) "=" msg))) +#define __linear_size(sz) __attribute__((btf_decl_tag("comment:test_linear_size=" XSTR(sz)))) /* Define common capabilities tested using __caps_unpriv */ #define CAP_NET_ADMIN 12 diff --git a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c index 28b602ac9cbe2..a61897e01a50d 100644 --- a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c +++ b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Converted from tools/testing/selftests/bpf/verifier/direct_packet_access.c */ +#include #include #include #include "bpf_misc.h" diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c index e065b467d5090..5fe1c3cd46056 100644 --- a/tools/testing/selftests/bpf/test_loader.c +++ b/tools/testing/selftests/bpf/test_loader.c @@ -42,6 +42,7 @@ #define TEST_TAG_EXPECT_STDERR_PFX_UNPRIV "comment:test_expect_stderr_unpriv=" #define TEST_TAG_EXPECT_STDOUT_PFX "comment:test_expect_stdout=" #define TEST_TAG_EXPECT_STDOUT_PFX_UNPRIV "comment:test_expect_stdout_unpriv=" +#define TEST_TAG_LINEAR_SIZE "comment:test_linear_size=" /* Warning: duplicated in bpf_misc.h */ #define POINTER_VALUE 0xbadcafe @@ -100,6 +101,7 @@ struct test_spec { int mode_mask; int arch_mask; int load_mask; + int linear_sz; bool auxiliary; bool valid; }; @@ -632,6 +634,11 @@ static int parse_test_spec(struct test_loader *tester, &spec->unpriv.stdout); if (err) goto cleanup; + } else if (str_has_pfx(s, TEST_TAG_LINEAR_SIZE)) { + val = s + sizeof(TEST_TAG_LINEAR_SIZE) - 1; + err = parse_int(val, &spec->linear_sz, "test linear size"); + if (err) + goto cleanup; } } @@ -906,10 +913,11 @@ static bool is_unpriv_capable_map(struct bpf_map *map) } } -static int do_prog_test_run(int fd_prog, int *retval, bool empty_opts) +static int do_prog_test_run(int fd_prog, int *retval, bool empty_opts, int linear_sz) { __u8 tmp_out[TEST_DATA_LEN << 2] = {}; __u8 tmp_in[TEST_DATA_LEN] = {}; + struct __sk_buff ctx = {}; int err, saved_errno; LIBBPF_OPTS(bpf_test_run_opts, topts, .data_in = tmp_in, @@ -919,6 +927,12 @@ static int do_prog_test_run(int fd_prog, int *retval, bool empty_opts) .repeat = 1, ); + if (linear_sz) { + ctx.data_end = linear_sz; + topts.ctx_in = &ctx; + topts.ctx_size_in = sizeof(ctx); + } + if (empty_opts) { memset(&topts, 0, sizeof(struct bpf_test_run_opts)); topts.sz = sizeof(struct bpf_test_run_opts); @@ -1168,7 +1182,8 @@ void run_subtest(struct test_loader *tester, } err = do_prog_test_run(bpf_program__fd(tprog), &retval, - bpf_program__type(tprog) == BPF_PROG_TYPE_SYSCALL ? true : false); + bpf_program__type(tprog) == BPF_PROG_TYPE_SYSCALL ? true : false, + spec->linear_sz); if (!err && retval != subspec->retval && subspec->retval != POINTER_VALUE) { PRINT_FAIL("Unexpected retval: %d != %d\n", retval, subspec->retval); goto tobj_cleanup; From 1dbd29ea7bb807a3130678a17e573eb49c9d832b Mon Sep 17 00:00:00 2001 From: Paul Chaignon Date: Thu, 18 Sep 2025 18:51:48 +0200 Subject: [PATCH 5/5] selftests/bpf: Test direct packet access on non-linear skbs This patch adds new selftests in the direct packet access suite, to cover the non-linear case with BPF_F_TEST_SKB_NON_LINEAR. The three first tests cover the behavior of the bounds check with a non-linear skb (first two with min. linear size, third with long enough linear size). The last test adds a call to bpf_skb_pull_data() to be able to access the packet. Signed-off-by: Paul Chaignon --- .../bpf/progs/verifier_direct_packet_access.c | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c index a61897e01a50d..c5745a4ae0fdb 100644 --- a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c +++ b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c @@ -801,4 +801,57 @@ l0_%=: /* exit(0) */ \ : __clobber_all); } +#define access_test_non_linear(name, desc, retval, linear_sz) \ + SEC("tc") \ + __description("direct packet access: " #name " (non-linear, " desc ")") \ + __success __retval(retval) \ + __linear_size(linear_sz) \ + __naked void access_##name(void) \ + { \ + asm volatile (" \ + r2 = *(u32*)(r1 + %[skb_data]); \ + r3 = *(u32*)(r1 + %[skb_data_end]); \ + r0 = r2; \ + r0 += 22; \ + if r0 > r3 goto l0_%=; \ + r0 = *(u8*)(r0 - 1); \ + exit; \ + l0_%=: r0 = 1; \ + exit; \ + " : \ + : __imm_const(skb_data, offsetof(struct __sk_buff, data)), \ + __imm_const(skb_data_end, offsetof(struct __sk_buff, data_end)) \ + : __clobber_all); \ + } + +access_test_non_linear(test31, "too short eth", 1, ETH_HLEN); +access_test_non_linear(test32, "too short 1", 1, 1); +access_test_non_linear(test33, "long enough", 0, 22); + +SEC("tc") +__description("direct packet access: test34 (non-linear, linearized)") +__success __retval(0) +__linear_size(ETH_HLEN) +__naked void access_test34_non_linear_linearized(void) +{ + asm volatile (" \ + r6 = r1; \ + r2 = 22; \ + call %[bpf_skb_pull_data]; \ + r2 = *(u32*)(r6 + %[skb_data]); \ + r3 = *(u32*)(r6 + %[skb_data_end]); \ + r0 = r2; \ + r0 += 22; \ + if r0 > r3 goto l0_%=; \ + r0 = *(u8*)(r0 - 1); \ + exit; \ +l0_%=: r0 = 1; \ + exit; \ +" : + : __imm(bpf_skb_pull_data), + __imm_const(skb_data, offsetof(struct __sk_buff, data)), + __imm_const(skb_data_end, offsetof(struct __sk_buff, data_end)) + : __clobber_all); +} + char _license[] SEC("license") = "GPL";