[PATCH bpf-next v2 5/6] selftests/bpf: Add tests for pointer type merge at a shared load

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
  [...]
  #238/1   mem_rdonly_untrusted/btf_id_to_ptr_mem:OK
  #238/2   mem_rdonly_untrusted/ldx_is_ok_bad_addr:OK
  #238/3   mem_rdonly_untrusted/ldx_is_ok_good_addr:OK
  #238/4   mem_rdonly_untrusted/offset_not_tracked:OK
  #238/5   mem_rdonly_untrusted/stx_not_ok:OK
  #238/6   mem_rdonly_untrusted/atomic_not_ok:OK
  #238/7   mem_rdonly_untrusted/atomic_rmw_not_ok:OK
  #238/8   mem_rdonly_untrusted/kfunc_param_not_ok:OK
  #238/9   mem_rdonly_untrusted/mixed_mem_type:OK
  #238/10  mem_rdonly_untrusted/mixed_mem_untrusted_btf_id_type:OK
  #238/11  mem_rdonly_untrusted/mixed_mem_btf_id_type:OK
  #238/12  mem_rdonly_untrusted/mixed_rdonly_mem_btf_id_type:OK
  #238/13  mem_rdonly_untrusted/mixed_mem_mem_type:OK
  #238/14  mem_rdonly_untrusted/mixed_map_value_mem_type:OK
  #238/15  mem_rdonly_untrusted/mixed_stack_mem_type:OK
  #238/16  mem_rdonly_untrusted/diff_size_access:OK
  #238/17  mem_rdonly_untrusted/misaligned_access:OK
  #238/18  mem_rdonly_untrusted/null_check:OK
  #238/19  mem_rdonly_untrusted/ldx_is_ok_commuted_addr:OK
  #238/20  mem_rdonly_untrusted/helper_param_not_ok:OK
  #238     mem_rdonly_untrusted:OK
  Summary: 1/20 PASSED, 0 SKIPPED, 0/0 FAILED

Signed-off-by: Daniel Borkmann <[email protected]>
---
 .../bpf/progs/mem_rdonly_untrusted.c          | 254 ++++++++++++++++++
 1 file changed, 254 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..ac8bba3e7d02 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,259 @@ int mixed_mem_type(void *ctx)
 	return *p;
 }
 
+struct {
+	__uint(type, BPF_MAP_TYPE_RINGBUF);
+	__uint(max_entries, 4096);
+} ringbuf SEC(".maps");
+
+int zero;
+
+static __noinline u64 *get_mem_or_untrusted_addr(u64 *mem)
+{
+	/*
+	 * Try to avoid compiler hoisting load to if branches by using
+	 * __noinline func.
+	 */
+	if (zero)
+		return mem;
+	else
+		return bpf_rdonly_cast(0, bpf_core_type_id_kernel(struct sock));
+}
+
+SEC("socket")
+__success
+__log_level(2)
+__msg("= *(u64 *)(r{{[0-9]}} +0){{.*}}=untrusted_ptr_sock")
+__msg("= *(u64 *)(r{{[0-9]}} +0){{.*}}=ringbuf_mem")
+__retval(0)
+int mixed_mem_untrusted_btf_id_type(void *ctx)
+{
+	u64 *p, v;
+
+	p = bpf_ringbuf_reserve(&ringbuf, sizeof(*p), 0);
+	if (!p)
+		return 1;
+	*p = 42;
+	/*
+	 * 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.
+	 */
+	v = *get_mem_or_untrusted_addr(p);
+	bpf_ringbuf_discard(p, 0);
+	return v;
+}
+
+static __noinline u32 *get_mem_or_btf_id_addr(u32 *mem)
+{
+	struct task_struct *task;
+
+	/*
+	 * Try to avoid compiler hoisting load to if branches by using
+	 * __noinline func.
+	 */
+	if (zero)
+		return mem;
+
+	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.
+	 */
+	return (u32 *)&task->nameidata->flags;
+}
+
+SEC("socket")
+__success
+__log_level(2)
+__msg("= *(u32 *)(r{{[0-9]}} +0){{.*}}=ptr_nameidata")
+__msg("= *(u32 *)(r{{[0-9]}} +0){{.*}}=ringbuf_mem")
+__retval(0)
+int mixed_mem_btf_id_type(void *ctx)
+{
+	u32 *p, v;
+
+	p = bpf_ringbuf_reserve(&ringbuf, sizeof(*p), 0);
+	if (!p)
+		return 1;
+	*p = 42;
+	/*
+	 * 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.
+	 */
+	v = *get_mem_or_btf_id_addr(p);
+	bpf_ringbuf_discard(p, 0);
+	return v;
+}
+
+char dynptr_data[8];
+
+static __noinline u32 *get_rdonly_mem_or_btf_id_addr(u32 *mem)
+{
+	struct task_struct *task;
+
+	/*
+	 * Try to avoid compiler hoisting load to if branches by using
+	 * __noinline func.
+	 */
+	if (zero)
+		return mem;
+
+	task = bpf_get_current_task_btf();
+	return (u32 *)&task->nameidata->flags;
+}
+
+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 bpf_dynptr dptr;
+	char buf[sizeof(u32)];
+	u32 *p;
+	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;
+	/*
+	 * 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.
+	 */
+	p = get_rdonly_mem_or_btf_id_addr(p);
+	/* asm block to have reliable match target for __msg. */
+	asm volatile (
+	"r7 = %[p];"
+	"r8 = *(u32 *)(r7 + 0);"
+	"%[v] = r8;"
+	: [v]"=r"(v)
+	: [p]"r"(p)
+	: "r7", "r8");
+	return v;
+}
+
+static __noinline u64 *get_mem_or_rdonly_untrusted_mem_addr(u64 *mem)
+{
+	u64 *p = bpf_rdonly_cast(0, 0);
+
+	/*
+	 * Hoist the cast above the branch so that the PTR_TO_MEM |
+	 * MEM_RINGBUF path is verified first and thus gets its type
+	 * recorded first.
+	 */
+	if (zero == 0)
+		return p;
+	return mem;
+}
+
+SEC("socket")
+__success
+__log_level(2)
+__msg("= *(u64 *)(r{{[0-9]}} +0){{.*}}=ringbuf_mem")
+__msg("= *(u64 *)(r{{[0-9]}} +0){{.*}}=rdonly_untrusted_mem")
+__retval(0)
+int mixed_mem_mem_type(void *ctx)
+{
+	u64 *p, v;
+
+	p = bpf_ringbuf_reserve(&ringbuf, sizeof(*p), 0);
+	if (!p)
+		return 1;
+	*p = 42;
+	/*
+	 * 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.
+	 */
+	v = *get_mem_or_rdonly_untrusted_mem_addr(p);
+	bpf_ringbuf_discard(p, 0);
+	return v;
+}
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, u32);
+	__type(value, u64);
+} array SEC(".maps");
+
+static __noinline u64 *get_map_value_or_rdonly_untrusted_mem_addr(u64 *mem)
+{
+	u64 *p = bpf_rdonly_cast(0, 0);
+
+	/*
+	 * Hoist the cast above the branch so that the PTR_TO_MAP_VALUE
+	 * path is verified first and thus gets its type recorded first.
+	 */
+	if (zero == 0)
+		return p;
+	return mem;
+}
+
+SEC("socket")
+__failure
+__msg("same insn cannot be used with different pointers")
+int mixed_map_value_mem_type(void *ctx)
+{
+	u64 *p, v;
+	u32 key = 0;
+
+	p = bpf_map_lookup_elem(&array, &key);
+	if (!p)
+		return 1;
+	/*
+	 * 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.
+	 */
+	v = *get_map_value_or_rdonly_untrusted_mem_addr(p);
+	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. A
+	 * subprog cannot return one, so the branch and the load have to
+	 * be kept in the same frame, and the asm block is there to have
+	 * the compiler not duplicate the latter.
+	 */
+	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