netdev - Re: [PATCH bpf-next 2/4] bpf: Split bpf_sk

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20181109171928.jb7k2lbe2rdncyet@kafai-mbp.dhcp.thefacebook.com>
Date:   Fri, 9 Nov 2018 17:19:34 +0000
From:   Martin Lau <kafai@...com>
To:     Andrey Ignatov <rdna@...com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "joe@...d.net.nz" <joe@...d.net.nz>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next 2/4] bpf: Split bpf_sk_lookup

On Thu, Nov 08, 2018 at 08:54:23AM -0800, Andrey Ignatov wrote:
> Split bpf_sk_lookup to separate core functionality, that can be reused
> to make socket lookup available to more program types, from
> functionality specific to program types that have access to skb.
> 
> Core functionality is placed to __bpf_sk_lookup. And bpf_sk_lookup only
> gets caller netns and ifindex from skb and passes it to __bpf_sk_lookup.
> 
> Program types that don't have access to skb can just pass NULL to
> __bpf_sk_lookup that will be handled correctly by both inet{,6}_sdif and
> lookup functions.
> 
> This is refactoring that simply moves blocks around and does NOT change
> existing logic.
> 
> Signed-off-by: Andrey Ignatov <rdna@...com>
> Acked-by: Alexei Starovoitov <ast@...nel.org>
> ---
>  net/core/filter.c | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9a1327eb25fa..dc0f86a707b7 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4825,14 +4825,10 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
>  
>  #ifdef CONFIG_INET
>  static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> -			      struct sk_buff *skb, u8 family, u8 proto)
> +			      struct sk_buff *skb, u8 family, u8 proto, int dif)
>  {
>  	bool refcounted = false;
>  	struct sock *sk = NULL;
> -	int dif = 0;
> -
> -	if (skb->dev)
> -		dif = skb->dev->ifindex;
>  
>  	if (family == AF_INET) {
>  		__be32 src4 = tuple->ipv4.saddr;
> @@ -4875,16 +4871,16 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
>  	return sk;
>  }
>  
> -/* bpf_sk_lookup performs the core lookup for different types of sockets,
> +/* __bpf_sk_lookup performs the core lookup for different types of sockets,
>   * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
>   * Returns the socket as an 'unsigned long' to simplify the casting in the
>   * callers to satisfy BPF_CALL declarations.
>   */
>  static unsigned long
> -bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> -	      u8 proto, u64 netns_id, u64 flags)
> +__bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> +		u8 proto, u64 netns_id, struct net *caller_net, int ifindex,
> +		u64 flags)
That looks a bit different from the one landed to bpf-next.
You may need to respin the set.

>  {
> -	struct net *caller_net;
>  	struct sock *sk = NULL;
>  	u8 family = AF_UNSPEC;
>  	struct net *net;
> @@ -4893,19 +4889,15 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
>  	if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
>  		goto out;
>  
> -	if (skb->dev)
> -		caller_net = dev_net(skb->dev);
> -	else
> -		caller_net = sock_net(skb->sk);
>  	if (netns_id) {
>  		net = get_net_ns_by_id(caller_net, netns_id);
>  		if (unlikely(!net))
>  			goto out;
> -		sk = sk_lookup(net, tuple, skb, family, proto);
> +		sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
>  		put_net(net);
>  	} else {
>  		net = caller_net;
> -		sk = sk_lookup(net, tuple, skb, family, proto);
> +		sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
>  	}
>  
>  	if (sk)
> @@ -4914,6 +4906,22 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
>  	return (unsigned long) sk;
>  }
>  
> +static unsigned long
> +bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> +	      u8 proto, u64 netns_id, u64 flags)
> +{
> +	struct net *caller_net = sock_net(skb->sk);
> +	int ifindex = 0;
> +
> +	if (skb->dev) {
> +		caller_net = dev_net(skb->dev);
> +		ifindex = skb->dev->ifindex;
> +	}
> +
> +	return __bpf_sk_lookup(skb, tuple, len, proto, netns_id, caller_net,
> +			       ifindex, flags);
> +}
> +
>  BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
>  	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
>  {
> -- 
> 2.17.1
>