netdev - Re: [PATCH v2 bpf-next] libbpf: always specify expected_attach

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:   Tue, 14 Apr 2020 07:03:59 +0000
From:   Song Liu <songliubraving@...com>
To:     Andrii Nakryiko <andriin@...com>
CC:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        "Alexei Starovoitov" <ast@...com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        Andrey Ignatov <rdna@...com>,
        "andrii.nakryiko@...il.com" <andrii.nakryiko@...il.com>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH v2 bpf-next] libbpf: always specify expected_attach_type
 on program load if supported



> On Apr 13, 2020, at 9:56 PM, Andrii Nakryiko <andriin@...com> wrote:
> 
> For some types of BPF programs that utilize expected_attach_type, libbpf won't
> set load_attr.expected_attach_type, even if expected_attach_type is known from
> section definition. This was done to preserve backwards compatibility with old
> kernels that didn't recognize expected_attach_type attribute yet (which was
> added in 5e43f899b03a ("bpf: Check attach type at prog load time"). But this
> is problematic for some BPF programs that utilize never features that require
> kernel to know specific expected_attach_type (e.g., extended set of return
> codes for cgroup_skb/egress programs).
> 
> This patch makes libbpf specify expected_attach_type by default, but also
> detect support for this field in kernel and not set it during program load.
> This allows to have a good metadata for bpf_program
> (e.g., bpf_program__get_extected_attach_type()), but still work with old
> kernels (for cases where it can work at all).
> 
> Additionally, due to expected_attach_type being always set for recognized
> program types, bpf_program__attach_cgroup doesn't have to do extra checks to
> determine correct attach type, so remove that additional logic.
> 
> Also adjust section_names selftest to account for this change.
> 
> More detailed discussion can be found in [0].
> 
>  [0] https://lore.kernel.org/bpf/20200412003604.GA15986@rdna-mbp.dhcp.thefacebook.com/
> 
> Reported-by: Andrey Ignatov <rdna@...com>
> Signed-off-by: Andrii Nakryiko <andriin@...com>

Acked-by: Song Liu <songliubraving@...com>

With one nit below. 

> ---
> v1->v2:
> - fixed prog_type/expected_attach_type combo (Andrey);
> - added comment explaining what we are doing in probe_exp_attach_type (Andrey).
> 
> tools/lib/bpf/libbpf.c                        | 127 ++++++++++++------
> .../selftests/bpf/prog_tests/section_names.c  |  42 +++---
> 2 files changed, 110 insertions(+), 59 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ff9174282a8c..c7393182e2ae 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -178,6 +178,8 @@ struct bpf_capabilities {
> 	__u32 array_mmap:1;
> 	/* BTF_FUNC_GLOBAL is supported */
> 	__u32 btf_func_global:1;
> +	/* kernel support for expected_attach_type in BPF_PROG_LOAD */
> +	__u32 exp_attach_type:1;
> };

[...]

> -#define BPF_PROG_SEC_IMPL(string, ptype, eatype, is_attachable, btf, atype) \
> -	{ string, sizeof(string) - 1, ptype, eatype, is_attachable, btf, atype }
> +#define BPF_PROG_SEC_IMPL(string, ptype, eatype, eatype_optional,	    \
> +			  attachable, attach_btf)			    \
> +	{								    \
> +		.sec = string,						    \
> +		.len = sizeof(string) - 1,				    \
> +		.prog_type = ptype,					    \
> +		.sec = string,						    \

Two lines of ".sec = string". 

> +		.expected_attach_type = eatype,				    \
> +		.is_exp_attach_type_optional = eatype_optional,		    \
> +		.is_attachable = attachable,				    \
> +		.is_attach_btf = attach_btf,				    \
> +	}