| Message ID | 20230818083920.3771-3-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Fix an issue in verifing allow_ptr_leaks | expand |
On Fri, Aug 18, 2023 at 1:39 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > - Without prev commit > > $ tools/testing/selftests/bpf/test_progs --name=tc_bpf > #232/1 tc_bpf/tc_bpf_root:OK > test_tc_bpf_non_root:PASS:set_cap_bpf_cap_net_admin 0 nsec > test_tc_bpf_non_root:PASS:disable_cap_sys_admin 0 nsec > 0: R1=ctx(off=0,imm=0) R10=fp0 > ; if ((long)(iph + 1) > (long)skb->data_end) > 0: (61) r2 = *(u32 *)(r1 +80) ; R1=ctx(off=0,imm=0) R2_w=pkt_end(off=0,imm=0) > ; struct iphdr *iph = (void *)(long)skb->data + sizeof(struct ethhdr); > 1: (61) r1 = *(u32 *)(r1 +76) ; R1_w=pkt(off=0,r=0,imm=0) > ; if ((long)(iph + 1) > (long)skb->data_end) > 2: (07) r1 += 34 ; R1_w=pkt(off=34,r=0,imm=0) > 3: (b4) w0 = 1 ; R0_w=1 > 4: (2d) if r1 > r2 goto pc+1 > R2 pointer comparison prohibited > processed 5 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > test_tc_bpf_non_root:FAIL:test_tc_bpf__open_and_load unexpected error: -13 > #233/2 tc_bpf_non_root:FAIL > > - With prev commit > > $ tools/testing/selftests/bpf/test_progs --name=tc_bpf > #232/1 tc_bpf/tc_bpf_root:OK > #232/2 tc_bpf/tc_bpf_non_root:OK > #232 tc_bpf:OK > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > --- > tools/testing/selftests/bpf/prog_tests/tc_bpf.c | 36 ++++++++++++++++++++++++- > tools/testing/selftests/bpf/progs/test_tc_bpf.c | 14 ++++++++++ > 2 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > index e873766..48b5553 100644 > --- a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > +++ b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > @@ -3,6 +3,7 @@ > #include <test_progs.h> > #include <linux/pkt_cls.h> > > +#include "cap_helpers.h" > #include "test_tc_bpf.skel.h" > > #define LO_IFINDEX 1 > @@ -327,7 +328,7 @@ static int test_tc_bpf_api(struct bpf_tc_hook *hook, int fd) > return 0; > } > > -void test_tc_bpf(void) > +void tc_bpf_root(void) > { > DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = LO_IFINDEX, > .attach_point = BPF_TC_INGRESS); > @@ -393,3 +394,36 @@ void test_tc_bpf(void) > } > test_tc_bpf__destroy(skel); > } > + > +void tc_bpf_non_root(void) > +{ > + struct test_tc_bpf *skel = NULL; > + __u64 caps = 0; > + int ret; > + > + /* In case CAP_BPF and CAP_PERFMON is not set */ > + ret = cap_enable_effective(1ULL << CAP_BPF | 1ULL << CAP_NET_ADMIN, &caps); > + if (!ASSERT_OK(ret, "set_cap_bpf_cap_net_admin")) > + return; > + ret = cap_disable_effective(1ULL << CAP_SYS_ADMIN | 1ULL << CAP_PERFMON, NULL); > + if (!ASSERT_OK(ret, "disable_cap_sys_admin")) > + goto restore_cap; > + > + skel = test_tc_bpf__open_and_load(); > + if (!ASSERT_OK_PTR(skel, "test_tc_bpf__open_and_load")) > + goto restore_cap; > + > + test_tc_bpf__destroy(skel); > + > +restore_cap: > + if (caps) > + cap_enable_effective(caps, NULL); > +} > + > +void test_tc_bpf(void) > +{ > + if (test__start_subtest("tc_bpf_root")) > + tc_bpf_root(); > + if (test__start_subtest("tc_bpf_non_root")) > + tc_bpf_non_root(); > +} > diff --git a/tools/testing/selftests/bpf/progs/test_tc_bpf.c b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > index d28ca8d..3e0f218 100644 > --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c > +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > @@ -1,5 +1,8 @@ > // SPDX-License-Identifier: GPL-2.0 > > +#include <linux/pkt_cls.h> > +#include <linux/ip.h> > +#include <linux/if_ether.h> Due to above it fails to compile: In file included from progs/test_tc_bpf.c:4: In file included from /usr/include/linux/ip.h:21: In file included from /usr/include/asm/byteorder.h:5: In file included from /usr/include/linux/byteorder/little_endian.h:13: /usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline' 136 | static __always_inline unsigned long __swab(const unsigned long y) | ^
On Tue, Aug 22, 2023 at 6:45 AM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Aug 18, 2023 at 1:39 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > - Without prev commit > > > > $ tools/testing/selftests/bpf/test_progs --name=tc_bpf > > #232/1 tc_bpf/tc_bpf_root:OK > > test_tc_bpf_non_root:PASS:set_cap_bpf_cap_net_admin 0 nsec > > test_tc_bpf_non_root:PASS:disable_cap_sys_admin 0 nsec > > 0: R1=ctx(off=0,imm=0) R10=fp0 > > ; if ((long)(iph + 1) > (long)skb->data_end) > > 0: (61) r2 = *(u32 *)(r1 +80) ; R1=ctx(off=0,imm=0) R2_w=pkt_end(off=0,imm=0) > > ; struct iphdr *iph = (void *)(long)skb->data + sizeof(struct ethhdr); > > 1: (61) r1 = *(u32 *)(r1 +76) ; R1_w=pkt(off=0,r=0,imm=0) > > ; if ((long)(iph + 1) > (long)skb->data_end) > > 2: (07) r1 += 34 ; R1_w=pkt(off=34,r=0,imm=0) > > 3: (b4) w0 = 1 ; R0_w=1 > > 4: (2d) if r1 > r2 goto pc+1 > > R2 pointer comparison prohibited > > processed 5 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > > test_tc_bpf_non_root:FAIL:test_tc_bpf__open_and_load unexpected error: -13 > > #233/2 tc_bpf_non_root:FAIL > > > > - With prev commit > > > > $ tools/testing/selftests/bpf/test_progs --name=tc_bpf > > #232/1 tc_bpf/tc_bpf_root:OK > > #232/2 tc_bpf/tc_bpf_non_root:OK > > #232 tc_bpf:OK > > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > --- > > tools/testing/selftests/bpf/prog_tests/tc_bpf.c | 36 ++++++++++++++++++++++++- > > tools/testing/selftests/bpf/progs/test_tc_bpf.c | 14 ++++++++++ > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > index e873766..48b5553 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > +++ b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > @@ -3,6 +3,7 @@ > > #include <test_progs.h> > > #include <linux/pkt_cls.h> > > > > +#include "cap_helpers.h" > > #include "test_tc_bpf.skel.h" > > > > #define LO_IFINDEX 1 > > @@ -327,7 +328,7 @@ static int test_tc_bpf_api(struct bpf_tc_hook *hook, int fd) > > return 0; > > } > > > > -void test_tc_bpf(void) > > +void tc_bpf_root(void) > > { > > DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = LO_IFINDEX, > > .attach_point = BPF_TC_INGRESS); > > @@ -393,3 +394,36 @@ void test_tc_bpf(void) > > } > > test_tc_bpf__destroy(skel); > > } > > + > > +void tc_bpf_non_root(void) > > +{ > > + struct test_tc_bpf *skel = NULL; > > + __u64 caps = 0; > > + int ret; > > + > > + /* In case CAP_BPF and CAP_PERFMON is not set */ > > + ret = cap_enable_effective(1ULL << CAP_BPF | 1ULL << CAP_NET_ADMIN, &caps); > > + if (!ASSERT_OK(ret, "set_cap_bpf_cap_net_admin")) > > + return; > > + ret = cap_disable_effective(1ULL << CAP_SYS_ADMIN | 1ULL << CAP_PERFMON, NULL); > > + if (!ASSERT_OK(ret, "disable_cap_sys_admin")) > > + goto restore_cap; > > + > > + skel = test_tc_bpf__open_and_load(); > > + if (!ASSERT_OK_PTR(skel, "test_tc_bpf__open_and_load")) > > + goto restore_cap; > > + > > + test_tc_bpf__destroy(skel); > > + > > +restore_cap: > > + if (caps) > > + cap_enable_effective(caps, NULL); > > +} > > + > > +void test_tc_bpf(void) > > +{ > > + if (test__start_subtest("tc_bpf_root")) > > + tc_bpf_root(); > > + if (test__start_subtest("tc_bpf_non_root")) > > + tc_bpf_non_root(); > > +} > > diff --git a/tools/testing/selftests/bpf/progs/test_tc_bpf.c b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > index d28ca8d..3e0f218 100644 > > --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > @@ -1,5 +1,8 @@ > > // SPDX-License-Identifier: GPL-2.0 > > > > +#include <linux/pkt_cls.h> > > +#include <linux/ip.h> > > +#include <linux/if_ether.h> > > Due to above it fails to compile: > > In file included from progs/test_tc_bpf.c:4: > In file included from /usr/include/linux/ip.h:21: > In file included from /usr/include/asm/byteorder.h:5: > In file included from /usr/include/linux/byteorder/little_endian.h:13: > /usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline' > 136 | static __always_inline unsigned long __swab(const unsigned long y) > | ^ I can't find the above error log in the BPF CI log. The BPF CI log just shows that it fails the test_map on s390 without logs. Not sure why. __always_inline is defined in bpf_helpers.h, so I think below additional change could fix it. --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c @@ -1,10 +1,10 @@ // SPDX-License-Identifier: GPL-2.0 +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> #include <linux/pkt_cls.h> #include <linux/ip.h> #include <linux/if_ether.h> -#include <linux/bpf.h> -#include <bpf/bpf_helpers.h>
On Mon, Aug 21, 2023 at 7:43 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > On Tue, Aug 22, 2023 at 6:45 AM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Fri, Aug 18, 2023 at 1:39 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > > > - Without prev commit > > > > > > $ tools/testing/selftests/bpf/test_progs --name=tc_bpf > > > #232/1 tc_bpf/tc_bpf_root:OK > > > test_tc_bpf_non_root:PASS:set_cap_bpf_cap_net_admin 0 nsec > > > test_tc_bpf_non_root:PASS:disable_cap_sys_admin 0 nsec > > > 0: R1=ctx(off=0,imm=0) R10=fp0 > > > ; if ((long)(iph + 1) > (long)skb->data_end) > > > 0: (61) r2 = *(u32 *)(r1 +80) ; R1=ctx(off=0,imm=0) R2_w=pkt_end(off=0,imm=0) > > > ; struct iphdr *iph = (void *)(long)skb->data + sizeof(struct ethhdr); > > > 1: (61) r1 = *(u32 *)(r1 +76) ; R1_w=pkt(off=0,r=0,imm=0) > > > ; if ((long)(iph + 1) > (long)skb->data_end) > > > 2: (07) r1 += 34 ; R1_w=pkt(off=34,r=0,imm=0) > > > 3: (b4) w0 = 1 ; R0_w=1 > > > 4: (2d) if r1 > r2 goto pc+1 > > > R2 pointer comparison prohibited > > > processed 5 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > > > test_tc_bpf_non_root:FAIL:test_tc_bpf__open_and_load unexpected error: -13 > > > #233/2 tc_bpf_non_root:FAIL > > > > > > - With prev commit > > > > > > $ tools/testing/selftests/bpf/test_progs --name=tc_bpf > > > #232/1 tc_bpf/tc_bpf_root:OK > > > #232/2 tc_bpf/tc_bpf_non_root:OK > > > #232 tc_bpf:OK > > > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED > > > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > > --- > > > tools/testing/selftests/bpf/prog_tests/tc_bpf.c | 36 ++++++++++++++++++++++++- > > > tools/testing/selftests/bpf/progs/test_tc_bpf.c | 14 ++++++++++ > > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > > index e873766..48b5553 100644 > > > --- a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > > +++ b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > > @@ -3,6 +3,7 @@ > > > #include <test_progs.h> > > > #include <linux/pkt_cls.h> > > > > > > +#include "cap_helpers.h" > > > #include "test_tc_bpf.skel.h" > > > > > > #define LO_IFINDEX 1 > > > @@ -327,7 +328,7 @@ static int test_tc_bpf_api(struct bpf_tc_hook *hook, int fd) > > > return 0; > > > } > > > > > > -void test_tc_bpf(void) > > > +void tc_bpf_root(void) > > > { > > > DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = LO_IFINDEX, > > > .attach_point = BPF_TC_INGRESS); > > > @@ -393,3 +394,36 @@ void test_tc_bpf(void) > > > } > > > test_tc_bpf__destroy(skel); > > > } > > > + > > > +void tc_bpf_non_root(void) > > > +{ > > > + struct test_tc_bpf *skel = NULL; > > > + __u64 caps = 0; > > > + int ret; > > > + > > > + /* In case CAP_BPF and CAP_PERFMON is not set */ > > > + ret = cap_enable_effective(1ULL << CAP_BPF | 1ULL << CAP_NET_ADMIN, &caps); > > > + if (!ASSERT_OK(ret, "set_cap_bpf_cap_net_admin")) > > > + return; > > > + ret = cap_disable_effective(1ULL << CAP_SYS_ADMIN | 1ULL << CAP_PERFMON, NULL); > > > + if (!ASSERT_OK(ret, "disable_cap_sys_admin")) > > > + goto restore_cap; > > > + > > > + skel = test_tc_bpf__open_and_load(); > > > + if (!ASSERT_OK_PTR(skel, "test_tc_bpf__open_and_load")) > > > + goto restore_cap; > > > + > > > + test_tc_bpf__destroy(skel); > > > + > > > +restore_cap: > > > + if (caps) > > > + cap_enable_effective(caps, NULL); > > > +} > > > + > > > +void test_tc_bpf(void) > > > +{ > > > + if (test__start_subtest("tc_bpf_root")) > > > + tc_bpf_root(); > > > + if (test__start_subtest("tc_bpf_non_root")) > > > + tc_bpf_non_root(); > > > +} > > > diff --git a/tools/testing/selftests/bpf/progs/test_tc_bpf.c b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > index d28ca8d..3e0f218 100644 > > > --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > @@ -1,5 +1,8 @@ > > > // SPDX-License-Identifier: GPL-2.0 > > > > > > +#include <linux/pkt_cls.h> > > > +#include <linux/ip.h> > > > +#include <linux/if_ether.h> > > > > Due to above it fails to compile: > > > > In file included from progs/test_tc_bpf.c:4: > > In file included from /usr/include/linux/ip.h:21: > > In file included from /usr/include/asm/byteorder.h:5: > > In file included from /usr/include/linux/byteorder/little_endian.h:13: > > /usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline' > > 136 | static __always_inline unsigned long __swab(const unsigned long y) > > | ^ > > I can't find the above error log in the BPF CI log. > The BPF CI log just shows that it fails the test_map on s390 without > logs. Not sure why. It's not in CI. I caught it by manual build. > __always_inline is defined in bpf_helpers.h, so I think below > additional change could fix it. > > --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c > +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > @@ -1,10 +1,10 @@ > // SPDX-License-Identifier: GPL-2.0 > > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > #include <linux/pkt_cls.h> > #include <linux/ip.h> > #include <linux/if_ether.h> > -#include <linux/bpf.h> > -#include <bpf/bpf_helpers.h> Maybe, but I'd rather remove the includes and replace TC_ACT_STOLEN/ACT_OK with zeros, since the return values are irrelevant.
On Tue, Aug 22, 2023 at 11:28 AM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Mon, Aug 21, 2023 at 7:43 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > On Tue, Aug 22, 2023 at 6:45 AM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Fri, Aug 18, 2023 at 1:39 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > > > > > - Without prev commit > > > > > > > > $ tools/testing/selftests/bpf/test_progs --name=tc_bpf > > > > #232/1 tc_bpf/tc_bpf_root:OK > > > > test_tc_bpf_non_root:PASS:set_cap_bpf_cap_net_admin 0 nsec > > > > test_tc_bpf_non_root:PASS:disable_cap_sys_admin 0 nsec > > > > 0: R1=ctx(off=0,imm=0) R10=fp0 > > > > ; if ((long)(iph + 1) > (long)skb->data_end) > > > > 0: (61) r2 = *(u32 *)(r1 +80) ; R1=ctx(off=0,imm=0) R2_w=pkt_end(off=0,imm=0) > > > > ; struct iphdr *iph = (void *)(long)skb->data + sizeof(struct ethhdr); > > > > 1: (61) r1 = *(u32 *)(r1 +76) ; R1_w=pkt(off=0,r=0,imm=0) > > > > ; if ((long)(iph + 1) > (long)skb->data_end) > > > > 2: (07) r1 += 34 ; R1_w=pkt(off=34,r=0,imm=0) > > > > 3: (b4) w0 = 1 ; R0_w=1 > > > > 4: (2d) if r1 > r2 goto pc+1 > > > > R2 pointer comparison prohibited > > > > processed 5 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > > > > test_tc_bpf_non_root:FAIL:test_tc_bpf__open_and_load unexpected error: -13 > > > > #233/2 tc_bpf_non_root:FAIL > > > > > > > > - With prev commit > > > > > > > > $ tools/testing/selftests/bpf/test_progs --name=tc_bpf > > > > #232/1 tc_bpf/tc_bpf_root:OK > > > > #232/2 tc_bpf/tc_bpf_non_root:OK > > > > #232 tc_bpf:OK > > > > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED > > > > > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > > > --- > > > > tools/testing/selftests/bpf/prog_tests/tc_bpf.c | 36 ++++++++++++++++++++++++- > > > > tools/testing/selftests/bpf/progs/test_tc_bpf.c | 14 ++++++++++ > > > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > > > index e873766..48b5553 100644 > > > > --- a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > > > +++ b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c > > > > @@ -3,6 +3,7 @@ > > > > #include <test_progs.h> > > > > #include <linux/pkt_cls.h> > > > > > > > > +#include "cap_helpers.h" > > > > #include "test_tc_bpf.skel.h" > > > > > > > > #define LO_IFINDEX 1 > > > > @@ -327,7 +328,7 @@ static int test_tc_bpf_api(struct bpf_tc_hook *hook, int fd) > > > > return 0; > > > > } > > > > > > > > -void test_tc_bpf(void) > > > > +void tc_bpf_root(void) > > > > { > > > > DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = LO_IFINDEX, > > > > .attach_point = BPF_TC_INGRESS); > > > > @@ -393,3 +394,36 @@ void test_tc_bpf(void) > > > > } > > > > test_tc_bpf__destroy(skel); > > > > } > > > > + > > > > +void tc_bpf_non_root(void) > > > > +{ > > > > + struct test_tc_bpf *skel = NULL; > > > > + __u64 caps = 0; > > > > + int ret; > > > > + > > > > + /* In case CAP_BPF and CAP_PERFMON is not set */ > > > > + ret = cap_enable_effective(1ULL << CAP_BPF | 1ULL << CAP_NET_ADMIN, &caps); > > > > + if (!ASSERT_OK(ret, "set_cap_bpf_cap_net_admin")) > > > > + return; > > > > + ret = cap_disable_effective(1ULL << CAP_SYS_ADMIN | 1ULL << CAP_PERFMON, NULL); > > > > + if (!ASSERT_OK(ret, "disable_cap_sys_admin")) > > > > + goto restore_cap; > > > > + > > > > + skel = test_tc_bpf__open_and_load(); > > > > + if (!ASSERT_OK_PTR(skel, "test_tc_bpf__open_and_load")) > > > > + goto restore_cap; > > > > + > > > > + test_tc_bpf__destroy(skel); > > > > + > > > > +restore_cap: > > > > + if (caps) > > > > + cap_enable_effective(caps, NULL); > > > > +} > > > > + > > > > +void test_tc_bpf(void) > > > > +{ > > > > + if (test__start_subtest("tc_bpf_root")) > > > > + tc_bpf_root(); > > > > + if (test__start_subtest("tc_bpf_non_root")) > > > > + tc_bpf_non_root(); > > > > +} > > > > diff --git a/tools/testing/selftests/bpf/progs/test_tc_bpf.c b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > > index d28ca8d..3e0f218 100644 > > > > --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > > +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > > @@ -1,5 +1,8 @@ > > > > // SPDX-License-Identifier: GPL-2.0 > > > > > > > > +#include <linux/pkt_cls.h> > > > > +#include <linux/ip.h> > > > > +#include <linux/if_ether.h> > > > > > > Due to above it fails to compile: > > > > > > In file included from progs/test_tc_bpf.c:4: > > > In file included from /usr/include/linux/ip.h:21: > > > In file included from /usr/include/asm/byteorder.h:5: > > > In file included from /usr/include/linux/byteorder/little_endian.h:13: > > > /usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline' > > > 136 | static __always_inline unsigned long __swab(const unsigned long y) > > > | ^ > > > > I can't find the above error log in the BPF CI log. > > The BPF CI log just shows that it fails the test_map on s390 without > > logs. Not sure why. > > It's not in CI. I caught it by manual build. > > > __always_inline is defined in bpf_helpers.h, so I think below > > additional change could fix it. > > > > --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > @@ -1,10 +1,10 @@ > > // SPDX-License-Identifier: GPL-2.0 > > > > +#include <linux/bpf.h> > > +#include <bpf/bpf_helpers.h> > > #include <linux/pkt_cls.h> > > #include <linux/ip.h> > > #include <linux/if_ether.h> > > -#include <linux/bpf.h> > > -#include <bpf/bpf_helpers.h> > > Maybe, but I'd rather remove the includes and replace > TC_ACT_STOLEN/ACT_OK with zeros, since the return values are > irrelevant. Good point. That could remove the "linux/pkt_cls.h".
On Mon, 2023-08-21 at 15:45 -0700, Alexei Starovoitov wrote: [...] > > diff --git a/tools/testing/selftests/bpf/progs/test_tc_bpf.c b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > index d28ca8d..3e0f218 100644 > > --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > @@ -1,5 +1,8 @@ > > // SPDX-License-Identifier: GPL-2.0 > > > > +#include <linux/pkt_cls.h> > > +#include <linux/ip.h> > > +#include <linux/if_ether.h> > > Due to above it fails to compile: > > In file included from progs/test_tc_bpf.c:4: > In file included from /usr/include/linux/ip.h:21: > In file included from /usr/include/asm/byteorder.h:5: > In file included from /usr/include/linux/byteorder/little_endian.h:13: > /usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline' > 136 | static __always_inline unsigned long __swab(const unsigned long y) > | ^ > This is strange, I can compile it no problem. On my system: /usr/include/linux/swab.h includes /usr/include/linux/stddef.h which defines __always_inline. What distro are you using? I want to try it in chroot to see if we have any issues with test makefiles.
On Wed, Aug 23, 2023 at 2:45 AM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Mon, 2023-08-21 at 15:45 -0700, Alexei Starovoitov wrote: > [...] > > > diff --git a/tools/testing/selftests/bpf/progs/test_tc_bpf.c b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > index d28ca8d..3e0f218 100644 > > > --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c > > > @@ -1,5 +1,8 @@ > > > // SPDX-License-Identifier: GPL-2.0 > > > > > > +#include <linux/pkt_cls.h> > > > +#include <linux/ip.h> > > > +#include <linux/if_ether.h> > > > > Due to above it fails to compile: > > > > In file included from progs/test_tc_bpf.c:4: > > In file included from /usr/include/linux/ip.h:21: > > In file included from /usr/include/asm/byteorder.h:5: > > In file included from /usr/include/linux/byteorder/little_endian.h:13: > > /usr/include/linux/swab.h:136:8: error: unknown type name '__always_inline' > > 136 | static __always_inline unsigned long __swab(const unsigned long y) > > | ^ > > > > This is strange, I can compile it no problem. On my system: > /usr/include/linux/swab.h includes /usr/include/linux/stddef.h > which defines __always_inline. > > What distro are you using? That was centos8. dnf provides /usr/include/linux/swab.h kernel-headers-6.4.3.x86_64 : Header files for the Linux kernel for use by glibc Repo : kernel Matched from: Filename : /usr/include/linux/swab.h
diff --git a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c index e873766..48b5553 100644 --- a/tools/testing/selftests/bpf/prog_tests/tc_bpf.c +++ b/tools/testing/selftests/bpf/prog_tests/tc_bpf.c @@ -3,6 +3,7 @@ #include <test_progs.h> #include <linux/pkt_cls.h> +#include "cap_helpers.h" #include "test_tc_bpf.skel.h" #define LO_IFINDEX 1 @@ -327,7 +328,7 @@ static int test_tc_bpf_api(struct bpf_tc_hook *hook, int fd) return 0; } -void test_tc_bpf(void) +void tc_bpf_root(void) { DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = LO_IFINDEX, .attach_point = BPF_TC_INGRESS); @@ -393,3 +394,36 @@ void test_tc_bpf(void) } test_tc_bpf__destroy(skel); } + +void tc_bpf_non_root(void) +{ + struct test_tc_bpf *skel = NULL; + __u64 caps = 0; + int ret; + + /* In case CAP_BPF and CAP_PERFMON is not set */ + ret = cap_enable_effective(1ULL << CAP_BPF | 1ULL << CAP_NET_ADMIN, &caps); + if (!ASSERT_OK(ret, "set_cap_bpf_cap_net_admin")) + return; + ret = cap_disable_effective(1ULL << CAP_SYS_ADMIN | 1ULL << CAP_PERFMON, NULL); + if (!ASSERT_OK(ret, "disable_cap_sys_admin")) + goto restore_cap; + + skel = test_tc_bpf__open_and_load(); + if (!ASSERT_OK_PTR(skel, "test_tc_bpf__open_and_load")) + goto restore_cap; + + test_tc_bpf__destroy(skel); + +restore_cap: + if (caps) + cap_enable_effective(caps, NULL); +} + +void test_tc_bpf(void) +{ + if (test__start_subtest("tc_bpf_root")) + tc_bpf_root(); + if (test__start_subtest("tc_bpf_non_root")) + tc_bpf_non_root(); +} diff --git a/tools/testing/selftests/bpf/progs/test_tc_bpf.c b/tools/testing/selftests/bpf/progs/test_tc_bpf.c index d28ca8d..3e0f218 100644 --- a/tools/testing/selftests/bpf/progs/test_tc_bpf.c +++ b/tools/testing/selftests/bpf/progs/test_tc_bpf.c @@ -1,5 +1,8 @@ // SPDX-License-Identifier: GPL-2.0 +#include <linux/pkt_cls.h> +#include <linux/ip.h> +#include <linux/if_ether.h> #include <linux/bpf.h> #include <bpf/bpf_helpers.h> @@ -10,3 +13,14 @@ int cls(struct __sk_buff *skb) { return 0; } + +/* Prog to verify tc-bpf without cap_sys_admin and cap_perfmon */ +SEC("tcx/ingress") +int pkt_ptr(struct __sk_buff *skb) +{ + struct iphdr *iph = (void *)(long)skb->data + sizeof(struct ethhdr); + + if ((long)(iph + 1) > (long)skb->data_end) + return TC_ACT_STOLEN; + return TC_ACT_OK; +}
- Without prev commit $ tools/testing/selftests/bpf/test_progs --name=tc_bpf #232/1 tc_bpf/tc_bpf_root:OK test_tc_bpf_non_root:PASS:set_cap_bpf_cap_net_admin 0 nsec test_tc_bpf_non_root:PASS:disable_cap_sys_admin 0 nsec 0: R1=ctx(off=0,imm=0) R10=fp0 ; if ((long)(iph + 1) > (long)skb->data_end) 0: (61) r2 = *(u32 *)(r1 +80) ; R1=ctx(off=0,imm=0) R2_w=pkt_end(off=0,imm=0) ; struct iphdr *iph = (void *)(long)skb->data + sizeof(struct ethhdr); 1: (61) r1 = *(u32 *)(r1 +76) ; R1_w=pkt(off=0,r=0,imm=0) ; if ((long)(iph + 1) > (long)skb->data_end) 2: (07) r1 += 34 ; R1_w=pkt(off=34,r=0,imm=0) 3: (b4) w0 = 1 ; R0_w=1 4: (2d) if r1 > r2 goto pc+1 R2 pointer comparison prohibited processed 5 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 test_tc_bpf_non_root:FAIL:test_tc_bpf__open_and_load unexpected error: -13 #233/2 tc_bpf_non_root:FAIL - With prev commit $ tools/testing/selftests/bpf/test_progs --name=tc_bpf #232/1 tc_bpf/tc_bpf_root:OK #232/2 tc_bpf/tc_bpf_non_root:OK #232 tc_bpf:OK Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- tools/testing/selftests/bpf/prog_tests/tc_bpf.c | 36 ++++++++++++++++++++++++- tools/testing/selftests/bpf/progs/test_tc_bpf.c | 14 ++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-)