[PATCH bpf-next v3 1/3] selftests/bpf: Add tests for pointer type merge at a shared load

Daniel Borkmann <[email protected]>
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
Cover the ways in which the type recorded for a shared load used to lose
the BPF_PROBE_MEM rewrite which would then trigger a NULL deref if not
handled properly.

  # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t mem_rdonly_untrusted
  [...]
  #242/1   mem_rdonly_untrusted/btf_id_to_ptr_mem:OK
  #242/2   mem_rdonly_untrusted/ldx_is_ok_bad_addr:OK
  #242/3   mem_rdonly_untrusted/ldx_is_ok_good_addr:OK
  #242/4   mem_rdonly_untrusted/offset_not_tracked:OK
  #242/5   mem_rdonly_untrusted/stx_not_ok:OK
  #242/6   mem_rdonly_untrusted/atomic_not_ok:OK
  #242/7   mem_rdonly_untrusted/atomic_rmw_not_ok:OK
  #242/8   mem_rdonly_untrusted/kfunc_param_not_ok:OK
  #242/9   mem_rdonly_untrusted/mixed_mem_type:OK
  #242/10  mem_rdonly_untrusted/mixed_mem_untrusted_btf_id_type:OK
  #242/11  mem_rdonly_untrusted/mixed_mem_btf_id_type:OK
  #242/12  mem_rdonly_untrusted/mixed_rdonly_mem_btf_id_type:OK
  #242/13  mem_rdonly_untrusted/mixed_mem_mem_type:OK
  #242/14  mem_rdonly_untrusted/mixed_map_value_mem_type:OK
  #242/15  mem_rdonly_untrusted/mixed_stack_mem_type:OK
  #242/16  mem_rdonly_untrusted/diff_size_access:OK
  #242/17  mem_rdonly_untrusted/misaligned_access:OK
  #242/18  mem_rdonly_untrusted/null_check:OK
  #242/19  mem_rdonly_untrusted/ldx_is_ok_commuted_addr:OK
  #242/20  mem_rdonly_untrusted/helper_param_not_ok:OK
  #242     mem_rdonly_untrusted:OK
  Summary: 1/20 PASSED, 0 SKIPPED, 0/0 FAILED

Signed-off-by: Daniel Borkmann <[email protected]>
Acked-by: Eduard Zingerman <[email protected]>
---
 v2->v3:
  - address GCC-BPF failure (Eduard, CI)

 .../bpf/progs/mem_rdonly_untrusted.c          | 234 ++++++++++++++++++
 1 file changed, 234 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
index b91271d4caa4..3e0d4f687aaa 100644
--- a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
+++ b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
@@ -3,6 +3,7 @@
 #include <vmlinux.h>
 #include <bpf/bpf_core_read.h>
 #include "bpf_misc.h"
+#include "bpf_kfuncs.h"
 #include "../test_kmods/bpf_testmod_kfunc.h"
 
 SEC("tp_btf/sys_enter")
@@ -164,6 +165,239 @@ int mixed_mem_type(void *ctx)
 	return *p;
 }
 
+struct {
+	__uint(type, BPF_MAP_TYPE_RINGBUF);
+	__uint(max_entries, 4096);
+} ringbuf SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, u32);
+	__type(value, u64);
+} array SEC(".maps");
+
+char dynptr_data[8];
+
+int zero;
+
+SEC("socket")
+__success
+__log_level(2)
+__msg("r8 = *(u64 *)(r7 +0){{.*}}R7=untrusted_ptr_sock")
+__msg("r8 = *(u64 *)(r7 +0){{.*}}R7=ringbuf_mem")
+__retval(0)
+int mixed_mem_untrusted_btf_id_type(void *ctx)
+{
+	u64 *p, *q, v;
+
+	p = bpf_ringbuf_reserve(&ringbuf, sizeof(*p), 0);
+	if (!p)
+		return 1;
+	*p = 42;
+	q = bpf_rdonly_cast(0, bpf_core_type_id_kernel(struct sock));
+	/*
+	 * The load below is reached with PTR_TO_MEM | MEM_RINGBUF on one
+	 * path and with PTR_TO_BTF_ID | PTR_UNTRUSTED on the other. The
+	 * merged type has to keep the BPF_PROBE_MEM rewrite, otherwise
+	 * the NULL deref taken at runtime panics the kernel instead of
+	 * returning 0.
+	 */
+	asm volatile (
+	"r7 = %[p];"
+	"if %[zero] != 0 goto +1;"
+	"r7 = %[q];"
+	"r8 = *(u64 *)(r7 + 0);"
+	"%[v] = r8;"
+	: [v]"=r"(v)
+	: [p]"r"(p),
+	  [q]"r"(q),
+	  [zero]"r"(zero)
+	: "r7", "r8");
+	bpf_ringbuf_discard(p, 0);
+	return v;
+}
+
+SEC("socket")
+__success
+__log_level(2)
+__msg("r8 = *(u32 *)(r7 +0){{.*}}R7=ptr_nameidata")
+__msg("r8 = *(u32 *)(r7 +0){{.*}}R7=ringbuf_mem")
+__retval(0)
+int mixed_mem_btf_id_type(void *ctx)
+{
+	struct task_struct *task;
+	u32 *p, *q;
+	u64 v;
+
+	p = bpf_ringbuf_reserve(&ringbuf, sizeof(*p), 0);
+	if (!p)
+		return 1;
+	*p = 42;
+	task = bpf_get_current_task_btf();
+	/*
+	 * A plain BTF pointer walk yields a bare PTR_TO_BTF_ID, and
+	 * task->nameidata is NULL unless the task currently is in the
+	 * middle of a path lookup.
+	 */
+	q = (u32 *)&task->nameidata->flags;
+	/*
+	 * Same as above, except that the other path yields a bare
+	 * PTR_TO_BTF_ID. Merging it with PTR_TO_MEM used to drop the
+	 * BPF_PROBE_MEM rewrite the bare PTR_TO_BTF_ID would have
+	 * gotten on its own.
+	 */
+	asm volatile (
+	"r7 = %[p];"
+	"if %[zero] != 0 goto +1;"
+	"r7 = %[q];"
+	"r8 = *(u32 *)(r7 + 0);"
+	"%[v] = r8;"
+	: [v]"=r"(v)
+	: [p]"r"(p),
+	  [q]"r"(q),
+	  [zero]"r"(zero)
+	: "r7", "r8");
+	bpf_ringbuf_discard(p, 0);
+	return v;
+}
+
+SEC("socket")
+__success
+__log_level(2)
+__msg("r8 = *(u32 *)(r7 +0){{.*}}R7=ptr_nameidata")
+__msg("r8 = *(u32 *)(r7 +0){{.*}}R7=rdonly_mem")
+__retval(0)
+int mixed_rdonly_mem_btf_id_type(void *ctx)
+{
+	struct task_struct *task;
+	struct bpf_dynptr dptr;
+	char buf[sizeof(u32)];
+	u32 *p, *q;
+	u64 v;
+
+	if (bpf_dynptr_from_mem(dynptr_data, sizeof(dynptr_data), 0, &dptr))
+		return 1;
+	p = bpf_dynptr_slice(&dptr, 0, buf, sizeof(buf));
+	if (!p)
+		return 1;
+	task = bpf_get_current_task_btf();
+	q = (u32 *)&task->nameidata->flags;
+	/*
+	 * Same as above, except that the PTR_TO_MEM side already carries
+	 * MEM_RDONLY. Merging it with a bare PTR_TO_BTF_ID used to yield
+	 * PTR_TO_MEM | MEM_RDONLY, which is not rewritten either since
+	 * only its PTR_UNTRUSTED variant is.
+	 */
+	asm volatile (
+	"r7 = %[p];"
+	"if %[zero] != 0 goto +1;"
+	"r7 = %[q];"
+	"r8 = *(u32 *)(r7 + 0);"
+	"%[v] = r8;"
+	: [v]"=r"(v)
+	: [p]"r"(p),
+	  [q]"r"(q),
+	  [zero]"r"(zero)
+	: "r7", "r8");
+	return v;
+}
+
+SEC("socket")
+__success
+__log_level(2)
+__msg("r8 = *(u64 *)(r7 +0){{.*}}R7=ringbuf_mem")
+__msg("r8 = *(u64 *)(r7 +0){{.*}}R7=rdonly_untrusted_mem")
+__retval(0)
+int mixed_mem_mem_type(void *ctx)
+{
+	u64 *p, *q, v;
+
+	p = bpf_ringbuf_reserve(&ringbuf, sizeof(*p), 0);
+	if (!p)
+		return 1;
+	*p = 42;
+	q = bpf_rdonly_cast(0, 0);
+	/*
+	 * Both paths are PTR_TO_MEM based, so they used to not trip the
+	 * type mismatch check and skipped the merge altogether, leaving
+	 * the insn with the PTR_TO_MEM | MEM_RINGBUF recorded first and
+	 * hence without the BPF_PROBE_MEM rewrite the other path needs.
+	 */
+	asm volatile (
+	"r7 = %[q];"
+	"if %[zero] == 0 goto +1;"
+	"r7 = %[p];"
+	"r8 = *(u64 *)(r7 + 0);"
+	"%[v] = r8;"
+	: [v]"=r"(v)
+	: [p]"r"(p),
+	  [q]"r"(q),
+	  [zero]"r"(zero)
+	: "r7", "r8");
+	bpf_ringbuf_discard(p, 0);
+	return v;
+}
+
+SEC("socket")
+__failure
+__msg("same insn cannot be used with different pointers")
+int mixed_map_value_mem_type(void *ctx)
+{
+	u64 *p, *q, v;
+	u32 key = 0;
+
+	p = bpf_map_lookup_elem(&array, &key);
+	if (!p)
+		return 1;
+	q = bpf_rdonly_cast(0, 0);
+	/*
+	 * PTR_TO_MAP_VALUE is neither PTR_TO_MEM nor PTR_TO_BTF_ID based,
+	 * so it cannot be merged into a type which keeps the BPF_PROBE_MEM
+	 * rewrite the PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED of the other
+	 * path needs. Both bases were mismatch ok, hence the load used to be
+	 * accepted with the PTR_TO_MAP_VALUE recorded and the NULL deref on
+	 * the second path panicked the kernel.
+	 */
+	asm volatile (
+	"r7 = %[q];"
+	"if %[zero] == 0 goto +1;"
+	"r7 = %[p];"
+	"r8 = *(u64 *)(r7 + 0);"
+	"%[v] = r8;"
+	: [v]"=r"(v)
+	: [p]"r"(p),
+	  [q]"r"(q),
+	  [zero]"r"(zero)
+	: "r7", "r8");
+	return v;
+}
+
+SEC("socket")
+__failure
+__msg("same insn cannot be used with different pointers")
+int mixed_stack_mem_type(void *ctx)
+{
+	u64 *p = bpf_rdonly_cast(0, 0);
+	u64 s = 42, v;
+
+	/*
+	 * Same as above, but for a PTR_TO_STACK on the other path.
+	 */
+	asm volatile (
+	"r7 = %[p];"
+	"if %[zero] == 0 goto +1;"
+	"r7 = %[s];"
+	"r8 = *(u64 *)(r7 + 0);"
+	"%[v] = r8;"
+	: [v]"=r"(v)
+	: [p]"r"(p),
+	  [s]"r"(&s),
+	  [zero]"r"(zero)
+	: "r7", "r8");
+	return v;
+}
+
 __attribute__((__aligned__(8)))
 u8 global[] = {
 	0x11, 0x22, 0x33, 0x44,
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.