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

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

[Daniel Borkmann]

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 <daniel@iogearbox.net>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
---
 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

[PATCH bpf-next v3 2/3] selftests/bpf: Add tests for fault prone loads out of RCU pointers

[Daniel Borkmann]

Cover the two loads which used to lose the BPF_PROBE_MEM rewrite, both reached
from an RCU read-side critical section. The purpose of this patch is to assert
load success in order to make sure to not trigger verifier_bug_if() on
bpf_may_fault_on_deref() due to forgotten rewrite of a probed pointer.

  # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t rcu_read_lock
  [...]
  #332/1   rcu_read_lock/success:OK
  #332/2   rcu_read_lock/rcuptr_acquire:OK
  #332/3   rcu_read_lock/negative_tests_inproper_region:OK
  #332/4   rcu_read_lock/negative_tests_rcuptr_misuse:OK
  #332     rcu_read_lock:OK
  Summary: 1/4 PASSED, 0 SKIPPED, 0/0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../selftests/bpf/prog_tests/rcu_read_lock.c  |  2 +
 .../selftests/bpf/progs/rcu_read_lock.c       | 76 +++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c b/tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c
index 246eb259c08a..6a07b2b418d1 100644
--- a/tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c
@@ -34,6 +34,8 @@ static void test_success(void)
 	bpf_program__set_autoload(skel->progs.rcu_read_lock_global_subprog, true);
 	bpf_program__set_autoload(skel->progs.rcu_read_lock_subprog_lock, true);
 	bpf_program__set_autoload(skel->progs.rcu_read_lock_subprog_unlock, true);
+	bpf_program__set_autoload(skel->progs.non_own_ref_untrusted_ld, true);
+	bpf_program__set_autoload(skel->progs.rcu_untrusted_union_ld, true);
 	err = rcu_read_lock__load(skel);
 	if (!ASSERT_OK(err, "skel_load"))
 		goto out;
diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c
index b4e073168fb1..31d4081c3a9f 100644
--- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c
+++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c
@@ -549,3 +549,79 @@ int rcu_read_lock_sleepable_global_subprog_indirect(void *ctx)
 	bpf_rcu_read_unlock();
 	return 0;
 }
+
+struct rcu_node_data {
+	long key;
+	struct bpf_rb_node node;
+};
+
+struct rcu_node_stash {
+	struct rcu_node_data __kptr *node;
+};
+
+/*
+ * Necessary so that LLVM emits BTF for rcu_node_data rather than just a
+ * fwd reference to it, same as in progs/local_kptr_stash.c.
+ */
+struct rcu_node_data *just_here_because_btf_bug;
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, struct rcu_node_stash);
+} node_stash SEC(".maps");
+
+long non_own_ref_key;
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+int non_own_ref_untrusted_ld(void *ctx)
+{
+	struct rcu_node_stash *stash;
+	struct rcu_node_data *node;
+	int key = 0;
+
+	stash = bpf_map_lookup_elem(&node_stash, &key);
+	if (!stash)
+		return 0;
+	bpf_rcu_read_lock();
+	node = stash->node;
+	if (!node) {
+		bpf_rcu_read_unlock();
+		return 0;
+	}
+	bpf_rcu_read_unlock();
+	/*
+	 * The unlock leaves node as PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED
+	 * | NON_OWN_REF, and the load below has to get the BPF_PROBE_MEM
+	 * rewrite for it, otherwise a bad address panics the kernel.
+	 */
+	non_own_ref_key = node->key;
+	return 0;
+}
+
+long rcu_untrusted_wq_flags;
+
+SEC("?tp_btf/tcp_probe")
+int BPF_PROG(rcu_untrusted_union_ld, struct sock *sk)
+{
+	struct socket_wq *wq;
+
+	/*
+	 * sk_wq sits in a two member union, so btf_struct_walk() marks the
+	 * pointer PTR_UNTRUSTED, and the __rcu tag on the member adds MEM_RCU
+	 * on top of it. struct sock is not on the __safe_rcu_or_null allow
+	 * list, hence the two stay combined and the load below has to get the
+	 * BPF_PROBE_MEM rewrite for PTR_TO_BTF_ID | PTR_UNTRUSTED | MEM_RCU,
+	 * otherwise a bad address panics the kernel.
+	 *
+	 * The __rcu tag only reaches BTF on a clang built kernel, that is, one
+	 * with CONFIG_PAHOLE_HAS_BTF_TAG. On a gcc built kernel the walk yields
+	 * a plain untrusted pointer, which is rewritten either way.
+	 */
+	wq = sk->sk_wq;
+	if (!wq)
+		return 0;
+	rcu_untrusted_wq_flags = wq->flags;
+	return 0;
+}
-- 
2.43.0

[PATCH bpf-next v3 3/3] selftests/bpf: Add tests for a store on a fault prone qdisc pointer

[Daniel Borkmann]

Cover the store which used to be left as a plain BPF_STX without an
exception table entry:

  1: R1=trusted_ptr_Qdisc()
  ; struct Qdisc *next = sch->next_sched;
  1: (79) r1 = *(u64 *)(r1 +216)        ; R1=ptr_Qdisc()
  ; next->limit = 1000;
  3: (63) *(u32 *)(r1 +20) = r2         ; R1=ptr_Qdisc() R2=1000

Assert that it is rejected now.

  # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t ns_bpf_qdisc
  [...]
  #257/1   ns_bpf_qdisc/fifo:OK
  #257/2   ns_bpf_qdisc/fq:OK
  #257/3   ns_bpf_qdisc/attach to mq:OK
  #257/4   ns_bpf_qdisc/attach to non root:OK
  #257/5   ns_bpf_qdisc/incompl_ops:OK
  #257/6   ns_bpf_qdisc/invalid_dynptr:OK
  #257/7   ns_bpf_qdisc/invalid_dynptr_cross_frame:OK
  #257/8   ns_bpf_qdisc/invalid_dynptr_slice:OK
  #257/9   ns_bpf_qdisc/untrusted_write:OK
  #257/10  ns_bpf_qdisc/dynptr_use_after_invalidate_clone:OK
  #257     ns_bpf_qdisc:OK
  Summary: 1/10 PASSED, 0 SKIPPED, 0/0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 v2->v3:
  - new patch

 .../selftests/bpf/prog_tests/bpf_qdisc.c      |  2 +
 .../progs/bpf_qdisc_fail__untrusted_write.c   | 65 +++++++++++++++++++
 2 files changed, 67 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/bpf_qdisc_fail__untrusted_write.c

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
index 77f1c0550c9b..6dbd1487343c 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_qdisc.c
@@ -11,6 +11,7 @@
 #include "bpf_qdisc_fail__invalid_dynptr.skel.h"
 #include "bpf_qdisc_fail__invalid_dynptr_slice.skel.h"
 #include "bpf_qdisc_fail__invalid_dynptr_cross_frame.skel.h"
+#include "bpf_qdisc_fail__untrusted_write.skel.h"
 #include "bpf_qdisc_dynptr_use_after_invalidate_clone.skel.h"
 
 #define LO_IFINDEX 1
@@ -230,6 +231,7 @@ void test_ns_bpf_qdisc(void)
 	RUN_TESTS(bpf_qdisc_fail__invalid_dynptr);
 	RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_cross_frame);
 	RUN_TESTS(bpf_qdisc_fail__invalid_dynptr_slice);
+	RUN_TESTS(bpf_qdisc_fail__untrusted_write);
 	RUN_TESTS(bpf_qdisc_dynptr_use_after_invalidate_clone);
 }
 
diff --git a/tools/testing/selftests/bpf/progs/bpf_qdisc_fail__untrusted_write.c b/tools/testing/selftests/bpf/progs/bpf_qdisc_fail__untrusted_write.c
new file mode 100644
index 000000000000..688c2a049ae3
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_qdisc_fail__untrusted_write.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include "bpf_experimental.h"
+#include "bpf_qdisc_common.h"
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("struct_ops")
+__failure __msg("only read is supported")
+int BPF_PROG(untrusted_write, struct sk_buff *skb, struct Qdisc *sch,
+	     struct bpf_sk_buff_ptr *to_free)
+{
+	struct Qdisc *next = sch->next_sched;
+
+	/*
+	 * sch is trusted, but the walk of next_sched yields a plain
+	 * PTR_TO_BTF_ID which may fault on a dereference. A store through
+	 * it does not get an exception table entry, there is no probed
+	 * store to rewrite it into, hence it has to be rejected before
+	 * bpf_qdisc_btf_struct_access() gets to allow the write to limit.
+	 */
+	next->limit = 1000;
+
+	bpf_qdisc_skb_drop(skb, to_free);
+	return NET_XMIT_DROP;
+}
+
+SEC("struct_ops")
+__auxiliary
+struct sk_buff *BPF_PROG(bpf_qdisc_test_dequeue, struct Qdisc *sch)
+{
+	return NULL;
+}
+
+SEC("struct_ops")
+__auxiliary
+int BPF_PROG(bpf_qdisc_test_init, struct Qdisc *sch, struct nlattr *opt,
+	     struct netlink_ext_ack *extack)
+{
+	return 0;
+}
+
+SEC("struct_ops")
+__auxiliary
+void BPF_PROG(bpf_qdisc_test_reset, struct Qdisc *sch)
+{
+}
+
+SEC("struct_ops")
+__auxiliary
+void BPF_PROG(bpf_qdisc_test_destroy, struct Qdisc *sch)
+{
+}
+
+SEC(".struct_ops")
+struct Qdisc_ops test = {
+	.enqueue   = (void *)untrusted_write,
+	.dequeue   = (void *)bpf_qdisc_test_dequeue,
+	.init      = (void *)bpf_qdisc_test_init,
+	.reset     = (void *)bpf_qdisc_test_reset,
+	.destroy   = (void *)bpf_qdisc_test_destroy,
+	.id        = "bpf_qdisc_test",
+};
-- 
2.43.0