On Thu, 06 Jun 2019 15:24:14 +0200 Toke Høiland-Jørgensen <t...@redhat.com> wrote:
> From: Toke Høiland-Jørgensen <t...@redhat.com> > > We don't currently allow lookups into a devmap from eBPF, because the map > lookup returns a pointer directly to the dev->ifindex, which shouldn't be > modifiable from eBPF. > > However, being able to do lookups in devmaps is useful to know (e.g.) > whether forwarding to a specific interface is enabled. Currently, programs > work around this by keeping a shadow map of another type which indicates > whether a map index is valid. > > Since we now have a flag to make maps read-only from the eBPF side, we can > simply lift the lookup restriction if we make sure this flag is always set. Nice, I didn't know this was possible. I like it! :-) > Signed-off-by: Toke Høiland-Jørgensen <t...@redhat.com> > --- > kernel/bpf/devmap.c | 5 +++++ > kernel/bpf/verifier.c | 7 ++----- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c > index 5ae7cce5ef16..0e6875a462ef 100644 > --- a/kernel/bpf/devmap.c > +++ b/kernel/bpf/devmap.c > @@ -99,6 +99,11 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr) > attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK) > return ERR_PTR(-EINVAL); > > + /* Lookup returns a pointer straight to dev->ifindex, so make sure the > + * verifier prevents writes from the BPF side > + */ > + attr->map_flags |= BPF_F_RDONLY_PROG; > + > dtab = kzalloc(sizeof(*dtab), GFP_USER); > if (!dtab) > return ERR_PTR(-ENOMEM); > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 5c2cb5bd84ce..7128a9821481 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -2893,12 +2893,9 @@ static int check_map_func_compatibility(struct > bpf_verifier_env *env, > if (func_id != BPF_FUNC_get_local_storage) > goto error; > break; > - /* devmap returns a pointer to a live net_device ifindex that we cannot > - * allow to be modified from bpf side. So do not allow lookup elements > - * for now. > - */ > case BPF_MAP_TYPE_DEVMAP: > - if (func_id != BPF_FUNC_redirect_map) > + if (func_id != BPF_FUNC_redirect_map && > + func_id != BPF_FUNC_map_lookup_elem) > goto error; > break; > /* Restrict bpf side of cpumap and xskmap, open when use-cases > -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
