netdev - Re: [PATCH net-next v2 2/2] devmap: Allow map lookups from eBPF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Jun 2019 15:33:44 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>, brouer@...hat.com
Subject: Re: [PATCH net-next v2 2/2] devmap: Allow map lookups from eBPF

On Thu, 06 Jun 2019 15:24:14 +0200
Toke Høiland-Jørgensen <toke@...hat.com> wrote:

> From: Toke Høiland-Jørgensen <toke@...hat.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 <toke@...hat.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