| Message ID | 20221107230950.7117-6-memxor@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Local kptrs, BPF linked lists | expand |
On Mon, Nov 7, 2022 at 3:10 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > Currently, verifier uses MEM_ALLOC type tag to specially tag memory > returned from bpf_ringbuf_reserve helper. However, this is currently > only used for this purpose and there is an implicit assumption that it > only refers to ringbuf memory (e.g. the check for ARG_PTR_TO_ALLOC_MEM > in check_func_arg_reg_off). > > Hence, rename MEM_ALLOC to MEM_RINGBUF to indicate this special > relationship and instead open the use of MEM_ALLOC for more generic > allocations made for user types. > > Also, since ARG_PTR_TO_ALLOC_MEM_OR_NULL is unused, simply drop it. > > Finally, update selftests using 'alloc_' verifier string to 'ringbuf_'. > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > --- Ok, so you are doing what I asked in the previous patch, nice :) > include/linux/bpf.h | 11 ++++------- > kernel/bpf/ringbuf.c | 6 +++--- > kernel/bpf/verifier.c | 14 +++++++------- > tools/testing/selftests/bpf/prog_tests/dynptr.c | 2 +- > tools/testing/selftests/bpf/verifier/ringbuf.c | 2 +- > tools/testing/selftests/bpf/verifier/spill_fill.c | 2 +- > 6 files changed, 17 insertions(+), 20 deletions(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 2fe3ec620d54..afc1c51b59ff 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -488,10 +488,8 @@ enum bpf_type_flag { > */ > MEM_RDONLY = BIT(1 + BPF_BASE_TYPE_BITS), > > - /* MEM was "allocated" from a different helper, and cannot be mixed > - * with regular non-MEM_ALLOC'ed MEM types. > - */ > - MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS), > + /* MEM points to BPF ring buffer reservation. */ > + MEM_RINGBUF = BIT(2 + BPF_BASE_TYPE_BITS), What do we gain by having ringbuf memory as additional modified flag instead of its own type like PTR_TO_MAP_VALUE or PTR_TO_PACKET? It feels like here separate register type is more justified and is less error prone overall. > > /* MEM is in user address space. */ > MEM_USER = BIT(3 + BPF_BASE_TYPE_BITS), > @@ -565,7 +563,7 @@ enum bpf_arg_type { > ARG_PTR_TO_LONG, /* pointer to long */ > ARG_PTR_TO_SOCKET, /* pointer to bpf_sock (fullsock) */ > ARG_PTR_TO_BTF_ID, /* pointer to in-kernel struct */ > - ARG_PTR_TO_ALLOC_MEM, /* pointer to dynamically allocated memory */ > + ARG_PTR_TO_RINGBUF_MEM, /* pointer to dynamically reserved ringbuf memory */ > ARG_CONST_ALLOC_SIZE_OR_ZERO, /* number of allocated bytes requested */ > ARG_PTR_TO_BTF_ID_SOCK_COMMON, /* pointer to in-kernel sock_common or bpf-mirrored bpf_sock */ > ARG_PTR_TO_PERCPU_BTF_ID, /* pointer to in-kernel percpu type */ [...]
On Wed, Nov 09, 2022 at 04:44:16AM IST, Andrii Nakryiko wrote: > On Mon, Nov 7, 2022 at 3:10 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > > > Currently, verifier uses MEM_ALLOC type tag to specially tag memory > > returned from bpf_ringbuf_reserve helper. However, this is currently > > only used for this purpose and there is an implicit assumption that it > > only refers to ringbuf memory (e.g. the check for ARG_PTR_TO_ALLOC_MEM > > in check_func_arg_reg_off). > > > > Hence, rename MEM_ALLOC to MEM_RINGBUF to indicate this special > > relationship and instead open the use of MEM_ALLOC for more generic > > allocations made for user types. > > > > Also, since ARG_PTR_TO_ALLOC_MEM_OR_NULL is unused, simply drop it. > > > > Finally, update selftests using 'alloc_' verifier string to 'ringbuf_'. > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > --- > > Ok, so you are doing what I asked in the previous patch, nice :) > > > include/linux/bpf.h | 11 ++++------- > > kernel/bpf/ringbuf.c | 6 +++--- > > kernel/bpf/verifier.c | 14 +++++++------- > > tools/testing/selftests/bpf/prog_tests/dynptr.c | 2 +- > > tools/testing/selftests/bpf/verifier/ringbuf.c | 2 +- > > tools/testing/selftests/bpf/verifier/spill_fill.c | 2 +- > > 6 files changed, 17 insertions(+), 20 deletions(-) > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > index 2fe3ec620d54..afc1c51b59ff 100644 > > --- a/include/linux/bpf.h > > +++ b/include/linux/bpf.h > > @@ -488,10 +488,8 @@ enum bpf_type_flag { > > */ > > MEM_RDONLY = BIT(1 + BPF_BASE_TYPE_BITS), > > > > - /* MEM was "allocated" from a different helper, and cannot be mixed > > - * with regular non-MEM_ALLOC'ed MEM types. > > - */ > > - MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS), > > + /* MEM points to BPF ring buffer reservation. */ > > + MEM_RINGBUF = BIT(2 + BPF_BASE_TYPE_BITS), > > What do we gain by having ringbuf memory as additional modified flag > instead of its own type like PTR_TO_MAP_VALUE or PTR_TO_PACKET? It > feels like here separate register type is more justified and is less > error prone overall. > I'm not sure it's all that different. It only matters when checking argument during release. We want to ensure it came from ringbuf_reserve. That's all, apart from that it's no different from PTR_TO_MEM. In all other places it's folded and code for PTR_TO_MEM is used. Same idea as PTR_TO_BTF_ID | MEM_ALLOC. But I don't feel too strongly, so if you still think it's better I can make the switch.
On Tue, Nov 8, 2022 at 3:49 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > On Wed, Nov 09, 2022 at 04:44:16AM IST, Andrii Nakryiko wrote: > > On Mon, Nov 7, 2022 at 3:10 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > > > > > Currently, verifier uses MEM_ALLOC type tag to specially tag memory > > > returned from bpf_ringbuf_reserve helper. However, this is currently > > > only used for this purpose and there is an implicit assumption that it > > > only refers to ringbuf memory (e.g. the check for ARG_PTR_TO_ALLOC_MEM > > > in check_func_arg_reg_off). > > > > > > Hence, rename MEM_ALLOC to MEM_RINGBUF to indicate this special > > > relationship and instead open the use of MEM_ALLOC for more generic > > > allocations made for user types. > > > > > > Also, since ARG_PTR_TO_ALLOC_MEM_OR_NULL is unused, simply drop it. > > > > > > Finally, update selftests using 'alloc_' verifier string to 'ringbuf_'. > > > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > > --- > > > > Ok, so you are doing what I asked in the previous patch, nice :) > > > > > include/linux/bpf.h | 11 ++++------- > > > kernel/bpf/ringbuf.c | 6 +++--- > > > kernel/bpf/verifier.c | 14 +++++++------- > > > tools/testing/selftests/bpf/prog_tests/dynptr.c | 2 +- > > > tools/testing/selftests/bpf/verifier/ringbuf.c | 2 +- > > > tools/testing/selftests/bpf/verifier/spill_fill.c | 2 +- > > > 6 files changed, 17 insertions(+), 20 deletions(-) > > > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > > index 2fe3ec620d54..afc1c51b59ff 100644 > > > --- a/include/linux/bpf.h > > > +++ b/include/linux/bpf.h > > > @@ -488,10 +488,8 @@ enum bpf_type_flag { > > > */ > > > MEM_RDONLY = BIT(1 + BPF_BASE_TYPE_BITS), > > > > > > - /* MEM was "allocated" from a different helper, and cannot be mixed > > > - * with regular non-MEM_ALLOC'ed MEM types. > > > - */ > > > - MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS), > > > + /* MEM points to BPF ring buffer reservation. */ > > > + MEM_RINGBUF = BIT(2 + BPF_BASE_TYPE_BITS), > > > > What do we gain by having ringbuf memory as additional modified flag > > instead of its own type like PTR_TO_MAP_VALUE or PTR_TO_PACKET? It > > feels like here separate register type is more justified and is less > > error prone overall. > > > > I'm not sure it's all that different. It only matters when checking argument > during release. We want to ensure it came from ringbuf_reserve. That's all, > apart from that it's no different from PTR_TO_MEM. In all other places it's > folded and code for PTR_TO_MEM is used. Same idea as PTR_TO_BTF_ID | MEM_ALLOC. > > But I don't feel too strongly, so if you still think it's better I can make the > switch. Not strongly, but I think having this as a flag is more error prone. For cases where ringbuf memory should be treated just as memory, we should use the same mechanism we have for MAP_VALUE. But I haven't checked how we deal with MAP_VALUE, if that's a special case everywhere, in addition to generic PTR_TO_MEM, then fine. But if having PTR_TO_RINGBUF_MEM is converted to PTR_TO_MEM generically where needed, I'd have dedicated PTR_TO_RINGBUF_MEM.
On Tue, Nov 8, 2022 at 4:26 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Nov 8, 2022 at 3:49 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > > > On Wed, Nov 09, 2022 at 04:44:16AM IST, Andrii Nakryiko wrote: > > > On Mon, Nov 7, 2022 at 3:10 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > > > > > > > Currently, verifier uses MEM_ALLOC type tag to specially tag memory > > > > returned from bpf_ringbuf_reserve helper. However, this is currently > > > > only used for this purpose and there is an implicit assumption that it > > > > only refers to ringbuf memory (e.g. the check for ARG_PTR_TO_ALLOC_MEM > > > > in check_func_arg_reg_off). > > > > > > > > Hence, rename MEM_ALLOC to MEM_RINGBUF to indicate this special > > > > relationship and instead open the use of MEM_ALLOC for more generic > > > > allocations made for user types. > > > > > > > > Also, since ARG_PTR_TO_ALLOC_MEM_OR_NULL is unused, simply drop it. > > > > > > > > Finally, update selftests using 'alloc_' verifier string to 'ringbuf_'. > > > > > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > > > --- > > > > > > Ok, so you are doing what I asked in the previous patch, nice :) > > > > > > > include/linux/bpf.h | 11 ++++------- > > > > kernel/bpf/ringbuf.c | 6 +++--- > > > > kernel/bpf/verifier.c | 14 +++++++------- > > > > tools/testing/selftests/bpf/prog_tests/dynptr.c | 2 +- > > > > tools/testing/selftests/bpf/verifier/ringbuf.c | 2 +- > > > > tools/testing/selftests/bpf/verifier/spill_fill.c | 2 +- > > > > 6 files changed, 17 insertions(+), 20 deletions(-) > > > > > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > > > index 2fe3ec620d54..afc1c51b59ff 100644 > > > > --- a/include/linux/bpf.h > > > > +++ b/include/linux/bpf.h > > > > @@ -488,10 +488,8 @@ enum bpf_type_flag { > > > > */ > > > > MEM_RDONLY = BIT(1 + BPF_BASE_TYPE_BITS), > > > > > > > > - /* MEM was "allocated" from a different helper, and cannot be mixed > > > > - * with regular non-MEM_ALLOC'ed MEM types. > > > > - */ > > > > - MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS), > > > > + /* MEM points to BPF ring buffer reservation. */ > > > > + MEM_RINGBUF = BIT(2 + BPF_BASE_TYPE_BITS), > > > > > > What do we gain by having ringbuf memory as additional modified flag > > > instead of its own type like PTR_TO_MAP_VALUE or PTR_TO_PACKET? It > > > feels like here separate register type is more justified and is less > > > error prone overall. > > > > > > > I'm not sure it's all that different. It only matters when checking argument > > during release. We want to ensure it came from ringbuf_reserve. That's all, > > apart from that it's no different from PTR_TO_MEM. In all other places it's > > folded and code for PTR_TO_MEM is used. Same idea as PTR_TO_BTF_ID | MEM_ALLOC. > > > > But I don't feel too strongly, so if you still think it's better I can make the > > switch. > > Not strongly, but I think having this as a flag is more error prone. > For cases where ringbuf memory should be treated just as memory, we > should use the same mechanism we have for MAP_VALUE. But I haven't > checked how we deal with MAP_VALUE, if that's a special case > everywhere, in addition to generic PTR_TO_MEM, then fine. But if > having PTR_TO_RINGBUF_MEM is converted to PTR_TO_MEM generically where > needed, I'd have dedicated PTR_TO_RINGBUF_MEM. I don't think we can or at least it's not as easy to generalize ringbuf mem as map_value. iirc MEM_ALLOC was there to make sure reserver->commit is the same mem. That's what MEM_RINGBUF will doing after this patch.
On Tue, Nov 8, 2022 at 5:05 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Tue, Nov 8, 2022 at 4:26 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Tue, Nov 8, 2022 at 3:49 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > > > > > On Wed, Nov 09, 2022 at 04:44:16AM IST, Andrii Nakryiko wrote: > > > > On Mon, Nov 7, 2022 at 3:10 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > > > > > > > > > Currently, verifier uses MEM_ALLOC type tag to specially tag memory > > > > > returned from bpf_ringbuf_reserve helper. However, this is currently > > > > > only used for this purpose and there is an implicit assumption that it > > > > > only refers to ringbuf memory (e.g. the check for ARG_PTR_TO_ALLOC_MEM > > > > > in check_func_arg_reg_off). > > > > > > > > > > Hence, rename MEM_ALLOC to MEM_RINGBUF to indicate this special > > > > > relationship and instead open the use of MEM_ALLOC for more generic > > > > > allocations made for user types. > > > > > > > > > > Also, since ARG_PTR_TO_ALLOC_MEM_OR_NULL is unused, simply drop it. > > > > > > > > > > Finally, update selftests using 'alloc_' verifier string to 'ringbuf_'. > > > > > > > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > > > > > --- > > > > > > > > Ok, so you are doing what I asked in the previous patch, nice :) > > > > > > > > > include/linux/bpf.h | 11 ++++------- > > > > > kernel/bpf/ringbuf.c | 6 +++--- > > > > > kernel/bpf/verifier.c | 14 +++++++------- > > > > > tools/testing/selftests/bpf/prog_tests/dynptr.c | 2 +- > > > > > tools/testing/selftests/bpf/verifier/ringbuf.c | 2 +- > > > > > tools/testing/selftests/bpf/verifier/spill_fill.c | 2 +- > > > > > 6 files changed, 17 insertions(+), 20 deletions(-) > > > > > > > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > > > > index 2fe3ec620d54..afc1c51b59ff 100644 > > > > > --- a/include/linux/bpf.h > > > > > +++ b/include/linux/bpf.h > > > > > @@ -488,10 +488,8 @@ enum bpf_type_flag { > > > > > */ > > > > > MEM_RDONLY = BIT(1 + BPF_BASE_TYPE_BITS), > > > > > > > > > > - /* MEM was "allocated" from a different helper, and cannot be mixed > > > > > - * with regular non-MEM_ALLOC'ed MEM types. > > > > > - */ > > > > > - MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS), > > > > > + /* MEM points to BPF ring buffer reservation. */ > > > > > + MEM_RINGBUF = BIT(2 + BPF_BASE_TYPE_BITS), > > > > > > > > What do we gain by having ringbuf memory as additional modified flag > > > > instead of its own type like PTR_TO_MAP_VALUE or PTR_TO_PACKET? It > > > > feels like here separate register type is more justified and is less > > > > error prone overall. > > > > > > > > > > I'm not sure it's all that different. It only matters when checking argument > > > during release. We want to ensure it came from ringbuf_reserve. That's all, > > > apart from that it's no different from PTR_TO_MEM. In all other places it's > > > folded and code for PTR_TO_MEM is used. Same idea as PTR_TO_BTF_ID | MEM_ALLOC. > > > > > > But I don't feel too strongly, so if you still think it's better I can make the > > > switch. > > > > Not strongly, but I think having this as a flag is more error prone. > > For cases where ringbuf memory should be treated just as memory, we > > should use the same mechanism we have for MAP_VALUE. But I haven't > > checked how we deal with MAP_VALUE, if that's a special case > > everywhere, in addition to generic PTR_TO_MEM, then fine. But if > > having PTR_TO_RINGBUF_MEM is converted to PTR_TO_MEM generically where > > needed, I'd have dedicated PTR_TO_RINGBUF_MEM. > > I don't think we can or at least it's not as easy to generalize > ringbuf mem as map_value. > iirc MEM_ALLOC was there to make sure reserver->commit is the same mem. > That's what MEM_RINGBUF will doing after this patch. I'm not sure we are talking about the same thing. My only point was to have RINGBUF_MEM as *base type* instead of as an *add-on flag*. MAP_VALUE was an example of another special register type that is base type but behaves like PTR_TO_MEM for helpers that don't care about where specifically memory is coming from. I didn't mean to unify RINGBUF_MEM and MAP_VALUE in any way (if that's what you are proposing, I'm actually not sure exactly). But as I said, no big deal with a flag, we can always change that in the future as well.
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 2fe3ec620d54..afc1c51b59ff 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -488,10 +488,8 @@ enum bpf_type_flag { */ MEM_RDONLY = BIT(1 + BPF_BASE_TYPE_BITS), - /* MEM was "allocated" from a different helper, and cannot be mixed - * with regular non-MEM_ALLOC'ed MEM types. - */ - MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS), + /* MEM points to BPF ring buffer reservation. */ + MEM_RINGBUF = BIT(2 + BPF_BASE_TYPE_BITS), /* MEM is in user address space. */ MEM_USER = BIT(3 + BPF_BASE_TYPE_BITS), @@ -565,7 +563,7 @@ enum bpf_arg_type { ARG_PTR_TO_LONG, /* pointer to long */ ARG_PTR_TO_SOCKET, /* pointer to bpf_sock (fullsock) */ ARG_PTR_TO_BTF_ID, /* pointer to in-kernel struct */ - ARG_PTR_TO_ALLOC_MEM, /* pointer to dynamically allocated memory */ + ARG_PTR_TO_RINGBUF_MEM, /* pointer to dynamically reserved ringbuf memory */ ARG_CONST_ALLOC_SIZE_OR_ZERO, /* number of allocated bytes requested */ ARG_PTR_TO_BTF_ID_SOCK_COMMON, /* pointer to in-kernel sock_common or bpf-mirrored bpf_sock */ ARG_PTR_TO_PERCPU_BTF_ID, /* pointer to in-kernel percpu type */ @@ -582,7 +580,6 @@ enum bpf_arg_type { ARG_PTR_TO_MEM_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_MEM, ARG_PTR_TO_CTX_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_CTX, ARG_PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_SOCKET, - ARG_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_ALLOC_MEM, ARG_PTR_TO_STACK_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_STACK, ARG_PTR_TO_BTF_ID_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_BTF_ID, /* pointer to memory does not need to be initialized, helper function must fill @@ -617,7 +614,7 @@ enum bpf_return_type { RET_PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCKET, RET_PTR_TO_TCP_SOCK_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_TCP_SOCK, RET_PTR_TO_SOCK_COMMON_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_SOCK_COMMON, - RET_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_ALLOC | RET_PTR_TO_MEM, + RET_PTR_TO_RINGBUF_MEM_OR_NULL = PTR_MAYBE_NULL | MEM_RINGBUF | RET_PTR_TO_MEM, RET_PTR_TO_DYNPTR_MEM_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_MEM, RET_PTR_TO_BTF_ID_OR_NULL = PTR_MAYBE_NULL | RET_PTR_TO_BTF_ID, diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c index 9e832acf4692..80f4b4d88aaf 100644 --- a/kernel/bpf/ringbuf.c +++ b/kernel/bpf/ringbuf.c @@ -447,7 +447,7 @@ BPF_CALL_3(bpf_ringbuf_reserve, struct bpf_map *, map, u64, size, u64, flags) const struct bpf_func_proto bpf_ringbuf_reserve_proto = { .func = bpf_ringbuf_reserve, - .ret_type = RET_PTR_TO_ALLOC_MEM_OR_NULL, + .ret_type = RET_PTR_TO_RINGBUF_MEM_OR_NULL, .arg1_type = ARG_CONST_MAP_PTR, .arg2_type = ARG_CONST_ALLOC_SIZE_OR_ZERO, .arg3_type = ARG_ANYTHING, @@ -490,7 +490,7 @@ BPF_CALL_2(bpf_ringbuf_submit, void *, sample, u64, flags) const struct bpf_func_proto bpf_ringbuf_submit_proto = { .func = bpf_ringbuf_submit, .ret_type = RET_VOID, - .arg1_type = ARG_PTR_TO_ALLOC_MEM | OBJ_RELEASE, + .arg1_type = ARG_PTR_TO_RINGBUF_MEM | OBJ_RELEASE, .arg2_type = ARG_ANYTHING, }; @@ -503,7 +503,7 @@ BPF_CALL_2(bpf_ringbuf_discard, void *, sample, u64, flags) const struct bpf_func_proto bpf_ringbuf_discard_proto = { .func = bpf_ringbuf_discard, .ret_type = RET_VOID, - .arg1_type = ARG_PTR_TO_ALLOC_MEM | OBJ_RELEASE, + .arg1_type = ARG_PTR_TO_RINGBUF_MEM | OBJ_RELEASE, .arg2_type = ARG_ANYTHING, }; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 2407e3b179ec..5fca156eca43 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -577,8 +577,8 @@ static const char *reg_type_str(struct bpf_verifier_env *env, if (type & MEM_RDONLY) strncpy(prefix, "rdonly_", 32); - if (type & MEM_ALLOC) - strncpy(prefix, "alloc_", 32); + if (type & MEM_RINGBUF) + strncpy(prefix, "ringbuf_", 32); if (type & MEM_USER) strncpy(prefix, "user_", 32); if (type & MEM_PERCPU) @@ -5780,7 +5780,7 @@ static const struct bpf_reg_types mem_types = { PTR_TO_MAP_KEY, PTR_TO_MAP_VALUE, PTR_TO_MEM, - PTR_TO_MEM | MEM_ALLOC, + PTR_TO_MEM | MEM_RINGBUF, PTR_TO_BUF, }, }; @@ -5798,7 +5798,7 @@ static const struct bpf_reg_types int_ptr_types = { static const struct bpf_reg_types fullsock_types = { .types = { PTR_TO_SOCKET } }; static const struct bpf_reg_types scalar_types = { .types = { SCALAR_VALUE } }; static const struct bpf_reg_types context_types = { .types = { PTR_TO_CTX } }; -static const struct bpf_reg_types alloc_mem_types = { .types = { PTR_TO_MEM | MEM_ALLOC } }; +static const struct bpf_reg_types ringbuf_mem_types = { .types = { PTR_TO_MEM | MEM_RINGBUF } }; static const struct bpf_reg_types const_map_ptr_types = { .types = { CONST_PTR_TO_MAP } }; static const struct bpf_reg_types btf_ptr_types = { .types = { PTR_TO_BTF_ID } }; static const struct bpf_reg_types spin_lock_types = { .types = { PTR_TO_MAP_VALUE } }; @@ -5831,7 +5831,7 @@ static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = { [ARG_PTR_TO_BTF_ID] = &btf_ptr_types, [ARG_PTR_TO_SPIN_LOCK] = &spin_lock_types, [ARG_PTR_TO_MEM] = &mem_types, - [ARG_PTR_TO_ALLOC_MEM] = &alloc_mem_types, + [ARG_PTR_TO_RINGBUF_MEM] = &ringbuf_mem_types, [ARG_PTR_TO_INT] = &int_ptr_types, [ARG_PTR_TO_LONG] = &int_ptr_types, [ARG_PTR_TO_PERCPU_BTF_ID] = &percpu_btf_ptr_types, @@ -5952,14 +5952,14 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, case PTR_TO_MAP_VALUE: case PTR_TO_MEM: case PTR_TO_MEM | MEM_RDONLY: - case PTR_TO_MEM | MEM_ALLOC: + case PTR_TO_MEM | MEM_RINGBUF: case PTR_TO_BUF: case PTR_TO_BUF | MEM_RDONLY: case SCALAR_VALUE: /* Some of the argument types nevertheless require a * zero register offset. */ - if (base_type(arg_type) != ARG_PTR_TO_ALLOC_MEM) + if (base_type(arg_type) != ARG_PTR_TO_RINGBUF_MEM) return 0; break; /* All the rest must be rejected, except PTR_TO_BTF_ID which allows diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c index 8fc4e6c02bfd..b0c06f821cb8 100644 --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c @@ -17,7 +17,7 @@ static struct { {"ringbuf_missing_release2", "Unreleased reference id=2"}, {"ringbuf_missing_release_callback", "Unreleased reference id"}, {"use_after_invalid", "Expected an initialized dynptr as arg #3"}, - {"ringbuf_invalid_api", "type=mem expected=alloc_mem"}, + {"ringbuf_invalid_api", "type=mem expected=ringbuf_mem"}, {"add_dynptr_to_map1", "invalid indirect read from stack"}, {"add_dynptr_to_map2", "invalid indirect read from stack"}, {"data_slice_out_of_bounds_ringbuf", "value is outside of the allowed memory range"}, diff --git a/tools/testing/selftests/bpf/verifier/ringbuf.c b/tools/testing/selftests/bpf/verifier/ringbuf.c index b64d33e4833c..84838feba47f 100644 --- a/tools/testing/selftests/bpf/verifier/ringbuf.c +++ b/tools/testing/selftests/bpf/verifier/ringbuf.c @@ -28,7 +28,7 @@ }, .fixup_map_ringbuf = { 1 }, .result = REJECT, - .errstr = "dereference of modified alloc_mem ptr R1", + .errstr = "dereference of modified ringbuf_mem ptr R1", }, { "ringbuf: invalid reservation offset 2", diff --git a/tools/testing/selftests/bpf/verifier/spill_fill.c b/tools/testing/selftests/bpf/verifier/spill_fill.c index e23f07175e1b..9bb302dade23 100644 --- a/tools/testing/selftests/bpf/verifier/spill_fill.c +++ b/tools/testing/selftests/bpf/verifier/spill_fill.c @@ -84,7 +84,7 @@ }, .fixup_map_ringbuf = { 1 }, .result = REJECT, - .errstr = "R0 pointer arithmetic on alloc_mem_or_null prohibited", + .errstr = "R0 pointer arithmetic on ringbuf_mem_or_null prohibited", }, { "check corrupted spill/fill",
Currently, verifier uses MEM_ALLOC type tag to specially tag memory returned from bpf_ringbuf_reserve helper. However, this is currently only used for this purpose and there is an implicit assumption that it only refers to ringbuf memory (e.g. the check for ARG_PTR_TO_ALLOC_MEM in check_func_arg_reg_off). Hence, rename MEM_ALLOC to MEM_RINGBUF to indicate this special relationship and instead open the use of MEM_ALLOC for more generic allocations made for user types. Also, since ARG_PTR_TO_ALLOC_MEM_OR_NULL is unused, simply drop it. Finally, update selftests using 'alloc_' verifier string to 'ringbuf_'. Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> --- include/linux/bpf.h | 11 ++++------- kernel/bpf/ringbuf.c | 6 +++--- kernel/bpf/verifier.c | 14 +++++++------- tools/testing/selftests/bpf/prog_tests/dynptr.c | 2 +- tools/testing/selftests/bpf/verifier/ringbuf.c | 2 +- tools/testing/selftests/bpf/verifier/spill_fill.c | 2 +- 6 files changed, 17 insertions(+), 20 deletions(-)