bpf: add __user tagging support in vmlinux BTF

bpf: add __user tagging support in vmlinux BTF

[Yonghong Song]

The __user attribute is currently mainly used by sparse for type checking.
The attribute indicates whether a memory access is in user memory address
space or not. Such information is important during tracing kernel
internal functions or data structures as accessing user memory often
has different mechanisms compared to accessing kernel memory. For example,
the perf-probe needs explicit command line specification to indicate a
particular argument or string in user-space memory ([1], [2], [3]).
Currently, vmlinux BTF is available in kernel with many distributions.
If __user attribute information is available in vmlinux BTF, the explicit
user memory access information from users will not be necessary as
the kernel can figure it out by itself with vmlinux BTF.

Besides the above possible use for perf/probe, another use case is
for bpf verifier. Currently, for bpf BPF_PROG_TYPE_TRACING type of bpf
programs, users can write direct code like
  p->m1->m2
and "p" could be a function parameter. Without __user information in BTF,
the verifier will assume p->m1 accessing kernel memory and will generate
normal loads. Let us say "p" actually tagged with __user in the source
code.  In such cases, p->m1 is actually accessing user memory and direct
load is not right and may produce incorrect result. For such cases,
bpf_probe_read_user() will be the correct way to read p->m1.

To support encoding __user information in BTF, a new attribute
  __attribute__((btf_type_tag("<arbitrary_string>")))
is implemented in clang ([4]). For example, if we have
  #define __user __attribute__((btf_type_tag("user")))
during kernel compilation, the attribute "user" information will
be preserved in dwarf. After pahole converting dwarf to BTF, __user
information will be available in vmlinux BTF and such information
can be used by bpf verifier, perf/probe or other use cases.

Currently btf_type_tag is only supported in clang (>= clang14) and
pahole (>= 1.23). gcc support is also proposed and under development ([5]).

In the rest of patch set, Patch 1 added support of __user btf_type_tag
during compilation. Patch 2 added bpf verifier support to utilize __user
tag information to reject bpf programs not using proper helper to access
user memories. Patches 3-5 are for bpf selftests which demonstrate verifier
can reject direct user memory accesses.

  [1] http://lkml.kernel.org/r/155789874562.26965.10836126971405890891.stgit@devnote2
  [2] http://lkml.kernel.org/r/155789872187.26965.4468456816590888687.stgit@devnote2
  [3] http://lkml.kernel.org/r/155789871009.26965.14167558859557329331.stgit@devnote2
  [4] https://reviews.llvm.org/D111199
  [5] https://www.spinics.net/lists/bpf/msg45773.html

Yonghong Song (5):
  compiler_types: define __user as __attribute__((btf_type_tag("user")))
  bpf: reject program if a __user tagged memory accessed in kernel way
  selftests/bpf: rename btf_decl_tag.c to test_btf_decl_tag.c
  selftests/bpf: add a selftest with __user tag
  selftests/bpf: specify pahole version requirement for btf_tag test

 include/linux/bpf.h                           | 12 +++-
 include/linux/bpf_verifier.h                  |  1 +
 include/linux/btf.h                           |  5 ++
 include/linux/compiler_types.h                |  3 +
 kernel/bpf/btf.c                              | 40 +++++++++---
 kernel/bpf/verifier.c                         | 35 ++++++++---
 lib/Kconfig.debug                             |  8 +++
 net/bpf/bpf_dummy_struct_ops.c                |  6 +-
 net/ipv4/bpf_tcp_ca.c                         |  6 +-
 tools/testing/selftests/bpf/README.rst        |  1 +
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 18 ++++++
 .../selftests/bpf/prog_tests/btf_tag.c        | 63 +++++++++++++++++--
 .../selftests/bpf/progs/btf_type_tag_user.c   | 29 +++++++++
 .../{btf_decl_tag.c => test_btf_decl_tag.c}   |  0
 14 files changed, 200 insertions(+), 27 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/btf_type_tag_user.c
 rename tools/testing/selftests/bpf/progs/{btf_decl_tag.c => test_btf_decl_tag.c} (100%)

-- 
2.30.2

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

[Yonghong Song]

The __user attribute is currently mainly used by sparse for type checking.
The attribute indicates whether a memory access is in user memory address
space or not. Such information is important during tracing kernel
internal functions or data structures as accessing user memory often
has different mechanisms compared to accessing kernel memory. For example,
the perf-probe needs explicit command line specification to indicate a
particular argument or string in user-space memory ([1], [2], [3]).
Currently, vmlinux BTF is available in kernel with many distributions.
If __user attribute information is available in vmlinux BTF, the explicit
user memory access information from users will not be necessary as
the kernel can figure it out by itself with vmlinux BTF.

Besides the above possible use for perf/probe, another use case is
for bpf verifier. Currently, for bpf BPF_PROG_TYPE_TRACING type of bpf
programs, users can write direct code like
  p->m1->m2
and "p" could be a function parameter. Without __user information in BTF,
the verifier will assume p->m1 accessing kernel memory and will generate
normal loads. Let us say "p" actually tagged with __user in the source
code.  In such cases, p->m1 is actually accessing user memory and direct
load is not right and may produce incorrect result. For such cases,
bpf_probe_read_user() will be the correct way to read p->m1.

To support encoding __user information in BTF, a new attribute
  __attribute__((btf_type_tag("<arbitrary_string>")))
is implemented in clang ([4]). For example, if we have
  #define __user __attribute__((btf_type_tag("user")))
during kernel compilation, the attribute "user" information will
be preserved in dwarf. After pahole converting dwarf to BTF, __user
information will be available in vmlinux BTF.

The following is an example with latest upstream clang (clang14) and
pahole 1.23:

  [$ ~] cat test.c
  #define __user __attribute__((btf_type_tag("user")))
  int foo(int __user *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 user 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 __user *arg", its type is
described as
  PTR -> TYPE_TAG(user) -> INT
The kernel can use this information for bpf verification or other
use cases.

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

  [1] http://lkml.kernel.org/r/155789874562.26965.10836126971405890891.stgit@devnote2
  [2] http://lkml.kernel.org/r/155789872187.26965.4468456816590888687.stgit@devnote2
  [3] http://lkml.kernel.org/r/155789871009.26965.14167558859557329331.stgit@devnote2
  [4] https://reviews.llvm.org/D111199
  [5] https://www.spinics.net/lists/bpf/msg45773.html

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

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 1d32f4c03c9e..67e5d29cd2a1 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -31,6 +31,9 @@ static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
 # define __kernel
 # ifdef STRUCTLEAK_PLUGIN
 #  define __user	__attribute__((user))
+# elif defined(CONFIG_DEBUG_INFO_BTF) && 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..2b7b0bcb494a 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -324,6 +324,14 @@ 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
+	def_bool $(success, test `$(PAHOLE) --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'` -ge "123")
+	depends on CC_IS_CLANG
+	help
+	  Decide whether pahole emits btf_tag attributes (btf_type_tag and
+	  btf_decl_tag) or not. Currently only clang compiler implements
+	  these attributes, so make the config depend on CC_IS_CLANG.
+
 config DEBUG_INFO_BTF_MODULES
 	def_bool y
 	depends on DEBUG_INFO_BTF && MODULES && PAHOLE_HAS_SPLIT_BTF
-- 
2.30.2

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

[Yonghong Song]

BPF verifier supports direct memory access for BPF_PROG_TYPE_TRACING type
of bpf programs, 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 the 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. Without utilizing BTF __user tagging information,
current verifier will assume that a->b is a kernel memory access
and this may generate incorrect result.

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.

Currently, an enum type is used to define __user address space:
  enum btf_addr_space {
       BTF_ADDRSPACE_UNSPEC    = 0,
       BTF_ADDRSPACE_USER      = 1,
  }
In the future, we can easily extend btf_add_space for other
address space tagging, for example, rcu.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/bpf.h            | 12 ++++++++--
 include/linux/bpf_verifier.h   |  1 +
 include/linux/btf.h            |  5 +++++
 kernel/bpf/btf.c               | 40 +++++++++++++++++++++++++++-------
 kernel/bpf/verifier.c          | 35 +++++++++++++++++++++--------
 net/bpf/bpf_dummy_struct_ops.c |  6 +++--
 net/ipv4/bpf_tcp_ca.c          |  6 +++--
 7 files changed, 82 insertions(+), 23 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 8bbf08fbab66..3a18b7b980d4 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -463,6 +463,12 @@ enum bpf_reg_type {
 	__BPF_REG_TYPE_MAX,
 };
 
+/* The pointee address space encoded in BTF. */
+enum btf_addr_space {
+	BTF_ADDRSPACE_UNSPEC	= 0,
+	BTF_ADDRSPACE_USER	= 1,
+};
+
 /* The information passed from prog-specific *_is_valid_access
  * back to the verifier.
  */
@@ -473,6 +479,7 @@ struct bpf_insn_access_aux {
 		struct {
 			struct btf *btf;
 			u32 btf_id;
+			enum btf_addr_space addr_space;
 		};
 	};
 	struct bpf_verifier_log *log; /* for verbose logs */
@@ -519,7 +526,8 @@ struct bpf_verifier_ops {
 				 const struct btf *btf,
 				 const struct btf_type *t, int off, int size,
 				 enum bpf_access_type atype,
-				 u32 *next_btf_id);
+				 u32 *next_btf_id,
+				 enum btf_addr_space *addr_space);
 	bool (*check_kfunc_call)(u32 kfunc_btf_id, struct module *owner);
 };
 
@@ -1701,7 +1709,7 @@ static inline bool bpf_tracing_btf_ctx_access(int off, int size,
 int btf_struct_access(struct bpf_verifier_log *log, const struct btf *btf,
 		      const struct btf_type *t, int off, int size,
 		      enum bpf_access_type atype,
-		      u32 *next_btf_id);
+		      u32 *next_btf_id, enum btf_addr_space *addr_space);
 bool btf_struct_ids_match(struct bpf_verifier_log *log,
 			  const struct btf *btf, u32 id, int off,
 			  const struct btf *need_btf, u32 need_type_id);
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 182b16a91084..698b9837334d 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;
+			enum btf_addr_space addr_space;
 		};
 
 		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 acef6ef28768..56a1074155d9 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -216,6 +216,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 27b7de538697..831445b3d97c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4849,6 +4849,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 	const char *tname = prog->aux->attach_func_name;
 	struct bpf_verifier_log *log = info->log;
 	const struct btf_param *args;
+	const char *tag_value;
 	u32 nr_args, arg;
 	int i, ret;
 
@@ -4998,7 +4999,15 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 
 	info->btf = btf;
 	info->btf_id = t->type;
+	info->addr_space = BTF_ADDRSPACE_UNSPEC;
 	t = btf_type_by_id(btf, t->type);
+
+	if (btf_type_is_type_tag(t)) {
+		tag_value = __btf_name_by_offset(btf, t->name_off);
+		if (strcmp(tag_value, "user") == 0)
+			info->addr_space = BTF_ADDRSPACE_USER;
+	}
+
 	/* skip modifiers */
 	while (btf_type_is_modifier(t)) {
 		info->btf_id = t->type;
@@ -5010,8 +5019,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 addr_space %d type %s '%s'\n",
+		tname, arg, info->btf_id, info->addr_space,
+		btf_kind_str[BTF_INFO_KIND(t->info)],
 		__btf_name_by_offset(btf, t->name_off));
 	return true;
 }
@@ -5025,12 +5035,12 @@ enum bpf_struct_walk_result {
 
 static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 			   const struct btf_type *t, int off, int size,
-			   u32 *next_btf_id)
+			   u32 *next_btf_id, enum btf_addr_space *addr_space)
 {
 	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:
@@ -5214,7 +5224,8 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 		}
 
 		if (btf_type_is_ptr(mtype)) {
-			const struct btf_type *stype;
+			enum btf_addr_space tmp_addr_space = BTF_ADDRSPACE_UNSPEC;
+			const struct btf_type *stype, *t;
 			u32 id;
 
 			if (msize != size || off != moff) {
@@ -5223,9 +5234,19 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 					mname, moff, tname, off, size);
 				return -EACCES;
 			}
+
+			/* check __user tag */
+			t = btf_type_by_id(btf, mtype->type);
+			if (btf_type_is_type_tag(t)) {
+				tag_value = __btf_name_by_offset(btf, t->name_off);
+				if (strcmp(tag_value, "user") == 0)
+					tmp_addr_space = BTF_ADDRSPACE_USER;
+			}
+
 			stype = btf_type_skip_modifiers(btf, mtype->type, &id);
 			if (btf_type_is_struct(stype)) {
 				*next_btf_id = id;
+				*addr_space = tmp_addr_space;
 				return WALK_PTR;
 			}
 		}
@@ -5252,13 +5273,14 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 int btf_struct_access(struct bpf_verifier_log *log, const struct btf *btf,
 		      const struct btf_type *t, int off, int size,
 		      enum bpf_access_type atype __maybe_unused,
-		      u32 *next_btf_id)
+		      u32 *next_btf_id, enum btf_addr_space *addr_space)
 {
+	enum btf_addr_space tmp_addr_space;
 	int err;
 	u32 id;
 
 	do {
-		err = btf_struct_walk(log, btf, t, off, size, &id);
+		err = btf_struct_walk(log, btf, t, off, size, &id, &tmp_addr_space);
 
 		switch (err) {
 		case WALK_PTR:
@@ -5266,6 +5288,7 @@ int btf_struct_access(struct bpf_verifier_log *log, const struct btf *btf,
 			 * we're done.
 			 */
 			*next_btf_id = id;
+			*addr_space = tmp_addr_space;
 			return PTR_TO_BTF_ID;
 		case WALK_SCALAR:
 			return SCALAR_VALUE;
@@ -5309,6 +5332,7 @@ bool btf_struct_ids_match(struct bpf_verifier_log *log,
 			  const struct btf *btf, u32 id, int off,
 			  const struct btf *need_btf, u32 need_type_id)
 {
+	enum btf_addr_space addr_space;
 	const struct btf_type *type;
 	int err;
 
@@ -5320,7 +5344,7 @@ bool btf_struct_ids_match(struct bpf_verifier_log *log,
 	type = btf_type_by_id(btf, id);
 	if (!type)
 		return false;
-	err = btf_struct_walk(log, btf, type, off, 1, &id);
+	err = btf_struct_walk(log, btf, type, off, 1, &id, &addr_space);
 	if (err != WALK_STRUCT)
 		return false;
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1126b75fe650..b4968dcaf80f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -649,7 +649,9 @@ 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,addr_space=%d",
+					kernel_type_name(reg->btf, reg->btf_id),
+					reg->addr_space);
 			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);
@@ -1498,7 +1500,8 @@ static void mark_reg_not_init(struct bpf_verifier_env *env,
 static void mark_btf_ld_reg(struct bpf_verifier_env *env,
 			    struct bpf_reg_state *regs, u32 regno,
 			    enum bpf_reg_type reg_type,
-			    struct btf *btf, u32 btf_id)
+			    struct btf *btf, u32 btf_id,
+			    enum btf_addr_space addr_space)
 {
 	if (reg_type == SCALAR_VALUE) {
 		mark_reg_unknown(env, regs, regno);
@@ -1508,6 +1511,7 @@ static void mark_btf_ld_reg(struct bpf_verifier_env *env,
 	regs[regno].type = PTR_TO_BTF_ID;
 	regs[regno].btf = btf;
 	regs[regno].btf_id = btf_id;
+	regs[regno].addr_space = addr_space;
 }
 
 #define DEF_NOT_SUBREG	(0)
@@ -3553,7 +3557,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, enum btf_addr_space *addr_space)
 {
 	struct bpf_insn_access_aux info = {
 		.reg_type = *reg_type,
@@ -3574,6 +3578,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;
+			*addr_space = info.addr_space;
 		} else {
 			env->insn_aux_data[insn_idx].ctx_field_size = info.ctx_field_size;
 		}
@@ -4099,6 +4104,7 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
 	struct bpf_reg_state *reg = regs + regno;
 	const struct btf_type *t = btf_type_by_id(reg->btf, reg->btf_id);
 	const char *tname = btf_name_by_offset(reg->btf, t->name_off);
+	enum btf_addr_space addr_space;
 	u32 btf_id;
 	int ret;
 
@@ -4118,9 +4124,16 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
+	if (reg->addr_space == BTF_ADDRSPACE_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);
+						  off, size, atype, &btf_id, &addr_space);
 	} else {
 		if (atype != BPF_READ) {
 			verbose(env, "only read is supported\n");
@@ -4128,14 +4141,14 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
 		}
 
 		ret = btf_struct_access(&env->log, reg->btf, t, off, size,
-					atype, &btf_id);
+					atype, &btf_id, &addr_space);
 	}
 
 	if (ret < 0)
 		return ret;
 
 	if (atype == BPF_READ && value_regno >= 0)
-		mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id);
+		mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, addr_space);
 
 	return 0;
 }
@@ -4148,6 +4161,7 @@ static int check_ptr_to_map_access(struct bpf_verifier_env *env,
 {
 	struct bpf_reg_state *reg = regs + regno;
 	struct bpf_map *map = reg->map_ptr;
+	enum btf_addr_space addr_space;
 	const struct btf_type *t;
 	const char *tname;
 	u32 btf_id;
@@ -4185,12 +4199,12 @@ static int check_ptr_to_map_access(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
-	ret = btf_struct_access(&env->log, btf_vmlinux, t, off, size, atype, &btf_id);
+	ret = btf_struct_access(&env->log, btf_vmlinux, t, off, size, atype, &btf_id, &addr_space);
 	if (ret < 0)
 		return ret;
 
 	if (value_regno >= 0)
-		mark_btf_ld_reg(env, regs, value_regno, ret, btf_vmlinux, btf_id);
+		mark_btf_ld_reg(env, regs, value_regno, ret, btf_vmlinux, btf_id, addr_space);
 
 	return 0;
 }
@@ -4362,6 +4376,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 		if (!err && t == BPF_READ && value_regno >= 0)
 			mark_reg_unknown(env, regs, value_regno);
 	} else if (reg->type == PTR_TO_CTX) {
+		enum btf_addr_space addr_space = BTF_ADDRSPACE_UNSPEC;
 		enum bpf_reg_type reg_type = SCALAR_VALUE;
 		struct btf *btf = NULL;
 		u32 btf_id = 0;
@@ -4376,7 +4391,8 @@ 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, &addr_space);
 		if (err)
 			verbose_linfo(env, insn_idx, "; ");
 		if (!err && t == BPF_READ && value_regno >= 0) {
@@ -4401,6 +4417,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].addr_space = addr_space;
 				}
 			}
 			regs[value_regno].type = reg_type;
diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
index fbc896323bec..57db75427a88 100644
--- a/net/bpf/bpf_dummy_struct_ops.c
+++ b/net/bpf/bpf_dummy_struct_ops.c
@@ -145,7 +145,8 @@ static int bpf_dummy_ops_btf_struct_access(struct bpf_verifier_log *log,
 					   const struct btf *btf,
 					   const struct btf_type *t, int off,
 					   int size, enum bpf_access_type atype,
-					   u32 *next_btf_id)
+					   u32 *next_btf_id,
+					   enum btf_addr_space *addr_space)
 {
 	const struct btf_type *state;
 	s32 type_id;
@@ -162,7 +163,8 @@ static int bpf_dummy_ops_btf_struct_access(struct bpf_verifier_log *log,
 		return -EACCES;
 	}
 
-	err = btf_struct_access(log, btf, t, off, size, atype, next_btf_id);
+	err = btf_struct_access(log, btf, t, off, size, atype, next_btf_id,
+				addr_space);
 	if (err < 0)
 		return err;
 
diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
index 67466dbff152..dba60f85369b 100644
--- a/net/ipv4/bpf_tcp_ca.c
+++ b/net/ipv4/bpf_tcp_ca.c
@@ -95,12 +95,14 @@ static int bpf_tcp_ca_btf_struct_access(struct bpf_verifier_log *log,
 					const struct btf *btf,
 					const struct btf_type *t, int off,
 					int size, enum bpf_access_type atype,
-					u32 *next_btf_id)
+					u32 *next_btf_id,
+					enum btf_addr_space *addr_space)
 {
 	size_t end;
 
 	if (atype == BPF_READ)
-		return btf_struct_access(log, btf, t, off, size, atype, next_btf_id);
+		return btf_struct_access(log, btf, t, off, size, atype, next_btf_id,
+					 addr_space);
 
 	if (t != tcp_sock_type) {
 		bpf_log(log, "only read is supported\n");
-- 
2.30.2

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

[Alexei Starovoitov]

On Thu, Dec 09, 2021 at 09:35:48AM -0800, Yonghong Song wrote:
> BPF verifier supports direct memory access for BPF_PROG_TYPE_TRACING type
> of bpf programs, 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 the 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. Without utilizing BTF __user tagging information,
> current verifier will assume that a->b is a kernel memory access
> and this may generate incorrect result.

The patch set looks great overall.

> +/* The pointee address space encoded in BTF. */
> +enum btf_addr_space {
> +	BTF_ADDRSPACE_UNSPEC	= 0,
> +	BTF_ADDRSPACE_USER	= 1,
> +};
> +
>  /* The information passed from prog-specific *_is_valid_access
>   * back to the verifier.
>   */
> @@ -473,6 +479,7 @@ struct bpf_insn_access_aux {
>  		struct {
>  			struct btf *btf;
>  			u32 btf_id;
> +			enum btf_addr_space addr_space;
>  		};
...
> @@ -4998,7 +4999,15 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>  
>  	info->btf = btf;
>  	info->btf_id = t->type;
> +	info->addr_space = BTF_ADDRSPACE_UNSPEC;
>  	t = btf_type_by_id(btf, t->type);
> +
> +	if (btf_type_is_type_tag(t)) {
> +		tag_value = __btf_name_by_offset(btf, t->name_off);
> +		if (strcmp(tag_value, "user") == 0)
> +			info->addr_space = BTF_ADDRSPACE_USER;
> +	}
> +
>  	/* skip modifiers */
>  	while (btf_type_is_modifier(t)) {

bpf_insn_access_aux approach will work only for the first
pointer deref, right?
Also addr_space will consume the last 4 free bytes in bpf_reg_state.
Maybe encode user/kernel as a flag into bpf_reg_type?
The verifier just got extended with PTR_MAYBE_NULL and MEM_RDONLY flags.
MEM_RDONLY is roughly equivalent 'const' modifier.
In that sense __user attribute is similar.
wdyt?

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

[Yonghong Song]

On 12/19/21 5:45 PM, Alexei Starovoitov wrote:
> On Thu, Dec 09, 2021 at 09:35:48AM -0800, Yonghong Song wrote:
>> BPF verifier supports direct memory access for BPF_PROG_TYPE_TRACING type
>> of bpf programs, 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 the 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. Without utilizing BTF __user tagging information,
>> current verifier will assume that a->b is a kernel memory access
>> and this may generate incorrect result.
> 
> The patch set looks great overall.
> 
>> +/* The pointee address space encoded in BTF. */
>> +enum btf_addr_space {
>> +	BTF_ADDRSPACE_UNSPEC	= 0,
>> +	BTF_ADDRSPACE_USER	= 1,
>> +};
>> +
>>   /* The information passed from prog-specific *_is_valid_access
>>    * back to the verifier.
>>    */
>> @@ -473,6 +479,7 @@ struct bpf_insn_access_aux {
>>   		struct {
>>   			struct btf *btf;
>>   			u32 btf_id;
>> +			enum btf_addr_space addr_space;
>>   		};
> ...
>> @@ -4998,7 +4999,15 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>>   
>>   	info->btf = btf;
>>   	info->btf_id = t->type;
>> +	info->addr_space = BTF_ADDRSPACE_UNSPEC;
>>   	t = btf_type_by_id(btf, t->type);
>> +
>> +	if (btf_type_is_type_tag(t)) {
>> +		tag_value = __btf_name_by_offset(btf, t->name_off);
>> +		if (strcmp(tag_value, "user") == 0)
>> +			info->addr_space = BTF_ADDRSPACE_USER;
>> +	}
>> +
>>   	/* skip modifiers */
>>   	while (btf_type_is_modifier(t)) {
> 
> bpf_insn_access_aux approach will work only for the first
> pointer deref, right?

That is true.

> Also addr_space will consume the last 4 free bytes in bpf_reg_state.
> Maybe encode user/kernel as a flag into bpf_reg_type?
> The verifier just got extended with PTR_MAYBE_NULL and MEM_RDONLY flags.
> MEM_RDONLY is roughly equivalent 'const' modifier.
> In that sense __user attribute is similar.
> wdyt?

I haven't carefully read Hao's patch set yet. But after roughly going 
through it I think your suggestion should work. Will give a try!

[PATCH bpf-next 3/5] selftests/bpf: rename btf_decl_tag.c to test_btf_decl_tag.c

[Yonghong Song]

The uapi btf.h contains the following declaration:
  struct btf_decl_tag {
       __s32   component_idx;
  };

The skeleton will also generate a struct with name
"btf_decl_tag" for bpf program btf_decl_tag.c.

Rename btf_decl_tag.c to test_btf_decl_tag.c so
the corresponding skeleton struct name becomes
"test_btf_decl_tag". This way, we could include
uapi btf.h in prog_tests/btf_tag.c.
There is no functionality change for this patch.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/prog_tests/btf_tag.c          | 8 ++++----
 .../bpf/progs/{btf_decl_tag.c => test_btf_decl_tag.c}     | 0
 2 files changed, 4 insertions(+), 4 deletions(-)
 rename tools/testing/selftests/bpf/progs/{btf_decl_tag.c => test_btf_decl_tag.c} (100%)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_tag.c b/tools/testing/selftests/bpf/prog_tests/btf_tag.c
index 88d63e23e35f..c4cf27777ff7 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_tag.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_tag.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2021 Facebook */
 #include <test_progs.h>
-#include "btf_decl_tag.skel.h"
+#include "test_btf_decl_tag.skel.h"
 
 /* struct btf_type_tag_test is referenced in btf_type_tag.skel.h */
 struct btf_type_tag_test {
@@ -11,9 +11,9 @@ struct btf_type_tag_test {
 
 static void test_btf_decl_tag(void)
 {
-	struct btf_decl_tag *skel;
+	struct test_btf_decl_tag *skel;
 
-	skel = btf_decl_tag__open_and_load();
+	skel = test_btf_decl_tag__open_and_load();
 	if (!ASSERT_OK_PTR(skel, "btf_decl_tag"))
 		return;
 
@@ -22,7 +22,7 @@ static void test_btf_decl_tag(void)
 		test__skip();
 	}
 
-	btf_decl_tag__destroy(skel);
+	test_btf_decl_tag__destroy(skel);
 }
 
 static void test_btf_type_tag(void)
diff --git a/tools/testing/selftests/bpf/progs/btf_decl_tag.c b/tools/testing/selftests/bpf/progs/test_btf_decl_tag.c
similarity index 100%
rename from tools/testing/selftests/bpf/progs/btf_decl_tag.c
rename to tools/testing/selftests/bpf/progs/test_btf_decl_tag.c
-- 
2.30.2

[PATCH bpf-next 4/5] selftests/bpf: add a selftest with __user tag

[Yonghong Song]

Added a selftest with two __user usages: a __user pointer-type argument
and a __user pointer-type struct member. In both cases,
directly accessing the user memory will result verification failure.

  $ ./test_progs -v -n 22/3
  ...
  libbpf: load bpf program failed: Permission denied
  libbpf: -- BEGIN DUMP LOG ---
  libbpf:
  R1 type=ctx expected=fp
  ; int BPF_PROG(test_user1, struct bpf_testmod_btf_type_tag_1 *arg)
  0: (79) r1 = *(u64 *)(r1 +0)
  func 'bpf_testmod_test_btf_type_tag_user_1' arg0 has btf_id 143948 addr_space 1 type STRUCT 'bpf_testmod_btf_type_tag_1'
  ; g = arg->a;
  1: (61) r1 = *(u32 *)(r1 +0)
  R1 is ptr_bpf_testmod_btf_type_tag_1 access user memory: off=0
  ...
  #22/3 btf_tag/btf_type_tag_user_1:OK

  $ ./test_progs -v -n 22/4
  ...
  libbpf: load bpf program failed: Permission denied
  libbpf: -- BEGIN DUMP LOG ---
  libbpf:
  R1 type=ctx expected=fp
  ; int BPF_PROG(test_user2, struct bpf_testmod_btf_type_tag_2 *arg)
  0: (79) r1 = *(u64 *)(r1 +0)
  func 'bpf_testmod_test_btf_type_tag_user_2' arg0 has btf_id 143950 addr_space 0 type STRUCT 'bpf_testmod_btf_type_tag_2'
  ; g = arg->p->a;
  1: (79) r1 = *(u64 *)(r1 +0)
  ; g = arg->p->a;
  2: (61) r1 = *(u32 *)(r1 +0)
  R1 is ptr_bpf_testmod_btf_type_tag_1 access user memory: off=0
  ...
  #22/4 btf_tag/btf_type_tag_user_2:OK

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 18 ++++++
 .../selftests/bpf/prog_tests/btf_tag.c        | 55 +++++++++++++++++++
 .../selftests/bpf/progs/btf_type_tag_user.c   | 29 ++++++++++
 3 files changed, 102 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..119fd5ba7ce4 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -21,6 +21,24 @@ bpf_testmod_test_mod_kfunc(int i)
 	*(int *)this_cpu_ptr(&bpf_testmod_ksym_percpu) = i;
 }
 
+struct bpf_testmod_btf_type_tag_1 {
+	int a;
+};
+
+struct bpf_testmod_btf_type_tag_2 {
+	struct bpf_testmod_btf_type_tag_1 __user *p;
+};
+
+noinline int
+bpf_testmod_test_btf_type_tag_user_1(struct bpf_testmod_btf_type_tag_1 __user *arg) {
+	return arg->a;
+}
+
+noinline int
+bpf_testmod_test_btf_type_tag_user_2(struct bpf_testmod_btf_type_tag_2 *arg) {
+	return arg->p->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 c4cf27777ff7..794752265ede 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_tag.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_tag.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2021 Facebook */
 #include <test_progs.h>
+#include <bpf/btf.h>
 #include "test_btf_decl_tag.skel.h"
 
 /* struct btf_type_tag_test is referenced in btf_type_tag.skel.h */
@@ -8,6 +9,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 +43,63 @@ static void test_btf_type_tag(void)
 	btf_type_tag__destroy(skel);
 }
 
+static void test_btf_type_tag_user(bool load_test_user1)
+{
+	const char *module_name = "bpf_testmod";
+	struct btf *vmlinux_btf, *module_btf;
+	struct btf_type_tag_user *skel;
+	__s32 type_id;
+	int err;
+
+	if (!env.has_testmod) {
+		test__skip();
+		return;
+	}
+
+	/* skip the test is the module does not have __user tags */
+	vmlinux_btf = btf__load_vmlinux_btf();
+	if (!ASSERT_OK_PTR(vmlinux_btf, "could not load vmlinux BTF"))
+		return;
+
+	module_btf = btf__load_module_btf(module_name, vmlinux_btf);
+	if (!ASSERT_OK_PTR(module_btf, "could not load module BTF"))
+		goto free_vmlinux_btf;
+
+	type_id = btf__find_by_name_kind(module_btf, "user", BTF_KIND_TYPE_TAG);
+	if (type_id <= 0) {
+		printf("%s:SKIP: btf_type_tag attribute not in %s", __func__, module_name);
+		test__skip();
+		goto free_module_btf;
+	}
+
+	skel = btf_type_tag_user__open();
+	if (!ASSERT_OK_PTR(skel, "btf_type_tag_user"))
+		goto free_module_btf;
+
+	if (load_test_user1)
+		bpf_program__set_autoload(skel->progs.test_user2, false);
+	else
+		bpf_program__set_autoload(skel->progs.test_user1, false);
+
+	err = btf_type_tag_user__load(skel);
+	ASSERT_ERR(err, "btf_type_tag_user");
+
+	btf_type_tag_user__destroy(skel);
+
+free_module_btf:
+	btf__free(module_btf);
+free_vmlinux_btf:
+	btf__free(vmlinux_btf);
+}
+
 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_1"))
+		test_btf_type_tag_user(true);
+	if (test__start_subtest("btf_type_tag_user_2"))
+		test_btf_type_tag_user(false);
 }
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..e149854f42dd
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/btf_type_tag_user.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+struct bpf_testmod_btf_type_tag_1 {
+	int a;
+};
+
+struct bpf_testmod_btf_type_tag_2 {
+	struct bpf_testmod_btf_type_tag_1 *p;
+};
+
+int g;
+
+SEC("fentry/bpf_testmod_test_btf_type_tag_user_1")
+int BPF_PROG(test_user1, struct bpf_testmod_btf_type_tag_1 *arg)
+{
+	g = arg->a;
+	return 0;
+}
+
+SEC("fentry/bpf_testmod_test_btf_type_tag_user_2")
+int BPF_PROG(test_user2, struct bpf_testmod_btf_type_tag_2 *arg)
+{
+	g = arg->p->a;
+	return 0;
+}
-- 
2.30.2

Re: [PATCH bpf-next 4/5] selftests/bpf: add a selftest with __user tag

[Alexei Starovoitov]

On Thu, Dec 09, 2021 at 09:35:59AM -0800, Yonghong Song wrote:
> Added a selftest with two __user usages: a __user pointer-type argument
> and a __user pointer-type struct member. In both cases,
> directly accessing the user memory will result verification failure.
...
> 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..e149854f42dd
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/btf_type_tag_user.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2021 Facebook */
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +struct bpf_testmod_btf_type_tag_1 {
> +	int a;
> +};
> +
> +struct bpf_testmod_btf_type_tag_2 {
> +	struct bpf_testmod_btf_type_tag_1 *p;
> +};
> +
> +int g;
> +
> +SEC("fentry/bpf_testmod_test_btf_type_tag_user_1")
> +int BPF_PROG(test_user1, struct bpf_testmod_btf_type_tag_1 *arg)
> +{
> +	g = arg->a;
> +	return 0;
> +}
> +
> +SEC("fentry/bpf_testmod_test_btf_type_tag_user_2")
> +int BPF_PROG(test_user2, struct bpf_testmod_btf_type_tag_2 *arg)
> +{
> +	g = arg->p->a;
> +	return 0;
> +}

This is a targeted synthetic test. Great, but can you add one
that probes real kernel function like:
getsockname(int fd, struct sockaddr __user *usockaddr
or
getpeername(int fd, struct sockaddr __user *usockaddr
and the bpf prog tries to deref usockaddr ?

Re: [PATCH bpf-next 4/5] selftests/bpf: add a selftest with __user tag

[Yonghong Song]

On 12/19/21 5:51 PM, Alexei Starovoitov wrote:
> On Thu, Dec 09, 2021 at 09:35:59AM -0800, Yonghong Song wrote:
>> Added a selftest with two __user usages: a __user pointer-type argument
>> and a __user pointer-type struct member. In both cases,
>> directly accessing the user memory will result verification failure.
> ...
>> 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..e149854f42dd
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/btf_type_tag_user.c
>> @@ -0,0 +1,29 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2021 Facebook */
>> +#include "vmlinux.h"
>> +#include <bpf/bpf_helpers.h>
>> +#include <bpf/bpf_tracing.h>
>> +
>> +struct bpf_testmod_btf_type_tag_1 {
>> +	int a;
>> +};
>> +
>> +struct bpf_testmod_btf_type_tag_2 {
>> +	struct bpf_testmod_btf_type_tag_1 *p;
>> +};
>> +
>> +int g;
>> +
>> +SEC("fentry/bpf_testmod_test_btf_type_tag_user_1")
>> +int BPF_PROG(test_user1, struct bpf_testmod_btf_type_tag_1 *arg)
>> +{
>> +	g = arg->a;
>> +	return 0;
>> +}
>> +
>> +SEC("fentry/bpf_testmod_test_btf_type_tag_user_2")
>> +int BPF_PROG(test_user2, struct bpf_testmod_btf_type_tag_2 *arg)
>> +{
>> +	g = arg->p->a;
>> +	return 0;
>> +}
> 
> This is a targeted synthetic test. Great, but can you add one
> that probes real kernel function like:
> getsockname(int fd, struct sockaddr __user *usockaddr
> or
> getpeername(int fd, struct sockaddr __user *usockaddr > and the bpf prog tries to deref usockaddr ?

Ack. Will do.

[PATCH bpf-next 5/5] selftests/bpf: specify pahole version requirement for btf_tag test

[Yonghong Song]

Specify pahole version requirement (1.23) for btf_tag subtests
btf_type_tag_user_{1, 2}.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/README.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst
index 42ef250c7acc..eee6656de0e6 100644
--- a/tools/testing/selftests/bpf/README.rst
+++ b/tools/testing/selftests/bpf/README.rst
@@ -206,6 +206,7 @@ btf_tag test and Clang version
 
 The btf_tag selftest requires LLVM support to recognize the btf_decl_tag and
 btf_type_tag attributes. They are introduced in `Clang 14` [0_, 1_].
+The subtests ``btf_type_tag_user_{1, 2}`` also requires pahole version ``1.23``.
 
 Without them, the btf_tag selftest will be skipped and you will observe:
 
-- 
2.30.2