[RFC PATCH bpf-next 0/3] compiler attribute: define __user as __attribute__((btf_type_tag("user")))

[RFC PATCH bpf-next 0/3] compiler attribute: define __user as __attribute__((btf_type_tag("user")))

[Yonghong Song]

Latest clang compiler supports a type attribute like
  __attribute__((btf_type_tag("<arbitrary string>")))
which can be used in places like __user/__rcu.
With the above attribute, clang was able to preserve the "<arbitrary string>"
info in dwarf and subsequently BTF. For example __user/__rcu
attribute can be preserved in BTF, which currently is loaded inside
the kernel. Such information can then be used by verifier to check
bpf program memory access conforms to the access attribute.
Please see Patches 1 and 2 for details.

This is a RFC patch as it depends on pahole patch and
a new pahole version. The following is pahole patch link:
  https://lore.kernel.org/bpf/20211117202214.3268824-1-yhs@fb.com/
Second, for bpf verifier use of this new __user tag information,
I only implemented support to check function parameter
dereference. More work will be needed to check other
non function parameter memory accesses.

Yonghong Song (3):
  compiler_types: define __user as __attribute__((btf_type_tag("user")))
  bpf: reject program if a __user tagged memory accessed in kernel way
  selftests/bpf: add a selftest with __user tag

 include/linux/bpf.h                           |  1 +
 include/linux/bpf_verifier.h                  |  1 +
 include/linux/btf.h                           |  5 ++++
 include/linux/compiler_types.h                |  2 ++
 kernel/bpf/btf.c                              | 15 +++++++++---
 kernel/bpf/verifier.c                         | 16 ++++++++++---
 lib/Kconfig.debug                             |  5 ++++
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  9 +++++++
 .../selftests/bpf/prog_tests/btf_tag.c        | 23 ++++++++++++++++++
 .../selftests/bpf/progs/btf_type_tag_user.c   | 24 +++++++++++++++++++
 10 files changed, 95 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/btf_type_tag_user.c

-- 
2.30.2

[RFC PATCH bpf-next 1/3] compiler_types: define __user as __attribute__((btf_type_tag("user")))

[Yonghong Song]

If pahole and compiler supports btf_type_tag attributes,
during kernel build, we can define __user as
__attribute__((btf_type_tag("user"))). This will encode __user
information in BTF. Such information, encoded in BTF
as BTF_KIND_TYPE_TAG, can help bpf verifier to
ensure proper memory dereference mechanism depending
on user memory or kernel memory.

The encoded __user info is also useful for other tracing
facility where instead of to require user to specify
kernel/user address type, the kernel can detect it
by itself with btf.

The following is an example with latest upstream clang
(clang14, [1]) and latest pahole:
  [$ ~] cat test.c
  #define __tag1 __attribute__((btf_type_tag("tag1")))
  int foo(int __tag1 *arg) {
          return *arg;
  }
  [$ ~] clang -O2 -g -c test.c
  [$ ~] pahole -JV test.o
  ...
  [1] INT int size=4 nr_bits=32 encoding=SIGNED
  [2] TYPE_TAG tag1 type_id=1
  [3] PTR (anon) type_id=2
  [4] FUNC_PROTO (anon) return=1 args=(3 arg)
  [5] FUNC foo type_id=4
  [$ ~]

You can see for the function argument "int __tag1 *arg",
its type is described as
  PTR -> TYPE_TAG(tag1) -> INT

The kernel can take advantage of this information
to bpf verification or other use cases.

Current btf_type_tag is only supported in clang (>= clang14).
gcc support is also proposed and under development ([2]).

  [1] https://reviews.llvm.org/D111199
  [2] https://www.spinics.net/lists/bpf/msg45773.html

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/compiler_types.h | 2 ++
 lib/Kconfig.debug              | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1d32f4c03c9e..65725e183a38 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -31,6 +31,8 @@ static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
 # define __kernel
 # ifdef STRUCTLEAK_PLUGIN
 #  define __user	__attribute__((user))
+# elif defined(CONFIG_PAHOLE_HAS_BTF_TAG) && __has_attribute(btf_type_tag)
+#  define __user	__attribute__((btf_type_tag("user")))
 # else
 #  define __user
 # endif
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ef7ce18b4f5..bf21b501c66d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -324,6 +324,11 @@ config DEBUG_INFO_BTF
 config PAHOLE_HAS_SPLIT_BTF
 	def_bool $(success, test `$(PAHOLE) --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'` -ge "119")
 
+config PAHOLE_HAS_BTF_TAG
+	depends on DEBUG_INFO_BTF
+	depends on CC_IS_CLANG
+	def_bool $(success, test `$(PAHOLE) --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'` -ge "122")
+
 config DEBUG_INFO_BTF_MODULES
 	def_bool y
 	depends on DEBUG_INFO_BTF && MODULES && PAHOLE_HAS_SPLIT_BTF
-- 
2.30.2

[RFC PATCH bpf-next 2/3] bpf: reject program if a __user tagged memory accessed in kernel way

[Yonghong Song]

BPF verifier supports direct access, e.g., a->b. If "a" is a pointer
pointing to kernel memory, bpf verifier will allow user to write
code in C like a->b and bpf verifier will translate it to a kernel
load properly. If "a" is a pointer to user memory, it is expected
that bpf developer should be bpf_probe_read_user() helper to
get the value a->b. In the current mechanism, if "a" is a user pointer,
a->b access may trigger a page fault and the verifier generated
code will simulate bpf_probe_read() and return 0 for a->b, which
may not be correct value.

Now BTF contains __user information, it can check whether the
pointer points to a user memory or not. If it is, the verifier
can reject the program and force users to use bpf_probe_read_user()
helper explicitly.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/bpf.h          |  1 +
 include/linux/bpf_verifier.h |  1 +
 include/linux/btf.h          |  5 +++++
 kernel/bpf/btf.c             | 15 ++++++++++++---
 kernel/bpf/verifier.c        | 16 +++++++++++++---
 5 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index cc7a0c36e7df..d09df9ec3100 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -473,6 +473,7 @@ struct bpf_insn_access_aux {
 		struct {
 			struct btf *btf;
 			u32 btf_id;
+			bool is_user;
 		};
 	};
 	struct bpf_verifier_log *log; /* for verbose logs */
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index c8a78e830fca..2ddba4767118 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -66,6 +66,7 @@ struct bpf_reg_state {
 		struct {
 			struct btf *btf;
 			u32 btf_id;
+			bool is_user;
 		};
 
 		u32 mem_size; /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 203eef993d76..fcb6041f8fff 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -169,6 +169,11 @@ static inline bool btf_type_is_var(const struct btf_type *t)
 	return BTF_INFO_KIND(t->info) == BTF_KIND_VAR;
 }
 
+static inline bool btf_type_is_type_tag(const struct btf_type *t)
+{
+	return BTF_INFO_KIND(t->info) == BTF_KIND_TYPE_TAG;
+}
+
 /* union is only a special case of struct:
  * all its offsetof(member) == 0
  */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 6b9d23be1e99..dadf1680a677 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4999,6 +4999,14 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 	info->btf = btf;
 	info->btf_id = t->type;
 	t = btf_type_by_id(btf, t->type);
+
+	if (btf_type_is_type_tag(t)) {
+		const char *tag_value = __btf_name_by_offset(btf, t->name_off);
+
+		if (strcmp(tag_value, "user") == 0)
+			info->is_user = true;
+	}
+
 	/* skip modifiers */
 	while (btf_type_is_modifier(t)) {
 		info->btf_id = t->type;
@@ -5010,8 +5018,9 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 			tname, arg, btf_kind_str[BTF_INFO_KIND(t->info)]);
 		return false;
 	}
-	bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n",
-		tname, arg, info->btf_id, btf_kind_str[BTF_INFO_KIND(t->info)],
+	bpf_log(log, "func '%s' arg%d has btf_id %d is_user %d type %s '%s'\n",
+		tname, arg, info->btf_id, info->is_user,
+		btf_kind_str[BTF_INFO_KIND(t->info)],
 		__btf_name_by_offset(btf, t->name_off));
 	return true;
 }
@@ -5030,7 +5039,7 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 	u32 i, moff, mtrue_end, msize = 0, total_nelems = 0;
 	const struct btf_type *mtype, *elem_type = NULL;
 	const struct btf_member *member;
-	const char *tname, *mname;
+	const char *tname, *mname, *tag_value;
 	u32 vlen, elem_id, mid;
 
 again:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0763cca139a7..07ba7c8f6aa3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -647,7 +647,8 @@ static void print_verifier_state(struct bpf_verifier_env *env,
 			if (t == PTR_TO_BTF_ID ||
 			    t == PTR_TO_BTF_ID_OR_NULL ||
 			    t == PTR_TO_PERCPU_BTF_ID)
-				verbose(env, "%s", kernel_type_name(reg->btf, reg->btf_id));
+				verbose(env, "%s,is_user=%d", kernel_type_name(reg->btf, reg->btf_id),
+					reg->is_user);
 			verbose(env, "(id=%d", reg->id);
 			if (reg_type_may_be_refcounted_or_null(t))
 				verbose(env, ",ref_obj_id=%d", reg->ref_obj_id);
@@ -3551,7 +3552,7 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off,
 /* check access to 'struct bpf_context' fields.  Supports fixed offsets only */
 static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off, int size,
 			    enum bpf_access_type t, enum bpf_reg_type *reg_type,
-			    struct btf **btf, u32 *btf_id)
+			    struct btf **btf, u32 *btf_id, bool *is_user)
 {
 	struct bpf_insn_access_aux info = {
 		.reg_type = *reg_type,
@@ -3572,6 +3573,7 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
 		if (*reg_type == PTR_TO_BTF_ID || *reg_type == PTR_TO_BTF_ID_OR_NULL) {
 			*btf = info.btf;
 			*btf_id = info.btf_id;
+			*is_user = info.is_user;
 		} else {
 			env->insn_aux_data[insn_idx].ctx_field_size = info.ctx_field_size;
 		}
@@ -4116,6 +4118,13 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
+	if (reg->is_user) {
+		verbose(env,
+			"R%d is ptr_%s access user memory: off=%d\n",
+			regno, tname, off);
+		return -EACCES;
+	}
+
 	if (env->ops->btf_struct_access) {
 		ret = env->ops->btf_struct_access(&env->log, reg->btf, t,
 						  off, size, atype, &btf_id);
@@ -4374,7 +4383,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 		if (err < 0)
 			return err;
 
-		err = check_ctx_access(env, insn_idx, off, size, t, &reg_type, &btf, &btf_id);
+		err = check_ctx_access(env, insn_idx, off, size, t, &reg_type, &btf, &btf_id, &is_user);
 		if (err)
 			verbose_linfo(env, insn_idx, "; ");
 		if (!err && t == BPF_READ && value_regno >= 0) {
@@ -4399,6 +4408,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 				    reg_type == PTR_TO_BTF_ID_OR_NULL) {
 					regs[value_regno].btf = btf;
 					regs[value_regno].btf_id = btf_id;
+					regs[value_regno].is_user = is_user;
 				}
 			}
 			regs[value_regno].type = reg_type;
-- 
2.30.2

[RFC PATCH bpf-next 3/3] selftests/bpf: add a selftest with __user tag

[Yonghong Song]

Added a selftest where the argument is a pointer with __user tag.
Directly accessing its field without helper will result
verification failure.
  $ ./test_progs -v -n 21/3
  ...
  Successfully loaded bpf_testmod.ko.
  test_btf_type_tag_user:PASS:btf_type_tag_user 0 nsec
  libbpf: load bpf program failed: Permission denied
  libbpf: -- BEGIN DUMP LOG ---
  libbpf:
  R1 type=ctx expected=fp
  ; int BPF_PROG(sub, struct bpf_testmod_btf_type_tag *arg)
  0: (79) r1 = *(u64 *)(r1 +0)
  func 'bpf_testmod_test_btf_type_tag_user' arg0 accesses user memory
  invalid bpf_context access off=0 size=8
  processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
  ...
  test_btf_type_tag_user:PASS:btf_type_tag_user 0 nsec
  #21/3 btf_tag/btf_type_tag_user:OK
  #21 btf_tag:OK
  Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
  Successfully unloaded bpf_testmod.ko.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  9 +++++++
 .../selftests/bpf/prog_tests/btf_tag.c        | 23 ++++++++++++++++++
 .../selftests/bpf/progs/btf_type_tag_user.c   | 24 +++++++++++++++++++
 3 files changed, 56 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/btf_type_tag_user.c

diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 5d52ea2768df..5377dd2959bb 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -21,6 +21,15 @@ bpf_testmod_test_mod_kfunc(int i)
 	*(int *)this_cpu_ptr(&bpf_testmod_ksym_percpu) = i;
 }
 
+struct bpf_testmod_btf_type_tag {
+	int a;
+};
+
+noinline int
+bpf_testmod_test_btf_type_tag_user(struct bpf_testmod_btf_type_tag __user *arg) {
+	return arg->a;
+}
+
 noinline int bpf_testmod_loop_test(int n)
 {
 	int i, sum = 0;
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_tag.c b/tools/testing/selftests/bpf/prog_tests/btf_tag.c
index 88d63e23e35f..1e4003fa7cd0 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_tag.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_tag.c
@@ -8,6 +8,7 @@ struct btf_type_tag_test {
         int **p;
 };
 #include "btf_type_tag.skel.h"
+#include "btf_type_tag_user.skel.h"
 
 static void test_btf_decl_tag(void)
 {
@@ -41,10 +42,32 @@ static void test_btf_type_tag(void)
 	btf_type_tag__destroy(skel);
 }
 
+static void test_btf_type_tag_user(void)
+{
+	struct btf_type_tag_user *skel;
+	int err;
+
+	skel = btf_type_tag_user__open();
+	if (!ASSERT_OK_PTR(skel, "btf_type_tag_user"))
+		return;
+
+	if (skel->rodata->skip_tests) {
+		printf("%s:SKIP: btf_type_tag attribute not supported", __func__);
+		test__skip();
+	} else {
+		err = btf_type_tag_user__load(skel);
+		ASSERT_ERR(err, "btf_type_tag_user");
+	}
+
+	btf_type_tag_user__destroy(skel);
+}
+
 void test_btf_tag(void)
 {
 	if (test__start_subtest("btf_decl_tag"))
 		test_btf_decl_tag();
 	if (test__start_subtest("btf_type_tag"))
 		test_btf_type_tag();
+	if (test__start_subtest("btf_type_tag_user"))
+		test_btf_type_tag_user();
 }
diff --git a/tools/testing/selftests/bpf/progs/btf_type_tag_user.c b/tools/testing/selftests/bpf/progs/btf_type_tag_user.c
new file mode 100644
index 000000000000..41ab1ddd2cf5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf_type_tag_user.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+#if __has_attribute(btf_type_tag)
+volatile const bool skip_tests = false;
+#else
+volatile const bool skip_tests = true;
+#endif
+
+struct bpf_testmod_btf_type_tag {
+	int a;
+};
+
+int g;
+
+SEC("fentry/bpf_testmod_test_btf_type_tag_user")
+int BPF_PROG(sub, struct bpf_testmod_btf_type_tag *arg)
+{
+  g = arg->a;
+  return 0;
+}
-- 
2.30.2