netdev - Re: [PATCH bpf-next] bpf: libbpf: retry program creation without the name

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]

Message-ID: <20181120195121.td3bugcegudhh2ou@ast-mbp.dhcp.thefacebook.com>
Date:   Tue, 20 Nov 2018 11:51:23 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Stanislav Fomichev <sdf@...gle.com>
Cc:     netdev@...r.kernel.org, daniel@...earbox.net, ast@...nel.org,
        vladum@...gle.com
Subject: Re: [PATCH bpf-next] bpf: libbpf: retry program creation without the
 name

On Mon, Nov 19, 2018 at 04:46:25PM -0800, Stanislav Fomichev wrote:
> [Recent commit 23499442c319 ("bpf: libbpf: retry map creation without
> the name") fixed this issue for maps, let's do the same for programs.]
> 
> Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support
> to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name
> for programs. Pre v4.14 kernels don't know about programs names and
> return an error about unexpected non-zero data. Retry sys_bpf without
> a program name to cover older kernels.
> 
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> ---
>  tools/lib/bpf/bpf.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 961e1b9fc592..cbe9d757c646 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -212,6 +212,16 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr,
>  	if (fd >= 0 || !log_buf || !log_buf_sz)
>  		return fd;
>  
> +	if (fd < 0 && errno == E2BIG && load_attr->name) {
> +		/* Retry the same syscall, but without the name.
> +		 * Pre v4.14 kernels don't support prog names.
> +		 */

I'm afraid that will put unnecessary stress on the kernel.
This check needs to be tighter.
Like E2BIG and anything in the log_buf probably means that
E2BIG came from the verifier and nothing to do with prog_name.
Asking kernel to repeat is an unnecessary work.

In general we need to think beyond this single prog_name field.
There are bunch of other fields in bpf_load_program_xattr() and older kernels
won't support them. Are we going to zero them out one by one
and retry? I don't think that would be practical.

Also libbpf silently ignoring prog_name is not great for debugging.
A warning is needed.
But it cannot be done out of lib/bpf/bpf.c, since it's a set of syscall
wrappers.
Imo such "old kernel -> lets retry" feature should probably be done
at lib/bpf/libbpf.c level. inside load_program().