| Message ID | 4a0f45692b124b7bca139a6c58c131496ec2dc12.1607973529.git.me@ubique.spb.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Add support of pointer to struct in global functions | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 5 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 85 exceeds 80 columns WARNING: line length of 95 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Mon, Dec 14, 2020 at 11:53 AM Dmitrii Banshchikov <me@ubique.spb.ru> wrote: > > test_global_func9 - check valid scenarios for struct pointers > test_global_func10 - check that the smaller struct cannot be passed as a > the larger one > test_global_func11 - check that CTX pointer cannot be passed as a struct > pointer > test_global_func12 - check access to a null pointer > test_global_func13 - check access to an arbitrary pointer value > > Signed-off-by: Dmitrii Banshchikov <me@ubique.spb.ru> > --- > .../bpf/prog_tests/test_global_funcs.c | 5 ++ > .../selftests/bpf/progs/test_global_func10.c | 29 +++++++++ > .../selftests/bpf/progs/test_global_func11.c | 19 ++++++ > .../selftests/bpf/progs/test_global_func12.c | 21 +++++++ > .../selftests/bpf/progs/test_global_func13.c | 24 ++++++++ > .../selftests/bpf/progs/test_global_func9.c | 59 +++++++++++++++++++ > 6 files changed, 157 insertions(+) > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func10.c > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func11.c > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func12.c > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func13.c > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func9.c > [...] > + > +SEC("cgroup_skb/ingress") > +int test_cls(struct __sk_buff *skb) > +{ > + int result = 0; > + > + { > + const struct S s = {.x = skb->len }; > + > + result |= foo(&s); > + } > + > + { > + const __u32 key = 1; > + const struct S *s = bpf_map_lookup_elem(&map, &key); > + > + result |= foo(s); > + } Can you please also add a test with passing a pointer to a global variable as an input parameter? Also, none of these tests seem to validate that correct data is read and returned from foo. So it would be nice to have a dedicated selftest (not part of test_global_func) that would pass some input parameters (easiest to do with global variables) and see that the subprogram returns it correctly. > + > + { > + const struct C c = {.x = skb->len, .y = skb->family }; > + > + result |= foo((const struct S *)&c); > + } > + > + { > + result |= foo(NULL); > + } > + > + return result ? 1 : 0; > +} > -- > 2.25.1 >
On 12/14/20 11:52 AM, Dmitrii Banshchikov wrote: > test_global_func9 - check valid scenarios for struct pointers > test_global_func10 - check that the smaller struct cannot be passed as a > the larger one > test_global_func11 - check that CTX pointer cannot be passed as a struct > pointer > test_global_func12 - check access to a null pointer > test_global_func13 - check access to an arbitrary pointer value > > Signed-off-by: Dmitrii Banshchikov <me@ubique.spb.ru> > --- > .../bpf/prog_tests/test_global_funcs.c | 5 ++ > .../selftests/bpf/progs/test_global_func10.c | 29 +++++++++ > .../selftests/bpf/progs/test_global_func11.c | 19 ++++++ > .../selftests/bpf/progs/test_global_func12.c | 21 +++++++ > .../selftests/bpf/progs/test_global_func13.c | 24 ++++++++ > .../selftests/bpf/progs/test_global_func9.c | 59 +++++++++++++++++++ > 6 files changed, 157 insertions(+) > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func10.c > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func11.c > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func12.c > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func13.c > create mode 100644 tools/testing/selftests/bpf/progs/test_global_func9.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c b/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c > index 32e4348b714b..c4895e6c83c2 100644 > --- a/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c > +++ b/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c > @@ -61,6 +61,11 @@ void test_test_global_funcs(void) > { "test_global_func6.o" , "modified ctx ptr R2" }, > { "test_global_func7.o" , "foo() doesn't return scalar" }, > { "test_global_func8.o" }, > + { "test_global_func9.o" }, > + { "test_global_func10.o", "invalid indirect read from stack off -8+4 size 8" }, > + { "test_global_func11.o", "Caller passes invalid args into func#1" }, > + { "test_global_func12.o", "invalid mem access 'mem_or_null'" }, > + { "test_global_func13.o", "Caller passes invalid args into func#1" }, > }; > libbpf_print_fn_t old_print_fn = NULL; > int err, i, duration = 0; > diff --git a/tools/testing/selftests/bpf/progs/test_global_func10.c b/tools/testing/selftests/bpf/progs/test_global_func10.c > new file mode 100644 > index 000000000000..61c2ae92ce41 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_global_func10.c > @@ -0,0 +1,29 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include <stddef.h> > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > + > +struct Small { > + int x; > +}; > + > +struct Big { > + int x; > + int y; > +}; > + > +__noinline int foo(const struct Big *big) > +{ > + if (big == 0) > + return 0; > + > + return bpf_get_prandom_u32() < big->y; > +} > + > +SEC("cgroup_skb/ingress") > +int test_cls(struct __sk_buff *skb) > +{ > + const struct Small small = {.x = skb->len }; > + > + return foo((struct Big *)&small) ? 1 : 0; > +} > diff --git a/tools/testing/selftests/bpf/progs/test_global_func11.c b/tools/testing/selftests/bpf/progs/test_global_func11.c > new file mode 100644 > index 000000000000..28488047c849 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_global_func11.c > @@ -0,0 +1,19 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include <stddef.h> > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > + > +struct S { > + int x; > +}; > + > +__noinline int foo(const struct S *s) > +{ > + return s ? bpf_get_prandom_u32() < s->x : 0; > +} > + > +SEC("cgroup_skb/ingress") > +int test_cls(struct __sk_buff *skb) > +{ > + return foo(skb); > +} > diff --git a/tools/testing/selftests/bpf/progs/test_global_func12.c b/tools/testing/selftests/bpf/progs/test_global_func12.c > new file mode 100644 > index 000000000000..62343527cc59 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_global_func12.c > @@ -0,0 +1,21 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#include <stddef.h> > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > + > +struct S { > + int x; > +}; > + > +__noinline int foo(const struct S *s) > +{ > + return bpf_get_prandom_u32() < s->x; > +} > + > +SEC("cgroup_skb/ingress") > +int test_cls(struct __sk_buff *skb) > +{ > + const struct S s = {.x = skb->len }; > + > + return foo(&s); > +} I assume struct member write is also supported? In the next revision, could you also test case something like struct S { int x; int y; }; __noinline int foo(struct S *s) { s->x = 1; return s->y; }; SEC(...) int test(struct __sk_buff *skb) { struct S s = {.y = skb->len}; int ret = foo(&s); return ret < 100 ? 0 : s.x; } In the above, to facilitate implementation, if initializing s.x is desirable, we can do SEC(...) int test(struct __sk_buff *skb) { struct S s = {.x = 0, .y = skb->len}; int ret = foo(&s); return ret < 100 ? 0 : s.x; } > diff --git a/tools/testing/selftests/bpf/progs/test_global_func13.c b/tools/testing/selftests/bpf/progs/test_global_func13.c > new file mode 100644 > index 000000000000..ff8897c1ac22 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/test_global_func13.c [...]
diff --git a/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c b/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c index 32e4348b714b..c4895e6c83c2 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c +++ b/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c @@ -61,6 +61,11 @@ void test_test_global_funcs(void) { "test_global_func6.o" , "modified ctx ptr R2" }, { "test_global_func7.o" , "foo() doesn't return scalar" }, { "test_global_func8.o" }, + { "test_global_func9.o" }, + { "test_global_func10.o", "invalid indirect read from stack off -8+4 size 8" }, + { "test_global_func11.o", "Caller passes invalid args into func#1" }, + { "test_global_func12.o", "invalid mem access 'mem_or_null'" }, + { "test_global_func13.o", "Caller passes invalid args into func#1" }, }; libbpf_print_fn_t old_print_fn = NULL; int err, i, duration = 0; diff --git a/tools/testing/selftests/bpf/progs/test_global_func10.c b/tools/testing/selftests/bpf/progs/test_global_func10.c new file mode 100644 index 000000000000..61c2ae92ce41 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_global_func10.c @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stddef.h> +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +struct Small { + int x; +}; + +struct Big { + int x; + int y; +}; + +__noinline int foo(const struct Big *big) +{ + if (big == 0) + return 0; + + return bpf_get_prandom_u32() < big->y; +} + +SEC("cgroup_skb/ingress") +int test_cls(struct __sk_buff *skb) +{ + const struct Small small = {.x = skb->len }; + + return foo((struct Big *)&small) ? 1 : 0; +} diff --git a/tools/testing/selftests/bpf/progs/test_global_func11.c b/tools/testing/selftests/bpf/progs/test_global_func11.c new file mode 100644 index 000000000000..28488047c849 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_global_func11.c @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stddef.h> +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +struct S { + int x; +}; + +__noinline int foo(const struct S *s) +{ + return s ? bpf_get_prandom_u32() < s->x : 0; +} + +SEC("cgroup_skb/ingress") +int test_cls(struct __sk_buff *skb) +{ + return foo(skb); +} diff --git a/tools/testing/selftests/bpf/progs/test_global_func12.c b/tools/testing/selftests/bpf/progs/test_global_func12.c new file mode 100644 index 000000000000..62343527cc59 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_global_func12.c @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stddef.h> +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +struct S { + int x; +}; + +__noinline int foo(const struct S *s) +{ + return bpf_get_prandom_u32() < s->x; +} + +SEC("cgroup_skb/ingress") +int test_cls(struct __sk_buff *skb) +{ + const struct S s = {.x = skb->len }; + + return foo(&s); +} diff --git a/tools/testing/selftests/bpf/progs/test_global_func13.c b/tools/testing/selftests/bpf/progs/test_global_func13.c new file mode 100644 index 000000000000..ff8897c1ac22 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_global_func13.c @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stddef.h> +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +struct S { + int x; +}; + +__noinline int foo(const struct S *s) +{ + if (s) + return bpf_get_prandom_u32() < s->x; + + return 0; +} + +SEC("cgroup_skb/ingress") +int test_cls(struct __sk_buff *skb) +{ + const struct S *s = (const struct S *)(0xbedabeda); + + return foo(s); +} diff --git a/tools/testing/selftests/bpf/progs/test_global_func9.c b/tools/testing/selftests/bpf/progs/test_global_func9.c new file mode 100644 index 000000000000..17217d0fcd81 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_global_func9.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0-only +#include <stddef.h> +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +struct S { + int x; +}; + +struct C { + int x; + int y; +}; + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, struct S); +} map SEC(".maps"); + +__noinline int foo(const struct S *s) +{ + if (s) + return bpf_get_prandom_u32() < s->x; + + return 0; +} + +SEC("cgroup_skb/ingress") +int test_cls(struct __sk_buff *skb) +{ + int result = 0; + + { + const struct S s = {.x = skb->len }; + + result |= foo(&s); + } + + { + const __u32 key = 1; + const struct S *s = bpf_map_lookup_elem(&map, &key); + + result |= foo(s); + } + + { + const struct C c = {.x = skb->len, .y = skb->family }; + + result |= foo((const struct S *)&c); + } + + { + result |= foo(NULL); + } + + return result ? 1 : 0; +}
test_global_func9 - check valid scenarios for struct pointers test_global_func10 - check that the smaller struct cannot be passed as a the larger one test_global_func11 - check that CTX pointer cannot be passed as a struct pointer test_global_func12 - check access to a null pointer test_global_func13 - check access to an arbitrary pointer value Signed-off-by: Dmitrii Banshchikov <me@ubique.spb.ru> --- .../bpf/prog_tests/test_global_funcs.c | 5 ++ .../selftests/bpf/progs/test_global_func10.c | 29 +++++++++ .../selftests/bpf/progs/test_global_func11.c | 19 ++++++ .../selftests/bpf/progs/test_global_func12.c | 21 +++++++ .../selftests/bpf/progs/test_global_func13.c | 24 ++++++++ .../selftests/bpf/progs/test_global_func9.c | 59 +++++++++++++++++++ 6 files changed, 157 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/test_global_func10.c create mode 100644 tools/testing/selftests/bpf/progs/test_global_func11.c create mode 100644 tools/testing/selftests/bpf/progs/test_global_func12.c create mode 100644 tools/testing/selftests/bpf/progs/test_global_func13.c create mode 100644 tools/testing/selftests/bpf/progs/test_global_func9.c