Messages in this thread | | | Subject | Re: [PATCH bpf-next 4/6] bpf: override the meaning of ARG_PTR_TO_MAP_VALUE for sockmap and sockhash | From | Yonghong Song <> | Date | Wed, 19 Aug 2020 13:13:23 -0700 |
| |
On 8/19/20 2:24 AM, Lorenz Bauer wrote: > The verifier assumes that map values are simple blobs of memory, and > therefore treats ARG_PTR_TO_MAP_VALUE, etc. as such. However, there are > map types where this isn't true. For example, sockmap and sockhash store > sockets. In general this isn't a big problem: we can just > write helpers that explicitly requests PTR_TO_SOCKET instead of > ARG_PTR_TO_MAP_VALUE. > > The one exception are the standard map helpers like map_update_elem, > map_lookup_elem, etc. Here it would be nice we could overload the > function prototype for different kinds of maps. Unfortunately, this > isn't entirely straight forward: > We only know the type of the map once we have resolved meta->map_ptr > in check_func_arg. This means we can't swap out the prototype > in check_helper_call until we're half way through the function. > > Instead, modify check_func_arg to treat ARG_PTR_TO_MAP_VALUE* to > mean "the native type for the map" instead of "pointer to memory" > for sockmap and sockhash. This means we don't have to modify the > function prototype at all > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com> > --- > kernel/bpf/verifier.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index b6ccfce3bf4c..47f9b94bb9d4 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -3872,6 +3872,38 @@ static int int_ptr_type_to_size(enum bpf_arg_type type) > return -EINVAL; > } > > +static int override_map_arg_type(struct bpf_verifier_env *env, > + const struct bpf_call_arg_meta *meta, > + enum bpf_arg_type *arg_type) > +{ > + if (!meta->map_ptr) { > + /* kernel subsystem misconfigured verifier */ > + verbose(env, "invalid map_ptr to access map->type\n"); > + return -EACCES; > + } > + > + switch (meta->map_ptr->map_type) { > + case BPF_MAP_TYPE_SOCKMAP: > + case BPF_MAP_TYPE_SOCKHASH: > + switch (*arg_type) { > + case ARG_PTR_TO_MAP_VALUE: > + *arg_type = ARG_PTR_TO_SOCKET; > + break; > + case ARG_PTR_TO_MAP_VALUE_OR_NULL: > + *arg_type = ARG_PTR_TO_SOCKET_OR_NULL; > + break; > + default: > + verbose(env, "invalid arg_type for sockmap/sockhash\n"); > + return -EINVAL; > + } > + break; > + > + default: > + break; > + } > + return 0; > +} > + > static int check_func_arg(struct bpf_verifier_env *env, u32 arg, > struct bpf_call_arg_meta *meta, > const struct bpf_func_proto *fn) > @@ -3904,6 +3936,14 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, > return -EACCES; > } > > + if (arg_type == ARG_PTR_TO_MAP_VALUE || > + arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE || > + arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL) {
We probably do not need ARG_PTR_TO_UNINIT_MAP_VALUE here.
Do we need ARG_PTR_TO_MAP_VALUE_OR_NULL? bpf_map_update_elem arg type is ARG_PTR_TO_MAP_VALUE.
> + err = override_map_arg_type(env, meta, &arg_type); > + if (err) > + return err; > + } > + > if (arg_type == ARG_PTR_TO_MAP_KEY || > arg_type == ARG_PTR_TO_MAP_VALUE || > arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE || >
| |