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
