netdev - Re: [PATCH v4 bpf-next 6/7] libbpf: add BPF_CORE_READ/BPF_CORE_READ

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: <035617e9-2d0d-4082-8862-45bc4bb210fe@fb.com>
Date:   Tue, 8 Oct 2019 04:56:25 +0000
From:   Alexei Starovoitov <ast@...com>
To:     Andrii Nakryiko <andriin@...com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>
CC:     "andrii.nakryiko@...il.com" <andrii.nakryiko@...il.com>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH v4 bpf-next 6/7] libbpf: add
 BPF_CORE_READ/BPF_CORE_READ_INTO helpers

On 10/7/19 3:47 PM, Andrii Nakryiko wrote:
> Add few macros simplifying BCC-like multi-level probe reads, while also
> emitting CO-RE relocations for each read.
> 
> Acked-by: John Fastabend <john.fastabend@...il.com>
> Acked-by: Song Liu <songliubraving@...com>
> Signed-off-by: Andrii Nakryiko <andriin@...com>
...
> +/*
> + * BPF_CORE_READ() is used to simplify BPF CO-RE relocatable read, especially
> + * when there are few pointer chasing steps.
> + * E.g., what in non-BPF world (or in BPF w/ BCC) would be something like:
> + *	int x = s->a.b.c->d.e->f->g;
> + * can be succinctly achieved using BPF_CORE_READ as:
> + *	int x = BPF_CORE_READ(s, a.b.c, d.e, f, g);
> + *
> + * BPF_CORE_READ will decompose above statement into 4 bpf_core_read (BPF
> + * CO-RE relocatable bpf_probe_read() wrapper) calls, logically equivalent to:
> + * 1. const void *__t = s->a.b.c;
> + * 2. __t = __t->d.e;
> + * 3. __t = __t->f;
> + * 4. return __t->g;
> + *
> + * Equivalence is logical, because there is a heavy type casting/preservation
> + * involved, as well as all the reads are happening through bpf_probe_read()
> + * calls using __builtin_preserve_access_index() to emit CO-RE relocations.
> + *
> + * N.B. Only up to 9 "field accessors" are supported, which should be more
> + * than enough for any practical purpose.
> + */
> +#define BPF_CORE_READ(src, a, ...)					    \
> +	({								    \
> +		___type(src, a, ##__VA_ARGS__) __r;			    \
> +		BPF_CORE_READ_INTO(&__r, src, a, ##__VA_ARGS__);	    \
> +		__r;							    \
> +	})
> +

Since we're splitting things into
bpf_{helpers,helper_defs,endian,tracing}.h
how about adding all core macros into bpf_core_read.h ?
#define___concat, ___empty are very generic names.
I'd rather contain the risk of conflicts to progs that are going
to use co-re instead of forcing it on all progs that use bpf_helpers.h.
With my btf vmlinux stuff all these bpf_probe_read*() wrappers
hopefully will be obsolete eventually. So keeping them separate in 
bpf_core_read.h would help the transition too.