| Message ID | 20220416063429.3314021-1-joannelkoong@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Dynamic pointers | expand |
On Sat, Apr 16, 2022 at 12:04:22PM IST, Joanne Koong wrote: > This patchset implements the basics of dynamic pointers in bpf. > > A dynamic pointer (struct bpf_dynptr) is a pointer that stores extra metadata > alongside the address it points to. This abstraction is useful in bpf, given > that every memory access in a bpf program must be safe. The verifier and bpf > helper functions can use the metadata to enforce safety guarantees for things > such as dynamically sized strings and kernel heap allocations. > > From the program side, the bpf_dynptr is an opaque struct and the verifier > will enforce that its contents are never written to by the program. > It can only be written to through specific bpf helper functions. > > There are several uses cases for dynamic pointers in bpf programs. A list of > some are: dynamically sized ringbuf reservations without any extra memcpys, > dynamic string parsing and memory comparisons, dynamic memory allocations that > can be persisted in a map, and dynamic parsing of sk_buff and xdp_md packet > data. > > At a high-level, the patches are as follows: > 1/7 - Adds MEM_UNINIT as a bpf_type_flag > 2/7 - Adds MEM_RELEASE as a bpf_type_flag > 3/7 - Adds bpf_dynptr_from_mem, bpf_dynptr_alloc, and bpf_dynptr_put > 4/7 - Adds bpf_dynptr_read and bpf_dynptr_write > 5/7 - Adds dynptr data slices (ptr to underlying dynptr memory) > 6/7 - Adds dynptr support for ring buffers > 7/7 - Tests to check that verifier rejects certain fail cases and passes > certain success cases > > This is the first dynptr patchset in a larger series. The next series of > patches will add persisting dynamic memory allocations in maps, parsing packet > data through dynptrs, dynptrs to referenced objects, convenience helpers for > using dynptrs as iterators, and more helper functions for interacting with > strings and memory dynamically. > test_verifier has 5 failed tests, the following diff fixes them (three for changed verifier error string, and two because we missed to do offset checks for ARG_PTR_TO_ALLOC_MEM in check_func_arg_reg_off). Since this is all, I guess you can wait for the review to complete for this version before respinning. > Changelog: > ---------- > v1 -> v2: > v1: https://lore.kernel.org/bpf/20220402015826.3941317-1-joannekoong@fb.com/ > > 1/7 - > * Remove ARG_PTR_TO_MAP_VALUE_UNINIT alias and use > ARG_PTR_TO_MAP_VALUE | MEM_UNINIT directly (Andrii) > * Drop arg_type_is_mem_ptr() wrapper function (Andrii) > > 2/7 - > * Change name from MEM_RELEASE to OBJ_RELEASE (Andrii) > * Use meta.release_ref instead of ref_obj_id != 0 to determine whether > to release reference (Kumar) > * Drop type_is_release_mem() wrapper function (Andrii) > > 3/7 - > * Add checks for nested dynptrs edge-cases, which could lead to corrupt > * writes of the dynptr stack variable. > * Add u64 flags to bpf_dynptr_from_mem() and bpf_dynptr_alloc() (Andrii) > * Rename from bpf_malloc/bpf_free to bpf_dynptr_alloc/bpf_dynptr_put > (Alexei) > * Support alloc flag __GFP_ZERO (Andrii) > * Reserve upper 8 bits in dynptr size and offset fields instead of > reserving just the upper 4 bits (Andrii) > * Allow dynptr zero-slices (Andrii) > * Use the highest bit for is_rdonly instead of the 28th bit (Andrii) > * Rename check_* functions to is_* functions for better readability > (Andrii) > * Add comment for code that checks the spi bounds (Andrii) > > 4/7 - > * Fix doc description for bpf_dynpt_read (Toke) > * Move bpf_dynptr_check_off_len() from function patch 1 to here (Andrii) > > 5/7 - > * When finding the id for the dynptr to associate the data slice with, > look for dynptr arg instead of assuming it is BPF_REG_1. > > 6/7 - > * Add __force when casting from unsigned long to void * (kernel test robot) > * Expand on docs for ringbuf dynptr APIs (Andrii) > > 7/7 - > * Use table approach for defining test programs and error messages (Andrii) > * Print out full log if there’s an error (Andrii) > * Use bpf_object__find_program_by_name() instead of specifying > program name as a string (Andrii) > * Add 6 extra cases: invalid_nested_dynptrs1, invalid_nested_dynptrs2, > invalid_ref_mem1, invalid_ref_mem2, zero_slice_access, > and test_alloc_zero_bytes > * Add checking for edge cases (eg allocing with invalid flags) > > Joanne Koong (7): > bpf: Add MEM_UNINIT as a bpf_type_flag > bpf: Add OBJ_RELEASE as a bpf_type_flag > bpf: Add bpf_dynptr_from_mem, bpf_dynptr_alloc, bpf_dynptr_put > bpf: Add bpf_dynptr_read and bpf_dynptr_write > bpf: Add dynptr data slices > bpf: Dynptr support for ring buffers > bpf: Dynptr tests > > include/linux/bpf.h | 109 ++- > include/linux/bpf_verifier.h | 33 +- > include/uapi/linux/bpf.h | 110 +++ > kernel/bpf/bpf_lsm.c | 4 +- > kernel/bpf/btf.c | 3 +- > kernel/bpf/cgroup.c | 4 +- > kernel/bpf/helpers.c | 212 +++++- > kernel/bpf/ringbuf.c | 75 +- > kernel/bpf/stackmap.c | 6 +- > kernel/bpf/verifier.c | 538 +++++++++++++-- > kernel/trace/bpf_trace.c | 30 +- > net/core/filter.c | 28 +- > scripts/bpf_doc.py | 2 + > tools/include/uapi/linux/bpf.h | 110 +++ > .../testing/selftests/bpf/prog_tests/dynptr.c | 138 ++++ > .../testing/selftests/bpf/progs/dynptr_fail.c | 643 ++++++++++++++++++ > .../selftests/bpf/progs/dynptr_success.c | 217 ++++++ > 17 files changed, 2148 insertions(+), 114 deletions(-) > create mode 100644 tools/testing/selftests/bpf/prog_tests/dynptr.c > create mode 100644 tools/testing/selftests/bpf/progs/dynptr_fail.c > create mode 100644 tools/testing/selftests/bpf/progs/dynptr_success.c > > -- > 2.30.2 > -- Kartikeya
On Sat, Apr 16, 2022 at 01:43:41PM IST, Kumar Kartikeya Dwivedi wrote: > On Sat, Apr 16, 2022 at 12:04:22PM IST, Joanne Koong wrote: > > This patchset implements the basics of dynamic pointers in bpf. > > > > A dynamic pointer (struct bpf_dynptr) is a pointer that stores extra metadata > > alongside the address it points to. This abstraction is useful in bpf, given > > that every memory access in a bpf program must be safe. The verifier and bpf > > helper functions can use the metadata to enforce safety guarantees for things > > such as dynamically sized strings and kernel heap allocations. > > > > From the program side, the bpf_dynptr is an opaque struct and the verifier > > will enforce that its contents are never written to by the program. > > It can only be written to through specific bpf helper functions. > > > > There are several uses cases for dynamic pointers in bpf programs. A list of > > some are: dynamically sized ringbuf reservations without any extra memcpys, > > dynamic string parsing and memory comparisons, dynamic memory allocations that > > can be persisted in a map, and dynamic parsing of sk_buff and xdp_md packet > > data. > > > > At a high-level, the patches are as follows: > > 1/7 - Adds MEM_UNINIT as a bpf_type_flag > > 2/7 - Adds MEM_RELEASE as a bpf_type_flag > > 3/7 - Adds bpf_dynptr_from_mem, bpf_dynptr_alloc, and bpf_dynptr_put > > 4/7 - Adds bpf_dynptr_read and bpf_dynptr_write > > 5/7 - Adds dynptr data slices (ptr to underlying dynptr memory) > > 6/7 - Adds dynptr support for ring buffers > > 7/7 - Tests to check that verifier rejects certain fail cases and passes > > certain success cases > > > > This is the first dynptr patchset in a larger series. The next series of > > patches will add persisting dynamic memory allocations in maps, parsing packet > > data through dynptrs, dynptrs to referenced objects, convenience helpers for > > using dynptrs as iterators, and more helper functions for interacting with > > strings and memory dynamically. > > > > test_verifier has 5 failed tests, the following diff fixes them (three for > changed verifier error string, and two because we missed to do offset checks for > ARG_PTR_TO_ALLOC_MEM in check_func_arg_reg_off). Since this is all, I guess you > can wait for the review to complete for this version before respinning. > Ugh, hit send too early. diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index bf64946ced84..24e5d494d991 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5681,7 +5681,7 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, /* Some of the argument types nevertheless require a * zero register offset. */ - if (arg_type != ARG_PTR_TO_ALLOC_MEM) + if (base_type(arg_type) != ARG_PTR_TO_ALLOC_MEM) return 0; break; /* All the rest must be rejected, except PTR_TO_BTF_ID which allows diff --git a/tools/testing/selftests/bpf/verifier/ref_tracking.c b/tools/testing/selftests/bpf/verifier/ref_tracking.c index fbd682520e47..f1ad3b3cc145 100644 --- a/tools/testing/selftests/bpf/verifier/ref_tracking.c +++ b/tools/testing/selftests/bpf/verifier/ref_tracking.c @@ -796,7 +796,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "reference has not been acquired before", + .errstr = "arg 1 is an unacquired reference", }, { /* !bpf_sk_fullsock(sk) is checked but !bpf_tcp_sock(sk) is not checked */ diff --git a/tools/testing/selftests/bpf/verifier/sock.c b/tools/testing/selftests/bpf/verifier/sock.c index 86b24cad27a7..055a61205906 100644 --- a/tools/testing/selftests/bpf/verifier/sock.c +++ b/tools/testing/selftests/bpf/verifier/sock.c @@ -417,7 +417,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "reference has not been acquired before", + .errstr = "arg 1 is an unacquired reference", }, { "bpf_sk_release(bpf_sk_fullsock(skb->sk))", @@ -436,7 +436,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "reference has not been acquired before", + .errstr = "arg 1 is an unacquired reference", }, { "bpf_sk_release(bpf_tcp_sock(skb->sk))", @@ -455,7 +455,7 @@ }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .result = REJECT, - .errstr = "reference has not been acquired before", + .errstr = "arg 1 is an unacquired reference", }, { "sk_storage_get(map, skb->sk, NULL, 0): value == NULL", > [...] -- Kartikeya
On Sat, Apr 16, 2022 at 1:19 AM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > On Sat, Apr 16, 2022 at 01:43:41PM IST, Kumar Kartikeya Dwivedi wrote: > > On Sat, Apr 16, 2022 at 12:04:22PM IST, Joanne Koong wrote: > > > This patchset implements the basics of dynamic pointers in bpf. > > > > > > A dynamic pointer (struct bpf_dynptr) is a pointer that stores extra metadata > > > alongside the address it points to. This abstraction is useful in bpf, given > > > that every memory access in a bpf program must be safe. The verifier and bpf > > > helper functions can use the metadata to enforce safety guarantees for things > > > such as dynamically sized strings and kernel heap allocations. > > > > > > From the program side, the bpf_dynptr is an opaque struct and the verifier > > > will enforce that its contents are never written to by the program. > > > It can only be written to through specific bpf helper functions. > > > > > > There are several uses cases for dynamic pointers in bpf programs. A list of > > > some are: dynamically sized ringbuf reservations without any extra memcpys, > > > dynamic string parsing and memory comparisons, dynamic memory allocations that > > > can be persisted in a map, and dynamic parsing of sk_buff and xdp_md packet > > > data. > > > > > > At a high-level, the patches are as follows: > > > 1/7 - Adds MEM_UNINIT as a bpf_type_flag > > > 2/7 - Adds MEM_RELEASE as a bpf_type_flag > > > 3/7 - Adds bpf_dynptr_from_mem, bpf_dynptr_alloc, and bpf_dynptr_put > > > 4/7 - Adds bpf_dynptr_read and bpf_dynptr_write > > > 5/7 - Adds dynptr data slices (ptr to underlying dynptr memory) > > > 6/7 - Adds dynptr support for ring buffers > > > 7/7 - Tests to check that verifier rejects certain fail cases and passes > > > certain success cases > > > > > > This is the first dynptr patchset in a larger series. The next series of > > > patches will add persisting dynamic memory allocations in maps, parsing packet > > > data through dynptrs, dynptrs to referenced objects, convenience helpers for > > > using dynptrs as iterators, and more helper functions for interacting with > > > strings and memory dynamically. > > > > > > > test_verifier has 5 failed tests, the following diff fixes them (three for > > changed verifier error string, and two because we missed to do offset checks for > > ARG_PTR_TO_ALLOC_MEM in check_func_arg_reg_off). Since this is all, I guess you > > can wait for the review to complete for this version before respinning. > > > > Ugh, hit send too early. > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index bf64946ced84..24e5d494d991 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -5681,7 +5681,7 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, > /* Some of the argument types nevertheless require a > * zero register offset. > */ > - if (arg_type != ARG_PTR_TO_ALLOC_MEM) > + if (base_type(arg_type) != ARG_PTR_TO_ALLOC_MEM) > return 0; > break; > /* All the rest must be rejected, except PTR_TO_BTF_ID which allows > diff --git a/tools/testing/selftests/bpf/verifier/ref_tracking.c b/tools/testing/selftests/bpf/verifier/ref_tracking.c > index fbd682520e47..f1ad3b3cc145 100644 > --- a/tools/testing/selftests/bpf/verifier/ref_tracking.c > +++ b/tools/testing/selftests/bpf/verifier/ref_tracking.c > @@ -796,7 +796,7 @@ > }, > .prog_type = BPF_PROG_TYPE_SCHED_CLS, > .result = REJECT, > - .errstr = "reference has not been acquired before", > + .errstr = "arg 1 is an unacquired reference", > }, > { > /* !bpf_sk_fullsock(sk) is checked but !bpf_tcp_sock(sk) is not checked */ > diff --git a/tools/testing/selftests/bpf/verifier/sock.c b/tools/testing/selftests/bpf/verifier/sock.c > index 86b24cad27a7..055a61205906 100644 > --- a/tools/testing/selftests/bpf/verifier/sock.c > +++ b/tools/testing/selftests/bpf/verifier/sock.c > @@ -417,7 +417,7 @@ > }, > .prog_type = BPF_PROG_TYPE_SCHED_CLS, > .result = REJECT, > - .errstr = "reference has not been acquired before", > + .errstr = "arg 1 is an unacquired reference", > }, > { > "bpf_sk_release(bpf_sk_fullsock(skb->sk))", > @@ -436,7 +436,7 @@ > }, > .prog_type = BPF_PROG_TYPE_SCHED_CLS, > .result = REJECT, > - .errstr = "reference has not been acquired before", > + .errstr = "arg 1 is an unacquired reference", > }, > { > "bpf_sk_release(bpf_tcp_sock(skb->sk))", > @@ -455,7 +455,7 @@ > }, > .prog_type = BPF_PROG_TYPE_SCHED_CLS, > .result = REJECT, > - .errstr = "reference has not been acquired before", > + .errstr = "arg 1 is an unacquired reference", > }, > { > "sk_storage_get(map, skb->sk, NULL, 0): value == NULL", > > > [...] Awesome, thanks for noting this Kumar! I'll make sure to locally run the verifier tests before I submit the next iteration of it upstream > > -- > Kartikeya
This patchset implements the basics of dynamic pointers in bpf. A dynamic pointer (struct bpf_dynptr) is a pointer that stores extra metadata alongside the address it points to. This abstraction is useful in bpf, given that every memory access in a bpf program must be safe. The verifier and bpf helper functions can use the metadata to enforce safety guarantees for things such as dynamically sized strings and kernel heap allocations. From the program side, the bpf_dynptr is an opaque struct and the verifier will enforce that its contents are never written to by the program. It can only be written to through specific bpf helper functions. There are several uses cases for dynamic pointers in bpf programs. A list of some are: dynamically sized ringbuf reservations without any extra memcpys, dynamic string parsing and memory comparisons, dynamic memory allocations that can be persisted in a map, and dynamic parsing of sk_buff and xdp_md packet data. At a high-level, the patches are as follows: 1/7 - Adds MEM_UNINIT as a bpf_type_flag 2/7 - Adds MEM_RELEASE as a bpf_type_flag 3/7 - Adds bpf_dynptr_from_mem, bpf_dynptr_alloc, and bpf_dynptr_put 4/7 - Adds bpf_dynptr_read and bpf_dynptr_write 5/7 - Adds dynptr data slices (ptr to underlying dynptr memory) 6/7 - Adds dynptr support for ring buffers 7/7 - Tests to check that verifier rejects certain fail cases and passes certain success cases This is the first dynptr patchset in a larger series. The next series of patches will add persisting dynamic memory allocations in maps, parsing packet data through dynptrs, dynptrs to referenced objects, convenience helpers for using dynptrs as iterators, and more helper functions for interacting with strings and memory dynamically. Changelog: ---------- v1 -> v2: v1: https://lore.kernel.org/bpf/20220402015826.3941317-1-joannekoong@fb.com/ 1/7 - * Remove ARG_PTR_TO_MAP_VALUE_UNINIT alias and use ARG_PTR_TO_MAP_VALUE | MEM_UNINIT directly (Andrii) * Drop arg_type_is_mem_ptr() wrapper function (Andrii) 2/7 - * Change name from MEM_RELEASE to OBJ_RELEASE (Andrii) * Use meta.release_ref instead of ref_obj_id != 0 to determine whether to release reference (Kumar) * Drop type_is_release_mem() wrapper function (Andrii) 3/7 - * Add checks for nested dynptrs edge-cases, which could lead to corrupt * writes of the dynptr stack variable. * Add u64 flags to bpf_dynptr_from_mem() and bpf_dynptr_alloc() (Andrii) * Rename from bpf_malloc/bpf_free to bpf_dynptr_alloc/bpf_dynptr_put (Alexei) * Support alloc flag __GFP_ZERO (Andrii) * Reserve upper 8 bits in dynptr size and offset fields instead of reserving just the upper 4 bits (Andrii) * Allow dynptr zero-slices (Andrii) * Use the highest bit for is_rdonly instead of the 28th bit (Andrii) * Rename check_* functions to is_* functions for better readability (Andrii) * Add comment for code that checks the spi bounds (Andrii) 4/7 - * Fix doc description for bpf_dynpt_read (Toke) * Move bpf_dynptr_check_off_len() from function patch 1 to here (Andrii) 5/7 - * When finding the id for the dynptr to associate the data slice with, look for dynptr arg instead of assuming it is BPF_REG_1. 6/7 - * Add __force when casting from unsigned long to void * (kernel test robot) * Expand on docs for ringbuf dynptr APIs (Andrii) 7/7 - * Use table approach for defining test programs and error messages (Andrii) * Print out full log if there’s an error (Andrii) * Use bpf_object__find_program_by_name() instead of specifying program name as a string (Andrii) * Add 6 extra cases: invalid_nested_dynptrs1, invalid_nested_dynptrs2, invalid_ref_mem1, invalid_ref_mem2, zero_slice_access, and test_alloc_zero_bytes * Add checking for edge cases (eg allocing with invalid flags) Joanne Koong (7): bpf: Add MEM_UNINIT as a bpf_type_flag bpf: Add OBJ_RELEASE as a bpf_type_flag bpf: Add bpf_dynptr_from_mem, bpf_dynptr_alloc, bpf_dynptr_put bpf: Add bpf_dynptr_read and bpf_dynptr_write bpf: Add dynptr data slices bpf: Dynptr support for ring buffers bpf: Dynptr tests include/linux/bpf.h | 109 ++- include/linux/bpf_verifier.h | 33 +- include/uapi/linux/bpf.h | 110 +++ kernel/bpf/bpf_lsm.c | 4 +- kernel/bpf/btf.c | 3 +- kernel/bpf/cgroup.c | 4 +- kernel/bpf/helpers.c | 212 +++++- kernel/bpf/ringbuf.c | 75 +- kernel/bpf/stackmap.c | 6 +- kernel/bpf/verifier.c | 538 +++++++++++++-- kernel/trace/bpf_trace.c | 30 +- net/core/filter.c | 28 +- scripts/bpf_doc.py | 2 + tools/include/uapi/linux/bpf.h | 110 +++ .../testing/selftests/bpf/prog_tests/dynptr.c | 138 ++++ .../testing/selftests/bpf/progs/dynptr_fail.c | 643 ++++++++++++++++++ .../selftests/bpf/progs/dynptr_success.c | 217 ++++++ 17 files changed, 2148 insertions(+), 114 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/dynptr.c create mode 100644 tools/testing/selftests/bpf/progs/dynptr_fail.c create mode 100644 tools/testing/selftests/bpf/progs/dynptr_success.c