Date:   Tue, 28 Jun 2022 21:48:10 +0200
From:   Alexander Lobakin <alexandr.lobakin@...el.com>
To:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>
Cc:     Alexander Lobakin <alexandr.lobakin@...el.com>,
        Larysa Zaremba <larysa.zaremba@...el.com>,
        Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Björn Töpel <bjorn@...nel.org>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Toke Hoiland-Jorgensen <toke@...hat.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        John Fastabend <john.fastabend@...il.com>,
        Yajun Deng <yajun.deng@...ux.dev>,
        Willem de Bruijn <willemb@...gle.com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        xdp-hints@...-project.net
Subject: [PATCH RFC bpf-next 50/52] libbpf: introduce a couple memory access helpers

From: Larysa Zaremba <larysa.zaremba@...el.com>

In BPF programs, it is a common thing to declare that we're going
to do a memory access via such snippet:

	if (data + ETH_HLEN > data_end)
		// bail out

Offsets can be variable:

	if (VLAN_HLEN * vlan_count > SOME_ARBITRARY_MAX_OFFSET ||
	    ctx->data + VLAN_HLEN * vlan_count > data_end)
		//

Or even calculated from the end:

	if (ctx->data_end - ctx->data - ETH_FCS_LEN > SOME_ARB_MAX_OFF ||
	    ctx->data_end - ETH_FCS_LEN < ctx->data)
		//

As a bonus, LLVM sometimes has a hard time compiling sane C code
the way that it would pass the in-kernel verifier.
Add two new functions to sanitize memory accesses and get pointers
to the requested ranges: one taking an offset from the start and one
from the end (useful for metadata and different integrity check
headers). They are written in Asm, so the offset can be variable and
the code will pass the verifier. There are checks for the maximum
offset (backed by the original verifier value), going out of bounds
etc., so the pointer they return is ready to use (if it's
non-%NULL).
So now all is needed is:

	iphdr = bpf_access_mem(ctx->data, ctx->data_end, ETH_HLEN,
			       sizeof(*iphdr));
	if (!iphdr)
		// bail out

or

	some_meta_struct = bpf_access_mem_end(ctx->data_meta, ctx->data,
					      sizeof(*some_meta_struct),
					      sizeof(*some_meta_struct));
	if (!some_meta_struct)
		//

The Asm code was happily stolen from the Cilium project repo[0] and
then reworked.

[0] https://github.com/cilium/cilium/blob/master/bpf/include/bpf/ctx/xdp.h#L43

Suggested-by: Daniel Borkmann <daniel@...earbox.net> # original helper
Suggested-by: Toke Høiland-Jørgensen <toke@...hat.com>
Signed-off-by: Larysa Zaremba <larysa.zaremba@...el.com>
Co-developed-by: Alexander Lobakin <alexandr.lobakin@...el.com>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@...el.com>
---
 tools/lib/bpf/bpf_helpers.h | 64 +++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index fb04eaf367f1..cd16e3c9cd85 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -285,4 +285,68 @@ enum libbpf_tristate {
 /* Helper macro to print out debug messages */
 #define bpf_printk(fmt, args...) ___bpf_pick_printk(args)(fmt, ##args)
 
+/* Max offset as per kernel verifier */
+#define MAX_PACKET_OFF		0xffff
+
+/**
+ * bpf_access_mem - sanitize memory access to a range
+ * @mem: start of the memory segment
+ * @mem_end: end of the memory segment
+ * @off: offset from the start of the memory segment
+ * @len: length of the range to give access to
+ *
+ * Verifies that the memory operations we want to perform are sane and within
+ * bounds and gives pointer to the requested range. The checks are done in Asm,
+ * so that it is safe to pass variable offset (verifier might reject such code
+ * written in plain C).
+ * The intended way of using it is as follows:
+ *
+ * iphdr = bpf_access_mem(ctx->data, ctx->data_end, ETH_HLEN, sizeof(*iphdr));
+ *
+ * Returns pointer to the beginning of the range or %NULL.
+ */
+static __always_inline void *
+bpf_access_mem(__u64 mem, __u64 mem_end, __u64 off, const __u64 len)
+{
+	void *ret;
+
+	asm volatile("r1 = %[start]\n\t"
+		     "r2 = %[end]\n\t"
+		     "r3 = %[offmax] - %[len]\n\t"
+		     "if %[off] > r3 goto +5\n\t"
+		     "r1 += %[off]\n\t"
+		     "%[ret] = r1\n\t"
+		     "r1 += %[len]\n\t"
+		     "if r1 > r2 goto +1\n\t"
+		     "goto +1\n\t"
+		     "%[ret] = %[null]\n\t"
+		     : [ret]"=r"(ret)
+		     : [start]"r"(mem), [end]"r"(mem_end), [off]"r"(off),
+		       [len]"ri"(len), [offmax]"i"(MAX_PACKET_OFF),
+		       [null]"i"(NULL)
+		     : "r1", "r2", "r3");
+
+	return ret;
+}
+
+/**
+ * bpf_access_mem_end - sanitize memory access to a range at the end of segment
+ * @mem: start of the memory segment
+ * @mem_end: end of the memory segment
+ * @offend: offset from the end of the memory segment
+ * @len: length of the range to give access to
+ *
+ * Version of bpf_access_mem() which performs all needed calculations to
+ * access a memory segment from the end. E.g., to access FCS (if provided):
+ *
+ * cp = bpf_access_mem_end(ctx->data, ctx->data_end, ETH_FCS_LEN, ETH_FCS_LEN);
+ *
+ * Returns pointer to the beginning of the range or %NULL.
+ */
+static __always_inline void *
+bpf_access_mem_end(__u64 mem, __u64 mem_end, __u64 offend, const __u64 len)
+{
+	return bpf_access_mem(mem, mem_end, mem_end - mem - offend, len);
+}
+
 #endif
-- 
2.36.1

Powered by blists - more mailing lists