netdev - Re: [PATCH bpf-next 3/8] net: Introduce netns_bpf for BPF programs attached to netns

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: <20200527174036.GF49942@google.com>
Date:   Wed, 27 May 2020 10:40:36 -0700
From:   sdf@...gle.com
To:     Jakub Sitnicki <jakub@...udflare.com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org,
        kernel-team@...udflare.com
Subject: Re: [PATCH bpf-next 3/8] net: Introduce netns_bpf for BPF programs
 attached to netns

On 05/27, Jakub Sitnicki wrote:
> In order to:

>   (1) attach more than one BPF program type to netns, or
>   (2) support attaching BPF programs to netns with bpf_link, or
>   (3) support multi-prog attach points for netns

> we will need to keep more state per netns than a single pointer like we
> have now for BPF flow dissector program.

> Prepare for the above by extracting netns_bpf that is part of struct net,
> for storing all state related to BPF programs attached to netns.

> Turn flow dissector callbacks for querying/attaching/detaching a program
> into generic ones that operate on netns_bpf. Next patch will move the
> generic callbacks into their own module.

> This is similar to how it is organized for cgroup with cgroup_bpf.

> Signed-off-by: Jakub Sitnicki <jakub@...udflare.com>
> ---
>   include/linux/bpf-netns.h   | 56 ++++++++++++++++++++++
>   include/linux/skbuff.h      | 26 ----------
>   include/net/net_namespace.h |  4 +-
>   include/net/netns/bpf.h     | 17 +++++++
>   kernel/bpf/syscall.c        |  7 +--
>   net/core/flow_dissector.c   | 96 ++++++++++++++++++++++++-------------
>   6 files changed, 143 insertions(+), 63 deletions(-)
>   create mode 100644 include/linux/bpf-netns.h
>   create mode 100644 include/net/netns/bpf.h

> diff --git a/include/linux/bpf-netns.h b/include/linux/bpf-netns.h
> new file mode 100644
> index 000000000000..f3aec3d79824
> --- /dev/null
> +++ b/include/linux/bpf-netns.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _BPF_NETNS_H
> +#define _BPF_NETNS_H
> +
> +#include <linux/mutex.h>
> +#include <uapi/linux/bpf.h>
> +
> +enum netns_bpf_attach_type {
> +	NETNS_BPF_INVALID = -1,
> +	NETNS_BPF_FLOW_DISSECTOR = 0,
> +	MAX_NETNS_BPF_ATTACH_TYPE
> +};
> +
> +static inline enum netns_bpf_attach_type
> +to_netns_bpf_attach_type(enum bpf_attach_type attach_type)
> +{
> +	switch (attach_type) {
> +	case BPF_FLOW_DISSECTOR:
> +		return NETNS_BPF_FLOW_DISSECTOR;
> +	default:
> +		return NETNS_BPF_INVALID;
> +	}
> +}
> +
> +/* Protects updates to netns_bpf */
> +extern struct mutex netns_bpf_mutex;
I wonder whether it's a good time to make this mutex per-netns, WDYT?

The only problem I see is that it might complicate the global
mode of flow dissector where we go over every ns to make sure no
progs are attached. That will be racy with per-ns mutex unless
we do something about it...