DON'T MERGE: Trying to fix ctx access

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
cilium · Mar 15, 2023 · b8a7a6c · b8a7a6c
1 parent be12e9c
commit b8a7a6c
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 2 deletions.
diff --git a/bpf/include/bpf/ctx/skb.h b/bpf/include/bpf/ctx/skb.h
@@ -57,6 +57,29 @@
 #define get_hash(ctx)		ctx->hash
 #define get_hash_recalc(ctx)	get_hash(ctx)
 
+#define DEFINE_FUNC_CTX_POINTER(FIELD)						\
+static __always_inline void *							\
+__ctx_ptr_ ## FIELD(const struct __sk_buff *ctx)				\
+{										\
+	void *ptr;								\
+										\
+	/* LLVM may generate u32 assignments of ctx->{data,data_end,data_meta}.	\
+	 * With this inline asm, LLVM loses track of the fact this field is on	\
+	 * 32 bits.								\
+	 */									\
+	asm volatile("%0 = *(u32 *)(%1 + %2)"					\
+		     : "=r"(ptr)						\
+		     : "r"(ctx), "i"(offsetof(struct __sk_buff, FIELD)));	\
+	return ptr;								\
+}
+/* This defines ctx_data(). */
+DEFINE_FUNC_CTX_POINTER(data)
+/* This defines ctx_data_end(). */
+DEFINE_FUNC_CTX_POINTER(data_end)
+/* This defines ctx_data_meta(). */
+DEFINE_FUNC_CTX_POINTER(data_meta)
+#undef DEFINE_FUNC_CTX_POINTER
+
 static __always_inline __maybe_unused int
 ctx_redirect(const struct __sk_buff *ctx __maybe_unused, int ifindex, __u32 flags)
 {

diff --git a/bpf/include/bpf/ctx/xdp.h b/bpf/include/bpf/ctx/xdp.h
@@ -107,6 +107,29 @@ xdp_store_bytes(const struct xdp_md *ctx, __u64 off, const void *from,
 #define get_hash(ctx)			({ 0; })
 #define get_hash_recalc(ctx)		get_hash(ctx)
 
+#define DEFINE_FUNC_CTX_POINTER(FIELD)						\
+static __always_inline void *							\
+__ctx_ptr_ ## FIELD(const struct xdp_md *ctx)					\
+{										\
+	void *ptr;								\
+										\
+	/* LLVM may generate u32 assignments of ctx->{data,data_end,data_meta}.	\
+	 * With this inline asm, LLVM loses track of the fact this field is on	\
+	 * 32 bits.								\
+	 */									\
+	asm volatile("%0 = *(u32 *)(%1 + %2)"					\
+		     : "=r"(ptr)						\
+		     : "r"(ctx), "i"(offsetof(struct xdp_md, FIELD)));		\
+	return ptr;								\
+}
+/* This defines ctx_data(). */
+DEFINE_FUNC_CTX_POINTER(data)
+/* This defines ctx_data_end(). */
+DEFINE_FUNC_CTX_POINTER(data_end)
+/* This defines ctx_data_meta(). */
+DEFINE_FUNC_CTX_POINTER(data_meta)
+#undef DEFINE_FUNC_CTX_POINTER
+
 static __always_inline __maybe_unused void
 __csum_replace_by_diff(__sum16 *sum, __wsum diff)
 {

diff --git a/bpf/lib/nat.h b/bpf/lib/nat.h
@@ -1281,6 +1281,29 @@ static __always_inline void snat_v6_init_tuple(const struct ipv6hdr *ip6,
 	tuple->flags = dir;
 }
 
+static __always_inline bool
+__revalidate_data_pull_with_asm(struct __ctx_buff *ctx,
+				void **data_, void **data_end_,
+				void **l3, const __u32 l3_len)
+{
+        const __u64 tot_len = ETH_HLEN + l3_len;
+	void *data_end;
+	void *data;
+
+	/* Verifier workaround, do this unconditionally: invalid size of register spill. */
+	data_end = __ctx_ptr_data_end(ctx);
+	data = __ctx_ptr_data(ctx);
+	if (data + tot_len > data_end)
+		return false;
+
+	/* Verifier workaround: pointer arithmetic on pkt_end prohibited. */
+	*data_ = data;
+	*data_end_ = data_end;
+
+	*l3 = data + ETH_HLEN;
+	return true;
+}
+
 static __always_inline bool snat_v6_needed(struct __ctx_buff *ctx,
 					   union v6addr *addr)
 {
@@ -1291,8 +1314,9 @@ static __always_inline bool snat_v6_needed(struct __ctx_buff *ctx,
 	void *data, *data_end;
 	struct ipv6hdr *ip6;
 
-	if (!revalidate_data(ctx, &data, &data_end, &ip6))
-		return false;
+        if (!__revalidate_data_pull_with_asm(ctx, &data, &data_end,
+                                             (void **)&ip6, sizeof(*ip6)))
+          return false;
 
 	/* See comment in snat_v4_prepare_state(). */
 	if (DIRECT_ROUTING_DEV_IFINDEX == NATIVE_DEV_IFINDEX &&