linux-kernel - Re: [PATCH bpf-next v2 1/2] bpf: Add ifindex to 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]

Date:   Thu, 4 Nov 2021 11:06:21 -0700
From:   Yonghong Song <yhs@...com>
To:     Mark Pashmfouroush <markpash@...udflare.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>
CC:     <kernel-team@...udflare.com>, <netdev@...r.kernel.org>,
        <bpf@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 1/2] bpf: Add ifindex to bpf_sk_lookup



On 11/4/21 5:23 AM, Mark Pashmfouroush wrote:
> It may be helpful to have access to the ifindex during bpf socket
> lookup. An example may be to scope certain socket lookup logic to
> specific interfaces, i.e. an interface may be made exempt from custom
> lookup code.
> 
> Add the ifindex of the arriving connection to the bpf_sk_lookup API.
> 
> Signed-off-by: Mark Pashmfouroush <markpash@...udflare.com>
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 24b7ed2677af..0012a5176a32 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -1374,6 +1374,7 @@ struct bpf_sk_lookup_kern {
>   		const struct in6_addr *daddr;
>   	} v6;
>   	struct sock	*selected_sk;
> +	u32		ifindex;

In struct __sk_buff, we have two ifindex related fields:

         __u32 ingress_ifindex;
         __u32 ifindex;

Does newly-added ifindex corresponds to skb->ingress_ifindex or
skb->ifindex? From comments:
   > +	__u32 ifindex;		/* The arriving interface. Determined by inet_iif. */

looks like it corresponds to ingress? Should be use the name
ingress_ifindex to be consistent with __sk_buff?

>   	bool		no_reuseport;
>   };
>   
> @@ -1436,7 +1437,7 @@ extern struct static_key_false bpf_sk_lookup_enabled;
>   static inline bool bpf_sk_lookup_run_v4(struct net *net, int protocol,
>   					const __be32 saddr, const __be16 sport,
>   					const __be32 daddr, const u16 dport,
> -					struct sock **psk)
> +					const int ifindex, struct sock **psk)
>   {
>   	struct bpf_prog_array *run_array;
>   	struct sock *selected_sk = NULL;
> @@ -1452,6 +1453,7 @@ static inline bool bpf_sk_lookup_run_v4(struct net *net, int protocol,
>   			.v4.daddr	= daddr,
>   			.sport		= sport,
>   			.dport		= dport,
> +			.ifindex	= ifindex,
>   		};
>   		u32 act;
>   
> @@ -1474,7 +1476,7 @@ static inline bool bpf_sk_lookup_run_v6(struct net *net, int protocol,
>   					const __be16 sport,
>   					const struct in6_addr *daddr,
>   					const u16 dport,
> -					struct sock **psk)
> +					const int ifindex, struct sock **psk)
>   {
>   	struct bpf_prog_array *run_array;
>   	struct sock *selected_sk = NULL;
> @@ -1490,6 +1492,7 @@ static inline bool bpf_sk_lookup_run_v6(struct net *net, int protocol,
>   			.v6.daddr	= daddr,
>   			.sport		= sport,
>   			.dport		= dport,
> +			.ifindex	= ifindex,
>   		};
>   		u32 act;
>   
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index ba5af15e25f5..5b8618a4d485 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6296,6 +6296,7 @@ struct bpf_sk_lookup {
>   	__u32 local_ip4;	/* Network byte order */
>   	__u32 local_ip6[4];	/* Network byte order */
>   	__u32 local_port;	/* Host byte order */
> +	__u32 ifindex;		/* The arriving interface. Determined by inet_iif. */
>   };
[...]