All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types
@ 2021-11-30  1:29 Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 1/9] bpf: Introduce composable reg, ret and arg types Hao Luo
                   ` (8 more replies)
  0 siblings, 9 replies; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

This patch set consists of two changes:

 - a cleanup of arg_type, ret_type and reg_type which try to make those
   types composable. (patch 1/9 - patch 6/9)
 - a bug fix that prevents bpf programs from writing kernel memory.
   (patch 7/9 - patch 9/9)

The purpose of the cleanup is to find a scalable way to expressing type
nullness and read-onliness. This patchset introduces two flags that
can be applied on all three types: PTR_MAYBE_NULL and MEM_RDONLY.
Previous types such as ARG_XXX_OR_NULL can now be written as

 ARG_XXX | PTR_MAYBE_NULL

Similarly, PTR_TO_RDONLY_BUF is now "PTR_TO_BUF | MEM_RDONLY".

Flags can be composed, as ARGs can be both MEM_RDONLY and MAYBE_NULL.

 ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY

Based on this new composable types, patch 7/9 applies MEM_RDONLY on
PTR_TO_MEM, in order to tag the returned memory from per_cpu_ptr as
read-only. Therefore fixing a previous bug that one can leverage
per_cpu_ptr to modify kernel memory within BPF programs.

Patch 8/9 generalizes the use of MEM_RDONLY further by tagging a set of
helper arguments ARG_PTR_TO_MEM with MEM_RDONLY. Some helper functions
may override their arguments, such as bpf_d_path, bpf_snprintf. In this
patch, we narrow the ARG_PTR_TO_MEM to be compatible with only a subset
of memory types. This prevents these helpers from writing read-only
memories. For the helpers that do not write its arguments, we add tag
MEM_RDONLY to allow taking a RDONLY memory as argument.

Previous versions of this patchset:

[1] https://lore.kernel.org/bpf/20211109003052.3499225-1-haoluo@google.com/T/
[2] https://lore.kernel.org/bpf/20211109021624.1140446-8-haoluo@google.com/T/

Hao Luo (9):
  bpf: Introduce composable reg, ret and arg types.
  bpf: Replace ARG_XXX_OR_NULL with ARG_XXX | PTR_MAYBE_NULL
  bpf: Replace RET_XXX_OR_NULL with RET_XXX | PTR_MAYBE_NULL
  bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL
  bpf: Introduce MEM_RDONLY flag
  bpf: Convert PTR_TO_MEM_OR_NULL to composable types.
  bpf: Make per_cpu_ptr return rdonly PTR_TO_MEM.
  bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.
  bpf/selftests: Test PTR_TO_RDONLY_MEM

 include/linux/bpf.h                           | 105 +++-
 kernel/bpf/btf.c                              |  13 +-
 kernel/bpf/cgroup.c                           |   2 +-
 kernel/bpf/helpers.c                          |  12 +-
 kernel/bpf/map_iter.c                         |   4 +-
 kernel/bpf/ringbuf.c                          |   2 +-
 kernel/bpf/syscall.c                          |   2 +-
 kernel/bpf/verifier.c                         | 455 +++++++++---------
 kernel/trace/bpf_trace.c                      |  26 +-
 net/core/bpf_sk_storage.c                     |   2 +-
 net/core/filter.c                             |  64 +--
 net/core/sock_map.c                           |   2 +-
 .../selftests/bpf/prog_tests/ksyms_btf.c      |  14 +
 .../bpf/progs/test_ksyms_btf_write_check.c    |  29 ++
 14 files changed, 414 insertions(+), 318 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_btf_write_check.c

-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 1/9] bpf: Introduce composable reg, ret and arg types.
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  2021-12-01 20:29   ` Alexei Starovoitov
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 2/9] bpf: Replace ARG_XXX_OR_NULL with ARG_XXX | PTR_MAYBE_NULL Hao Luo
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

There are some common properties shared between bpf reg, ret and arg
values. For instance, a value may be a NULL pointer, or a pointer to
a read-only memory. Previously, to express these properties, enumeration
was used. For example, in order to test whether a reg value can be NULL,
reg_type_may_be_null() simply enumerates all types that are possibly
NULL. The problem of this approach is that it's not scalable and causes
a lot of duplication. These properties can be combined, for example, a
type could be either MAYBE_NULL or RDONLY, or both.

This patch series rewrites the layout of reg_type, arg_type and
ret_type, so that common properties can be extracted and represented as
composable flag. For example, one can write

 ARG_PTR_TO_MEM | PTR_MAYBE_NULL

which is equivalent to the previous

 ARG_PTR_TO_MEM_OR_NULL

The type ARG_PTR_TO_MEM are called "base types" in this patch. Base
types can be extended with flags. A flag occupies the higher bits while
base types sits in the lower bits.

This patch in particular sets up a set of macro for this purpose. The
followed patches rewrites arg_types, ret_types and reg_types
respectively.

Signed-off-by: Hao Luo <haoluo@google.com>
---
 include/linux/bpf.h | 50 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index cc7a0c36e7df..b592b3f7d223 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -297,6 +297,37 @@ bool bpf_map_meta_equal(const struct bpf_map *meta0,
 
 extern const struct bpf_map_ops bpf_map_offload_ops;
 
+/* bpf_type_flag contains a set of flags that are applicable to the values of
+ * arg_type, ret_type and reg_type. For example, a pointer value may be null,
+ * or a memory is read-only. We classify types into two categories: base types
+ * and extended types. Extended types are base types combined with a type flag.
+ *
+ * Currently there are no more than 32 base types in arg_type, ret_type and
+ * reg_types.
+ */
+#define BPF_BASE_TYPE_BITS	8
+
+enum bpf_type_flag {
+	/* PTR may be NULL. */
+	PTR_MAYBE_NULL		= BIT(0 + BPF_BASE_TYPE_BITS),
+
+	__BPF_TYPE_LAST_FLAG	= PTR_MAYBE_NULL,
+};
+
+#define BPF_BASE_TYPE_MASK	GENMASK(BPF_BASE_TYPE_BITS, 0)
+
+/* Max number of base types. */
+#define BPF_BASE_TYPE_LIMIT	(1UL << BPF_BASE_TYPE_BITS)
+
+/* Max number of all types. */
+#define BPF_TYPE_LIMIT		(__BPF_TYPE_LAST_FLAG | (__BPF_TYPE_LAST_FLAG - 1))
+
+/* extract base type. */
+#define BPF_BASE_TYPE(x)	((x) & BPF_BASE_TYPE_MASK)
+
+/* extract flags from an extended type. */
+#define BPF_TYPE_FLAG(x)	((enum bpf_type_flag)((x) & ~BPF_BASE_TYPE_MASK))
+
 /* function argument constraints */
 enum bpf_arg_type {
 	ARG_DONTCARE = 0,	/* unused argument in helper function */
@@ -343,7 +374,13 @@ enum bpf_arg_type {
 	ARG_PTR_TO_CONST_STR,	/* pointer to a null terminated read-only string */
 	ARG_PTR_TO_TIMER,	/* pointer to bpf_timer */
 	__BPF_ARG_TYPE_MAX,
+
+	/* This must be the last entry. Its purpose is to ensure the enum is
+	 * wide enough to hold the higher bits reserved for bpf_type_flag.
+	 */
+	__BPF_ARG_TYPE_LIMIT	= BPF_TYPE_LIMIT,
 };
+static_assert(__BPF_ARG_TYPE_MAX <= BPF_BASE_TYPE_LIMIT);
 
 /* type of values returned from helper functions */
 enum bpf_return_type {
@@ -359,7 +396,14 @@ enum bpf_return_type {
 	RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL, /* returns a pointer to a valid memory or a btf_id or NULL */
 	RET_PTR_TO_MEM_OR_BTF_ID,	/* returns a pointer to a valid memory or a btf_id */
 	RET_PTR_TO_BTF_ID,		/* returns a pointer to a btf_id */
+	__BPF_RET_TYPE_MAX,
+
+	/* This must be the last entry. Its purpose is to ensure the enum is
+	 * wide enough to hold the higher bits reserved for bpf_type_flag.
+	 */
+	__BPF_RET_TYPE_LIMIT	= BPF_TYPE_LIMIT,
 };
+static_assert(__BPF_RET_TYPE_MAX <= BPF_BASE_TYPE_LIMIT);
 
 /* eBPF function prototype used by verifier to allow BPF_CALLs from eBPF programs
  * to in-kernel helper functions and for adjusting imm32 field in BPF_CALL
@@ -461,7 +505,13 @@ enum bpf_reg_type {
 	PTR_TO_FUNC,		 /* reg points to a bpf program function */
 	PTR_TO_MAP_KEY,		 /* reg points to a map element key */
 	__BPF_REG_TYPE_MAX,
+
+	/* This must be the last entry. Its purpose is to ensure the enum is
+	 * wide enough to hold the higher bits reserved for bpf_type_flag.
+	 */
+	__BPF_REG_TYPE_LIMIT	= BPF_TYPE_LIMIT,
 };
+static_assert(__BPF_REG_TYPE_MAX <= BPF_BASE_TYPE_LIMIT);
 
 /* The information passed from prog-specific *_is_valid_access
  * back to the verifier.
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 2/9] bpf: Replace ARG_XXX_OR_NULL with ARG_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 1/9] bpf: Introduce composable reg, ret and arg types Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX " Hao Luo
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

We have introduced a new type to make bpf_arg composable, by
introducing flags to tag a particular type.

One of the flag is PTR_MAYBE_NULL, which indicates a pointer
may be NULL. This patch switches the qualified arg_types to
use this flag. The arg_types changed in this patch include:

1. ARG_PTR_TO_MAP_VALUE_OR_NULL
2. ARG_PTR_TO_MEM_OR_NULL
3. ARG_PTR_TO_CTX_OR_NULL
4. ARG_PTR_TO_SOCKET_OR_NULL
5. ARG_PTR_TO_ALLOC_MEM_OR_NULL
6. ARG_PTR_TO_STACK_OR_NULL

This patch does not eliminate the use of these arg_types, instead, it
makes them an alias to the 'ARG_XXX | PTR_MAYBE_NULL'.

Signed-off-by: Hao Luo <haoluo@google.com>
---
 include/linux/bpf.h   | 15 +++++++++------
 kernel/bpf/verifier.c | 35 ++++++++++++-----------------------
 2 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b592b3f7d223..da31c79724d4 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -339,13 +339,11 @@ enum bpf_arg_type {
 	ARG_PTR_TO_MAP_KEY,	/* pointer to stack used as map key */
 	ARG_PTR_TO_MAP_VALUE,	/* pointer to stack used as map value */
 	ARG_PTR_TO_UNINIT_MAP_VALUE,	/* pointer to valid memory used to store a map value */
-	ARG_PTR_TO_MAP_VALUE_OR_NULL,	/* pointer to stack used as map value or NULL */
 
 	/* the following constraints used to prototype bpf_memcmp() and other
 	 * functions that access data on eBPF program stack
 	 */
 	ARG_PTR_TO_MEM,		/* pointer to valid memory (stack, packet, map value) */
-	ARG_PTR_TO_MEM_OR_NULL, /* pointer to valid memory or NULL */
 	ARG_PTR_TO_UNINIT_MEM,	/* pointer to memory does not need to be initialized,
 				 * helper function must fill all bytes or clear
 				 * them in error case.
@@ -355,26 +353,31 @@ enum bpf_arg_type {
 	ARG_CONST_SIZE_OR_ZERO,	/* number of bytes accessed from memory or 0 */
 
 	ARG_PTR_TO_CTX,		/* pointer to context */
-	ARG_PTR_TO_CTX_OR_NULL,	/* pointer to context or NULL */
 	ARG_ANYTHING,		/* any (initialized) argument is ok */
 	ARG_PTR_TO_SPIN_LOCK,	/* pointer to bpf_spin_lock */
 	ARG_PTR_TO_SOCK_COMMON,	/* pointer to sock_common */
 	ARG_PTR_TO_INT,		/* pointer to int */
 	ARG_PTR_TO_LONG,	/* pointer to long */
 	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock (fullsock) */
-	ARG_PTR_TO_SOCKET_OR_NULL,	/* pointer to bpf_sock (fullsock) or NULL */
 	ARG_PTR_TO_BTF_ID,	/* pointer to in-kernel struct */
 	ARG_PTR_TO_ALLOC_MEM,	/* pointer to dynamically allocated memory */
-	ARG_PTR_TO_ALLOC_MEM_OR_NULL,	/* pointer to dynamically allocated memory or NULL */
 	ARG_CONST_ALLOC_SIZE_OR_ZERO,	/* number of allocated bytes requested */
 	ARG_PTR_TO_BTF_ID_SOCK_COMMON,	/* pointer to in-kernel sock_common or bpf-mirrored bpf_sock */
 	ARG_PTR_TO_PERCPU_BTF_ID,	/* pointer to in-kernel percpu type */
 	ARG_PTR_TO_FUNC,	/* pointer to a bpf program function */
-	ARG_PTR_TO_STACK_OR_NULL,	/* pointer to stack or NULL */
+	ARG_PTR_TO_STACK,	/* pointer to stack */
 	ARG_PTR_TO_CONST_STR,	/* pointer to a null terminated read-only string */
 	ARG_PTR_TO_TIMER,	/* pointer to bpf_timer */
 	__BPF_ARG_TYPE_MAX,
 
+	/* Extended arg_types. */
+	ARG_PTR_TO_MAP_VALUE_OR_NULL	= PTR_MAYBE_NULL | ARG_PTR_TO_MAP_VALUE,
+	ARG_PTR_TO_MEM_OR_NULL		= PTR_MAYBE_NULL | ARG_PTR_TO_MEM,
+	ARG_PTR_TO_CTX_OR_NULL		= PTR_MAYBE_NULL | ARG_PTR_TO_CTX,
+	ARG_PTR_TO_SOCKET_OR_NULL	= PTR_MAYBE_NULL | ARG_PTR_TO_SOCKET,
+	ARG_PTR_TO_ALLOC_MEM_OR_NULL	= PTR_MAYBE_NULL | ARG_PTR_TO_ALLOC_MEM,
+	ARG_PTR_TO_STACK_OR_NULL	= PTR_MAYBE_NULL | ARG_PTR_TO_STACK,
+
 	/* This must be the last entry. Its purpose is to ensure the enum is
 	 * wide enough to hold the higher bits reserved for bpf_type_flag.
 	 */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0763cca139a7..78227d2cffb1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -474,12 +474,7 @@ static bool arg_type_may_be_refcounted(enum bpf_arg_type type)
 
 static bool arg_type_may_be_null(enum bpf_arg_type type)
 {
-	return type == ARG_PTR_TO_MAP_VALUE_OR_NULL ||
-	       type == ARG_PTR_TO_MEM_OR_NULL ||
-	       type == ARG_PTR_TO_CTX_OR_NULL ||
-	       type == ARG_PTR_TO_SOCKET_OR_NULL ||
-	       type == ARG_PTR_TO_ALLOC_MEM_OR_NULL ||
-	       type == ARG_PTR_TO_STACK_OR_NULL;
+	return BPF_TYPE_FLAG(type) & PTR_MAYBE_NULL;
 }
 
 /* Determine whether the function releases some resources allocated by another
@@ -4932,9 +4927,8 @@ static int process_timer_func(struct bpf_verifier_env *env, int regno,
 
 static bool arg_type_is_mem_ptr(enum bpf_arg_type type)
 {
-	return type == ARG_PTR_TO_MEM ||
-	       type == ARG_PTR_TO_MEM_OR_NULL ||
-	       type == ARG_PTR_TO_UNINIT_MEM;
+	return BPF_BASE_TYPE(type) == ARG_PTR_TO_MEM ||
+	       BPF_BASE_TYPE(type) == ARG_PTR_TO_UNINIT_MEM;
 }
 
 static bool arg_type_is_mem_size(enum bpf_arg_type type)
@@ -5071,31 +5065,26 @@ static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = {
 	[ARG_PTR_TO_MAP_KEY]		= &map_key_value_types,
 	[ARG_PTR_TO_MAP_VALUE]		= &map_key_value_types,
 	[ARG_PTR_TO_UNINIT_MAP_VALUE]	= &map_key_value_types,
-	[ARG_PTR_TO_MAP_VALUE_OR_NULL]	= &map_key_value_types,
 	[ARG_CONST_SIZE]		= &scalar_types,
 	[ARG_CONST_SIZE_OR_ZERO]	= &scalar_types,
 	[ARG_CONST_ALLOC_SIZE_OR_ZERO]	= &scalar_types,
 	[ARG_CONST_MAP_PTR]		= &const_map_ptr_types,
 	[ARG_PTR_TO_CTX]		= &context_types,
-	[ARG_PTR_TO_CTX_OR_NULL]	= &context_types,
 	[ARG_PTR_TO_SOCK_COMMON]	= &sock_types,
 #ifdef CONFIG_NET
 	[ARG_PTR_TO_BTF_ID_SOCK_COMMON]	= &btf_id_sock_common_types,
 #endif
 	[ARG_PTR_TO_SOCKET]		= &fullsock_types,
-	[ARG_PTR_TO_SOCKET_OR_NULL]	= &fullsock_types,
 	[ARG_PTR_TO_BTF_ID]		= &btf_ptr_types,
 	[ARG_PTR_TO_SPIN_LOCK]		= &spin_lock_types,
 	[ARG_PTR_TO_MEM]		= &mem_types,
-	[ARG_PTR_TO_MEM_OR_NULL]	= &mem_types,
 	[ARG_PTR_TO_UNINIT_MEM]		= &mem_types,
 	[ARG_PTR_TO_ALLOC_MEM]		= &alloc_mem_types,
-	[ARG_PTR_TO_ALLOC_MEM_OR_NULL]	= &alloc_mem_types,
 	[ARG_PTR_TO_INT]		= &int_ptr_types,
 	[ARG_PTR_TO_LONG]		= &int_ptr_types,
 	[ARG_PTR_TO_PERCPU_BTF_ID]	= &percpu_btf_ptr_types,
 	[ARG_PTR_TO_FUNC]		= &func_ptr_types,
-	[ARG_PTR_TO_STACK_OR_NULL]	= &stack_ptr_types,
+	[ARG_PTR_TO_STACK]		= &stack_ptr_types,
 	[ARG_PTR_TO_CONST_STR]		= &const_str_ptr_types,
 	[ARG_PTR_TO_TIMER]		= &timer_types,
 };
@@ -5109,7 +5098,7 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
 	const struct bpf_reg_types *compatible;
 	int i, j;
 
-	compatible = compatible_reg_types[arg_type];
+	compatible = compatible_reg_types[BPF_BASE_TYPE(arg_type)];
 	if (!compatible) {
 		verbose(env, "verifier internal error: unsupported arg type %d\n", arg_type);
 		return -EFAULT;
@@ -5190,9 +5179,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 		return -EACCES;
 	}
 
-	if (arg_type == ARG_PTR_TO_MAP_VALUE ||
-	    arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE ||
-	    arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL) {
+	if (BPF_BASE_TYPE(arg_type) == ARG_PTR_TO_MAP_VALUE ||
+	    BPF_BASE_TYPE(arg_type) == ARG_PTR_TO_UNINIT_MAP_VALUE) {
 		err = resolve_map_arg_type(env, meta, &arg_type);
 		if (err)
 			return err;
@@ -5267,10 +5255,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 		err = check_helper_mem_access(env, regno,
 					      meta->map_ptr->key_size, false,
 					      NULL);
-	} else if (arg_type == ARG_PTR_TO_MAP_VALUE ||
-		   (arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL &&
-		    !register_is_null(reg)) ||
-		   arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE) {
+	} else if (BPF_BASE_TYPE(arg_type) == ARG_PTR_TO_MAP_VALUE ||
+		   BPF_BASE_TYPE(arg_type) == ARG_PTR_TO_UNINIT_MAP_VALUE) {
+		if (!arg_type_may_be_null(arg_type) && register_is_null(reg))
+			return err;
+
 		/* bpf_map_xxx(..., map_ptr, ..., value) call:
 		 * check [value, value + map->value_size) validity
 		 */
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 1/9] bpf: Introduce composable reg, ret and arg types Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 2/9] bpf: Replace ARG_XXX_OR_NULL with ARG_XXX | PTR_MAYBE_NULL Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  2021-11-30  2:59   ` kernel test robot
                     ` (2 more replies)
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX " Hao Luo
                   ` (5 subsequent siblings)
  8 siblings, 3 replies; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

We have introduced a new type to make bpf_ret composable, by
introducing flags to tag a particular type.

One of the flag is PTR_MAYBE_NULL, which indicates a pointer
may be NULL. This patch switches the qualified arg_types to
use this flag. The ret_types changed in this patch include:

1. RET_PTR_TO_MAP_VALUE_OR_NULL
2. RET_PTR_TO_SOCKET_OR_NULL
3. RET_PTR_TO_TCP_SOCK_OR_NULL
4. RET_PTR_TO_SOCK_COMMON_OR_NULL
5. RET_PTR_TO_ALLOC_MEM_OR_NULL
6. RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL
7. RET_PTR_TO_BTF_ID_OR_NULL

Signed-off-by: Hao Luo <haoluo@google.com>
---
 include/linux/bpf.h   | 19 ++++++++++------
 kernel/bpf/helpers.c  |  2 +-
 kernel/bpf/verifier.c | 53 +++++++++++++++++++++++--------------------
 3 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index da31c79724d4..d6189eb14a1f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -390,17 +390,22 @@ enum bpf_return_type {
 	RET_INTEGER,			/* function returns integer */
 	RET_VOID,			/* function doesn't return anything */
 	RET_PTR_TO_MAP_VALUE,		/* returns a pointer to map elem value */
-	RET_PTR_TO_MAP_VALUE_OR_NULL,	/* returns a pointer to map elem value or NULL */
-	RET_PTR_TO_SOCKET_OR_NULL,	/* returns a pointer to a socket or NULL */
-	RET_PTR_TO_TCP_SOCK_OR_NULL,	/* returns a pointer to a tcp_sock or NULL */
-	RET_PTR_TO_SOCK_COMMON_OR_NULL,	/* returns a pointer to a sock_common or NULL */
-	RET_PTR_TO_ALLOC_MEM_OR_NULL,	/* returns a pointer to dynamically allocated memory or NULL */
-	RET_PTR_TO_BTF_ID_OR_NULL,	/* returns a pointer to a btf_id or NULL */
-	RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL, /* returns a pointer to a valid memory or a btf_id or NULL */
+	RET_PTR_TO_SOCKET,		/* returns a pointer to a socket */
+	RET_PTR_TO_TCP_SOCK,		/* returns a pointer to a tcp_sock */
+	RET_PTR_TO_SOCK_COMMON,		/* returns a pointer to a sock_common */
+	RET_PTR_TO_ALLOC_MEM,		/* returns a pointer to dynamically allocated memory */
 	RET_PTR_TO_MEM_OR_BTF_ID,	/* returns a pointer to a valid memory or a btf_id */
 	RET_PTR_TO_BTF_ID,		/* returns a pointer to a btf_id */
 	__BPF_RET_TYPE_MAX,
 
+	/* Extended ret_types. */
+	RET_PTR_TO_MAP_VALUE_OR_NULL	= PTR_MAYBE_NULL | RET_PTR_TO_MAP_VALUE,
+	RET_PTR_TO_SOCKET_OR_NULL	= PTR_MAYBE_NULL | RET_PTR_TO_SOCKET,
+	RET_PTR_TO_TCP_SOCK_OR_NULL	= PTR_MAYBE_NULL | RET_PTR_TO_TCP_SOCK,
+	RET_PTR_TO_SOCK_COMMON_OR_NULL	= PTR_MAYBE_NULL | RET_PTR_TO_SOCK_COMMON,
+	RET_PTR_TO_ALLOC_MEM_OR_NULL	= PTR_MAYBE_NULL | RET_PTR_TO_ALLOC_MEM,
+	RET_PTR_TO_BTF_ID_OR_NULL	= PTR_MAYBE_NULL | RET_PTR_TO_BTF_ID,
+
 	/* This must be the last entry. Its purpose is to ensure the enum is
 	 * wide enough to hold the higher bits reserved for bpf_type_flag.
 	 */
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 1ffd469c217f..293d9314ec7f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -667,7 +667,7 @@ BPF_CALL_2(bpf_per_cpu_ptr, const void *, ptr, u32, cpu)
 const struct bpf_func_proto bpf_per_cpu_ptr_proto = {
 	.func		= bpf_per_cpu_ptr,
 	.gpl_only	= false,
-	.ret_type	= RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL,
+	.ret_type	= RET_PTR_TO_MEM_OR_BTF_ID | PTR_MAYBE_NULL,
 	.arg1_type	= ARG_PTR_TO_PERCPU_BTF_ID,
 	.arg2_type	= ARG_ANYTHING,
 };
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 78227d2cffb1..fb44db2456f2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -477,6 +477,11 @@ static bool arg_type_may_be_null(enum bpf_arg_type type)
 	return BPF_TYPE_FLAG(type) & PTR_MAYBE_NULL;
 }
 
+static bool ret_type_may_be_null(enum bpf_return_type type)
+{
+	return BPF_TYPE_FLAG(type) & PTR_MAYBE_NULL;
+}
+
 /* Determine whether the function releases some resources allocated by another
  * function call. The first reference type argument will be assumed to be
  * released by release_reference().
@@ -6370,6 +6375,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 			     int *insn_idx_p)
 {
 	const struct bpf_func_proto *fn = NULL;
+	enum bpf_return_type ret_type;
 	struct bpf_reg_state *regs;
 	struct bpf_call_arg_meta meta;
 	int insn_idx = *insn_idx_p;
@@ -6510,13 +6516,13 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 	regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
 
 	/* update return register (already marked as written above) */
-	if (fn->ret_type == RET_INTEGER) {
+	ret_type = fn->ret_type;
+	if (ret_type == RET_INTEGER) {
 		/* sets type to SCALAR_VALUE */
 		mark_reg_unknown(env, regs, BPF_REG_0);
-	} else if (fn->ret_type == RET_VOID) {
+	} else if (ret_type == RET_VOID) {
 		regs[BPF_REG_0].type = NOT_INIT;
-	} else if (fn->ret_type == RET_PTR_TO_MAP_VALUE_OR_NULL ||
-		   fn->ret_type == RET_PTR_TO_MAP_VALUE) {
+	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MAP_VALUE) {
 		/* There is no offset yet applied, variable or fixed */
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		/* remember map_ptr, so that check_map_access()
@@ -6530,28 +6536,27 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 		}
 		regs[BPF_REG_0].map_ptr = meta.map_ptr;
 		regs[BPF_REG_0].map_uid = meta.map_uid;
-		if (fn->ret_type == RET_PTR_TO_MAP_VALUE) {
+		if (ret_type_may_be_null(fn->ret_type)) {
+			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
+		} else {
 			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;
 			if (map_value_has_spin_lock(meta.map_ptr))
 				regs[BPF_REG_0].id = ++env->id_gen;
-		} else {
-			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
 		}
-	} else if (fn->ret_type == RET_PTR_TO_SOCKET_OR_NULL) {
+	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCKET) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
-	} else if (fn->ret_type == RET_PTR_TO_SOCK_COMMON_OR_NULL) {
+	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCK_COMMON) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_SOCK_COMMON_OR_NULL;
-	} else if (fn->ret_type == RET_PTR_TO_TCP_SOCK_OR_NULL) {
+	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_TCP_SOCK) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_TCP_SOCK_OR_NULL;
-	} else if (fn->ret_type == RET_PTR_TO_ALLOC_MEM_OR_NULL) {
+	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_ALLOC_MEM) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
 		regs[BPF_REG_0].mem_size = meta.mem_size;
-	} else if (fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL ||
-		   fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID) {
+	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MEM_OR_BTF_ID) {
 		const struct btf_type *t;
 
 		mark_reg_known_zero(env, regs, BPF_REG_0);
@@ -6570,28 +6575,28 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 				return -EINVAL;
 			}
 			regs[BPF_REG_0].type =
-				fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID ?
-				PTR_TO_MEM : PTR_TO_MEM_OR_NULL;
+				(ret_type & PTR_MAYBE_NULL) ?
+				PTR_TO_MEM_OR_NULL : PTR_TO_MEM;
 			regs[BPF_REG_0].mem_size = tsize;
 		} else {
 			regs[BPF_REG_0].type =
-				fn->ret_type == RET_PTR_TO_MEM_OR_BTF_ID ?
-				PTR_TO_BTF_ID : PTR_TO_BTF_ID_OR_NULL;
+				(ret_type & PTR_MAYBE_NULL) ?
+				PTR_TO_BTF_ID_OR_NULL : PTR_TO_BTF_ID;
 			regs[BPF_REG_0].btf = meta.ret_btf;
 			regs[BPF_REG_0].btf_id = meta.ret_btf_id;
 		}
-	} else if (fn->ret_type == RET_PTR_TO_BTF_ID_OR_NULL ||
-		   fn->ret_type == RET_PTR_TO_BTF_ID) {
+	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_BTF_ID) {
 		int ret_btf_id;
 
 		mark_reg_known_zero(env, regs, BPF_REG_0);
-		regs[BPF_REG_0].type = fn->ret_type == RET_PTR_TO_BTF_ID ?
-						     PTR_TO_BTF_ID :
-						     PTR_TO_BTF_ID_OR_NULL;
+		regs[BPF_REG_0].type = (ret_type & PTR_MAYBE_NULL) ?
+						     PTR_TO_BTF_ID_OR_NULL :
+						     PTR_TO_BTF_ID;
 		ret_btf_id = *fn->ret_btf_id;
 		if (ret_btf_id == 0) {
 			verbose(env, "invalid return type %d of func %s#%d\n",
-				fn->ret_type, func_id_name(func_id), func_id);
+				BPF_BASE_TYPE(ret_type), func_id_name(func_id),
+				func_id);
 			return -EINVAL;
 		}
 		/* current BPF helper definitions are only coming from
@@ -6601,7 +6606,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 		regs[BPF_REG_0].btf_id = ret_btf_id;
 	} else {
 		verbose(env, "unknown return type %d of func %s#%d\n",
-			fn->ret_type, func_id_name(func_id), func_id);
+			BPF_BASE_TYPE(ret_type), func_id_name(func_id), func_id);
 		return -EINVAL;
 	}
 
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
                   ` (2 preceding siblings ...)
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX " Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  2021-11-30  3:30   ` kernel test robot
                     ` (2 more replies)
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 5/9] bpf: Introduce MEM_RDONLY flag Hao Luo
                   ` (4 subsequent siblings)
  8 siblings, 3 replies; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

We have introduced a new type to make bpf_reg composable, by
introducing flags to tag a particular type.

One of the flag is PTR_MAYBE_NULL, which indicates a pointer
may be NULL. This patch switches the qualified reg_types to
use this flag. The reg_types changed in this patch include:

1. PTR_TO_MAP_VALUE_OR_NULL
2. PTR_TO_SOCKET_OR_NULL
3. PTR_TO_SOCK_COMMON_OR_NULL
4. PTR_TO_TCP_SOCK_OR_NULL
5. PTR_TO_BTF_ID_OR_NULL
6. PTR_TO_MEM_OR_NULL
7. PTR_TO_RDONLY_BUF_OR_NULL
8. PTR_TO_RDWR_BUF_OR_NULL

Signed-off-by: Hao Luo <haoluo@google.com>
---
 include/linux/bpf.h       |  16 +--
 kernel/bpf/btf.c          |   8 +-
 kernel/bpf/map_iter.c     |   4 +-
 kernel/bpf/verifier.c     | 265 +++++++++++++++-----------------------
 net/core/bpf_sk_storage.c |   2 +-
 net/core/sock_map.c       |   2 +-
 6 files changed, 121 insertions(+), 176 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d6189eb14a1f..d484f6637e60 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -473,18 +473,14 @@ enum bpf_reg_type {
 	PTR_TO_CTX,		 /* reg points to bpf_context */
 	CONST_PTR_TO_MAP,	 /* reg points to struct bpf_map */
 	PTR_TO_MAP_VALUE,	 /* reg points to map element value */
-	PTR_TO_MAP_VALUE_OR_NULL,/* points to map elem value or NULL */
 	PTR_TO_STACK,		 /* reg == frame_pointer + offset */
 	PTR_TO_PACKET_META,	 /* skb->data - meta_len */
 	PTR_TO_PACKET,		 /* reg points to skb->data */
 	PTR_TO_PACKET_END,	 /* skb->data + headlen */
 	PTR_TO_FLOW_KEYS,	 /* reg points to bpf_flow_keys */
 	PTR_TO_SOCKET,		 /* reg points to struct bpf_sock */
-	PTR_TO_SOCKET_OR_NULL,	 /* reg points to struct bpf_sock or NULL */
 	PTR_TO_SOCK_COMMON,	 /* reg points to sock_common */
-	PTR_TO_SOCK_COMMON_OR_NULL, /* reg points to sock_common or NULL */
 	PTR_TO_TCP_SOCK,	 /* reg points to struct tcp_sock */
-	PTR_TO_TCP_SOCK_OR_NULL, /* reg points to struct tcp_sock or NULL */
 	PTR_TO_TP_BUFFER,	 /* reg points to a writable raw tp's buffer */
 	PTR_TO_XDP_SOCK,	 /* reg points to struct xdp_sock */
 	/* PTR_TO_BTF_ID points to a kernel struct that does not need
@@ -502,18 +498,22 @@ enum bpf_reg_type {
 	 * been checked for null. Used primarily to inform the verifier
 	 * an explicit null check is required for this struct.
 	 */
-	PTR_TO_BTF_ID_OR_NULL,
 	PTR_TO_MEM,		 /* reg points to valid memory region */
-	PTR_TO_MEM_OR_NULL,	 /* reg points to valid memory region or NULL */
 	PTR_TO_RDONLY_BUF,	 /* reg points to a readonly buffer */
-	PTR_TO_RDONLY_BUF_OR_NULL, /* reg points to a readonly buffer or NULL */
 	PTR_TO_RDWR_BUF,	 /* reg points to a read/write buffer */
-	PTR_TO_RDWR_BUF_OR_NULL, /* reg points to a read/write buffer or NULL */
 	PTR_TO_PERCPU_BTF_ID,	 /* reg points to a percpu kernel variable */
 	PTR_TO_FUNC,		 /* reg points to a bpf program function */
 	PTR_TO_MAP_KEY,		 /* reg points to a map element key */
 	__BPF_REG_TYPE_MAX,
 
+	/* Extended reg_types. */
+	PTR_TO_MAP_VALUE_OR_NULL	= PTR_MAYBE_NULL | PTR_TO_MAP_VALUE,
+	PTR_TO_SOCKET_OR_NULL		= PTR_MAYBE_NULL | PTR_TO_SOCKET,
+	PTR_TO_SOCK_COMMON_OR_NULL	= PTR_MAYBE_NULL | PTR_TO_SOCK_COMMON,
+	PTR_TO_TCP_SOCK_OR_NULL		= PTR_MAYBE_NULL | PTR_TO_TCP_SOCK,
+	PTR_TO_BTF_ID_OR_NULL		= PTR_MAYBE_NULL | PTR_TO_BTF_ID,
+	PTR_TO_MEM_OR_NULL		= PTR_MAYBE_NULL | PTR_TO_MEM,
+
 	/* This must be the last entry. Its purpose is to ensure the enum is
 	 * wide enough to hold the higher bits reserved for bpf_type_flag.
 	 */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 6b9d23be1e99..62a86db7d8ec 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4941,10 +4941,14 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 	/* check for PTR_TO_RDONLY_BUF_OR_NULL or PTR_TO_RDWR_BUF_OR_NULL */
 	for (i = 0; i < prog->aux->ctx_arg_info_size; i++) {
 		const struct bpf_ctx_arg_aux *ctx_arg_info = &prog->aux->ctx_arg_info[i];
+		enum bpf_reg_type type;
+		enum bpf_type_flag flag;
 
+		type = BPF_BASE_TYPE(ctx_arg_info->reg_type);
+		flag = BPF_TYPE_FLAG(ctx_arg_info->reg_type);
 		if (ctx_arg_info->offset == off &&
-		    (ctx_arg_info->reg_type == PTR_TO_RDONLY_BUF_OR_NULL ||
-		     ctx_arg_info->reg_type == PTR_TO_RDWR_BUF_OR_NULL)) {
+		    (type == PTR_TO_RDWR_BUF || type == PTR_TO_RDONLY_BUF) &&
+		    (flag & PTR_MAYBE_NULL)) {
 			info->reg_type = ctx_arg_info->reg_type;
 			return true;
 		}
diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
index 6a9542af4212..631f0e44b7a9 100644
--- a/kernel/bpf/map_iter.c
+++ b/kernel/bpf/map_iter.c
@@ -174,9 +174,9 @@ static const struct bpf_iter_reg bpf_map_elem_reg_info = {
 	.ctx_arg_info_size	= 2,
 	.ctx_arg_info		= {
 		{ offsetof(struct bpf_iter__bpf_map_elem, key),
-		  PTR_TO_RDONLY_BUF_OR_NULL },
+		  PTR_TO_RDONLY_BUF | PTR_MAYBE_NULL },
 		{ offsetof(struct bpf_iter__bpf_map_elem, value),
-		  PTR_TO_RDWR_BUF_OR_NULL },
+		  PTR_TO_RDWR_BUF | PTR_MAYBE_NULL },
 	},
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fb44db2456f2..01a564a23562 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -441,14 +441,7 @@ static bool reg_type_not_null(enum bpf_reg_type type)
 
 static bool reg_type_may_be_null(enum bpf_reg_type type)
 {
-	return type == PTR_TO_MAP_VALUE_OR_NULL ||
-	       type == PTR_TO_SOCKET_OR_NULL ||
-	       type == PTR_TO_SOCK_COMMON_OR_NULL ||
-	       type == PTR_TO_TCP_SOCK_OR_NULL ||
-	       type == PTR_TO_BTF_ID_OR_NULL ||
-	       type == PTR_TO_MEM_OR_NULL ||
-	       type == PTR_TO_RDONLY_BUF_OR_NULL ||
-	       type == PTR_TO_RDWR_BUF_OR_NULL;
+	return BPF_TYPE_FLAG(type) & PTR_MAYBE_NULL;
 }
 
 static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
@@ -459,12 +452,9 @@ static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
 
 static bool reg_type_may_be_refcounted_or_null(enum bpf_reg_type type)
 {
-	return type == PTR_TO_SOCKET ||
-		type == PTR_TO_SOCKET_OR_NULL ||
-		type == PTR_TO_TCP_SOCK ||
-		type == PTR_TO_TCP_SOCK_OR_NULL ||
-		type == PTR_TO_MEM ||
-		type == PTR_TO_MEM_OR_NULL;
+	return BPF_BASE_TYPE(type) == PTR_TO_SOCKET ||
+		BPF_BASE_TYPE(type) == PTR_TO_TCP_SOCK ||
+		BPF_BASE_TYPE(type) == PTR_TO_MEM;
 }
 
 static bool arg_type_may_be_refcounted(enum bpf_arg_type type)
@@ -540,38 +530,35 @@ static bool is_cmpxchg_insn(const struct bpf_insn *insn)
 }
 
 /* string representation of 'enum bpf_reg_type' */
-static const char * const reg_type_str[] = {
-	[NOT_INIT]		= "?",
-	[SCALAR_VALUE]		= "inv",
-	[PTR_TO_CTX]		= "ctx",
-	[CONST_PTR_TO_MAP]	= "map_ptr",
-	[PTR_TO_MAP_VALUE]	= "map_value",
-	[PTR_TO_MAP_VALUE_OR_NULL] = "map_value_or_null",
-	[PTR_TO_STACK]		= "fp",
-	[PTR_TO_PACKET]		= "pkt",
-	[PTR_TO_PACKET_META]	= "pkt_meta",
-	[PTR_TO_PACKET_END]	= "pkt_end",
-	[PTR_TO_FLOW_KEYS]	= "flow_keys",
-	[PTR_TO_SOCKET]		= "sock",
-	[PTR_TO_SOCKET_OR_NULL] = "sock_or_null",
-	[PTR_TO_SOCK_COMMON]	= "sock_common",
-	[PTR_TO_SOCK_COMMON_OR_NULL] = "sock_common_or_null",
-	[PTR_TO_TCP_SOCK]	= "tcp_sock",
-	[PTR_TO_TCP_SOCK_OR_NULL] = "tcp_sock_or_null",
-	[PTR_TO_TP_BUFFER]	= "tp_buffer",
-	[PTR_TO_XDP_SOCK]	= "xdp_sock",
-	[PTR_TO_BTF_ID]		= "ptr_",
-	[PTR_TO_BTF_ID_OR_NULL]	= "ptr_or_null_",
-	[PTR_TO_PERCPU_BTF_ID]	= "percpu_ptr_",
-	[PTR_TO_MEM]		= "mem",
-	[PTR_TO_MEM_OR_NULL]	= "mem_or_null",
-	[PTR_TO_RDONLY_BUF]	= "rdonly_buf",
-	[PTR_TO_RDONLY_BUF_OR_NULL] = "rdonly_buf_or_null",
-	[PTR_TO_RDWR_BUF]	= "rdwr_buf",
-	[PTR_TO_RDWR_BUF_OR_NULL] = "rdwr_buf_or_null",
-	[PTR_TO_FUNC]		= "func",
-	[PTR_TO_MAP_KEY]	= "map_key",
-};
+static const char * const reg_type_str(enum bpf_reg_type type)
+{
+	static const char * const str[] = {
+		[NOT_INIT]		= "?",
+		[SCALAR_VALUE]		= "inv",
+		[PTR_TO_CTX]		= "ctx",
+		[CONST_PTR_TO_MAP]	= "map_ptr",
+		[PTR_TO_MAP_VALUE]	= "map_value",
+		[PTR_TO_STACK]		= "fp",
+		[PTR_TO_PACKET]		= "pkt",
+		[PTR_TO_PACKET_META]	= "pkt_meta",
+		[PTR_TO_PACKET_END]	= "pkt_end",
+		[PTR_TO_FLOW_KEYS]	= "flow_keys",
+		[PTR_TO_SOCKET]		= "sock",
+		[PTR_TO_SOCK_COMMON]	= "sock_common",
+		[PTR_TO_TCP_SOCK]	= "tcp_sock",
+		[PTR_TO_TP_BUFFER]	= "tp_buffer",
+		[PTR_TO_XDP_SOCK]	= "xdp_sock",
+		[PTR_TO_BTF_ID]		= "ptr_",
+		[PTR_TO_PERCPU_BTF_ID]	= "percpu_ptr_",
+		[PTR_TO_MEM]		= "mem",
+		[PTR_TO_RDONLY_BUF]	= "rdonly_buf",
+		[PTR_TO_RDWR_BUF]	= "rdwr_buf",
+		[PTR_TO_FUNC]		= "func",
+		[PTR_TO_MAP_KEY]	= "map_key",
+	};
+
+	return str[BPF_BASE_TYPE(type)];
+}
 
 static char slot_type_char[] = {
 	[STACK_INVALID]	= '?',
@@ -636,7 +623,7 @@ static void print_verifier_state(struct bpf_verifier_env *env,
 			continue;
 		verbose(env, " R%d", i);
 		print_liveness(env, reg->live);
-		verbose(env, "=%s", reg_type_str[t]);
+		verbose(env, "=%s", reg_type_str(t));
 		if (t == SCALAR_VALUE && reg->precise)
 			verbose(env, "P");
 		if ((t == SCALAR_VALUE || t == PTR_TO_STACK) &&
@@ -644,9 +631,8 @@ static void print_verifier_state(struct bpf_verifier_env *env,
 			/* reg->off should be 0 for SCALAR_VALUE */
 			verbose(env, "%lld", reg->var_off.value + reg->off);
 		} else {
-			if (t == PTR_TO_BTF_ID ||
-			    t == PTR_TO_BTF_ID_OR_NULL ||
-			    t == PTR_TO_PERCPU_BTF_ID)
+			if (BPF_BASE_TYPE(t) == PTR_TO_BTF_ID ||
+			    BPF_BASE_TYPE(t) == PTR_TO_PERCPU_BTF_ID)
 				verbose(env, "%s", kernel_type_name(reg->btf, reg->btf_id));
 			verbose(env, "(id=%d", reg->id);
 			if (reg_type_may_be_refcounted_or_null(t))
@@ -655,10 +641,9 @@ static void print_verifier_state(struct bpf_verifier_env *env,
 				verbose(env, ",off=%d", reg->off);
 			if (type_is_pkt_pointer(t))
 				verbose(env, ",r=%d", reg->range);
-			else if (t == CONST_PTR_TO_MAP ||
-				 t == PTR_TO_MAP_KEY ||
-				 t == PTR_TO_MAP_VALUE ||
-				 t == PTR_TO_MAP_VALUE_OR_NULL)
+			else if (BPF_BASE_TYPE(t) == CONST_PTR_TO_MAP ||
+				 BPF_BASE_TYPE(t) == PTR_TO_MAP_KEY ||
+				 BPF_BASE_TYPE(t) == PTR_TO_MAP_VALUE)
 				verbose(env, ",ks=%d,vs=%d",
 					reg->map_ptr->key_size,
 					reg->map_ptr->value_size);
@@ -728,7 +713,7 @@ static void print_verifier_state(struct bpf_verifier_env *env,
 		if (is_spilled_reg(&state->stack[i])) {
 			reg = &state->stack[i].spilled_ptr;
 			t = reg->type;
-			verbose(env, "=%s", reg_type_str[t]);
+			verbose(env, "=%s", reg_type_str(t));
 			if (t == SCALAR_VALUE && reg->precise)
 				verbose(env, "P");
 			if (t == SCALAR_VALUE && tnum_is_const(reg->var_off))
@@ -1141,8 +1126,7 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env,
 
 static void mark_ptr_not_null_reg(struct bpf_reg_state *reg)
 {
-	switch (reg->type) {
-	case PTR_TO_MAP_VALUE_OR_NULL: {
+	if (BPF_BASE_TYPE(reg->type) == PTR_TO_MAP_VALUE) {
 		const struct bpf_map *map = reg->map_ptr;
 
 		if (map->inner_map_meta) {
@@ -1160,32 +1144,10 @@ static void mark_ptr_not_null_reg(struct bpf_reg_state *reg)
 		} else {
 			reg->type = PTR_TO_MAP_VALUE;
 		}
-		break;
-	}
-	case PTR_TO_SOCKET_OR_NULL:
-		reg->type = PTR_TO_SOCKET;
-		break;
-	case PTR_TO_SOCK_COMMON_OR_NULL:
-		reg->type = PTR_TO_SOCK_COMMON;
-		break;
-	case PTR_TO_TCP_SOCK_OR_NULL:
-		reg->type = PTR_TO_TCP_SOCK;
-		break;
-	case PTR_TO_BTF_ID_OR_NULL:
-		reg->type = PTR_TO_BTF_ID;
-		break;
-	case PTR_TO_MEM_OR_NULL:
-		reg->type = PTR_TO_MEM;
-		break;
-	case PTR_TO_RDONLY_BUF_OR_NULL:
-		reg->type = PTR_TO_RDONLY_BUF;
-		break;
-	case PTR_TO_RDWR_BUF_OR_NULL:
-		reg->type = PTR_TO_RDWR_BUF;
-		break;
-	default:
-		WARN_ONCE(1, "unknown nullable register type");
+		return;
 	}
+
+	reg->type &= ~PTR_MAYBE_NULL;
 }
 
 static bool reg_is_pkt_pointer(const struct bpf_reg_state *reg)
@@ -2040,7 +2002,7 @@ static int mark_reg_read(struct bpf_verifier_env *env,
 			break;
 		if (parent->live & REG_LIVE_DONE) {
 			verbose(env, "verifier BUG type %s var_off %lld off %d\n",
-				reg_type_str[parent->type],
+				reg_type_str(parent->type),
 				parent->var_off.value, parent->off);
 			return -EFAULT;
 		}
@@ -2703,9 +2665,8 @@ static int mark_chain_precision_stack(struct bpf_verifier_env *env, int spi)
 
 static bool is_spillable_regtype(enum bpf_reg_type type)
 {
-	switch (type) {
+	switch (BPF_BASE_TYPE(type)) {
 	case PTR_TO_MAP_VALUE:
-	case PTR_TO_MAP_VALUE_OR_NULL:
 	case PTR_TO_STACK:
 	case PTR_TO_CTX:
 	case PTR_TO_PACKET:
@@ -2714,21 +2675,14 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
 	case PTR_TO_FLOW_KEYS:
 	case CONST_PTR_TO_MAP:
 	case PTR_TO_SOCKET:
-	case PTR_TO_SOCKET_OR_NULL:
 	case PTR_TO_SOCK_COMMON:
-	case PTR_TO_SOCK_COMMON_OR_NULL:
 	case PTR_TO_TCP_SOCK:
-	case PTR_TO_TCP_SOCK_OR_NULL:
 	case PTR_TO_XDP_SOCK:
 	case PTR_TO_BTF_ID:
-	case PTR_TO_BTF_ID_OR_NULL:
 	case PTR_TO_RDONLY_BUF:
-	case PTR_TO_RDONLY_BUF_OR_NULL:
 	case PTR_TO_RDWR_BUF:
-	case PTR_TO_RDWR_BUF_OR_NULL:
 	case PTR_TO_PERCPU_BTF_ID:
 	case PTR_TO_MEM:
-	case PTR_TO_MEM_OR_NULL:
 	case PTR_TO_FUNC:
 	case PTR_TO_MAP_KEY:
 		return true;
@@ -3569,7 +3523,7 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
 		 */
 		*reg_type = info.reg_type;
 
-		if (*reg_type == PTR_TO_BTF_ID || *reg_type == PTR_TO_BTF_ID_OR_NULL) {
+		if (BPF_BASE_TYPE(*reg_type) == PTR_TO_BTF_ID) {
 			*btf = info.btf;
 			*btf_id = info.btf_id;
 		} else {
@@ -3637,7 +3591,7 @@ static int check_sock_access(struct bpf_verifier_env *env, int insn_idx,
 	}
 
 	verbose(env, "R%d invalid %s access off=%d size=%d\n",
-		regno, reg_type_str[reg->type], off, size);
+		regno, reg_type_str(reg->type), off, size);
 
 	return -EACCES;
 }
@@ -4395,8 +4349,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 				 * a sub-register.
 				 */
 				regs[value_regno].subreg_def = DEF_NOT_SUBREG;
-				if (reg_type == PTR_TO_BTF_ID ||
-				    reg_type == PTR_TO_BTF_ID_OR_NULL) {
+				if (BPF_BASE_TYPE(reg_type) == PTR_TO_BTF_ID) {
 					regs[value_regno].btf = btf;
 					regs[value_regno].btf_id = btf_id;
 				}
@@ -4449,7 +4402,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 	} else if (type_is_sk_pointer(reg->type)) {
 		if (t == BPF_WRITE) {
 			verbose(env, "R%d cannot write into %s\n",
-				regno, reg_type_str[reg->type]);
+				regno, reg_type_str(reg->type));
 			return -EACCES;
 		}
 		err = check_sock_access(env, insn_idx, regno, off, size, t);
@@ -4468,7 +4421,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 	} else if (reg->type == PTR_TO_RDONLY_BUF) {
 		if (t == BPF_WRITE) {
 			verbose(env, "R%d cannot write into %s\n",
-				regno, reg_type_str[reg->type]);
+				regno, reg_type_str(reg->type));
 			return -EACCES;
 		}
 		err = check_buffer_access(env, reg, regno, off, size, false,
@@ -4484,7 +4437,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 			mark_reg_unknown(env, regs, value_regno);
 	} else {
 		verbose(env, "R%d invalid mem access '%s'\n", regno,
-			reg_type_str[reg->type]);
+			reg_type_str(reg->type));
 		return -EACCES;
 	}
 
@@ -4551,7 +4504,7 @@ static int check_atomic(struct bpf_verifier_env *env, int insn_idx, struct bpf_i
 	    is_sk_reg(env, insn->dst_reg)) {
 		verbose(env, "BPF_ATOMIC stores into R%d %s is not allowed\n",
 			insn->dst_reg,
-			reg_type_str[reg_state(env, insn->dst_reg)->type]);
+			reg_type_str(reg_state(env, insn->dst_reg)->type));
 		return -EACCES;
 	}
 
@@ -4772,8 +4725,8 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
 			return 0;
 
 		verbose(env, "R%d type=%s expected=%s\n", regno,
-			reg_type_str[reg->type],
-			reg_type_str[PTR_TO_STACK]);
+			reg_type_str(reg->type),
+			reg_type_str(PTR_TO_STACK));
 		return -EACCES;
 	}
 }
@@ -5118,10 +5071,10 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
 			goto found;
 	}
 
-	verbose(env, "R%d type=%s expected=", regno, reg_type_str[type]);
+	verbose(env, "R%d type=%s expected=", regno, reg_type_str(type));
 	for (j = 0; j + 1 < i; j++)
-		verbose(env, "%s, ", reg_type_str[compatible->types[j]]);
-	verbose(env, "%s\n", reg_type_str[compatible->types[j]]);
+		verbose(env, "%s, ", reg_type_str(compatible->types[j]));
+	verbose(env, "%s\n", reg_type_str(compatible->types[j]));
 	return -EACCES;
 
 found:
@@ -6376,6 +6329,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 {
 	const struct bpf_func_proto *fn = NULL;
 	enum bpf_return_type ret_type;
+	enum bpf_type_flag ret_flag;
 	struct bpf_reg_state *regs;
 	struct bpf_call_arg_meta meta;
 	int insn_idx = *insn_idx_p;
@@ -6517,6 +6471,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 
 	/* update return register (already marked as written above) */
 	ret_type = fn->ret_type;
+	ret_flag = BPF_TYPE_FLAG(fn->ret_type);
 	if (ret_type == RET_INTEGER) {
 		/* sets type to SCALAR_VALUE */
 		mark_reg_unknown(env, regs, BPF_REG_0);
@@ -6536,25 +6491,23 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 		}
 		regs[BPF_REG_0].map_ptr = meta.map_ptr;
 		regs[BPF_REG_0].map_uid = meta.map_uid;
-		if (ret_type_may_be_null(fn->ret_type)) {
-			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
-		} else {
-			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;
-			if (map_value_has_spin_lock(meta.map_ptr))
-				regs[BPF_REG_0].id = ++env->id_gen;
+		regs[BPF_REG_0].type = PTR_TO_MAP_VALUE | ret_flag;
+		if (!ret_type_may_be_null(fn->ret_type) &&
+		    map_value_has_spin_lock(meta.map_ptr)) {
+			regs[BPF_REG_0].id = ++env->id_gen;
 		}
 	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCKET) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
-		regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
+		regs[BPF_REG_0].type = PTR_TO_SOCKET | ret_flag;
 	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCK_COMMON) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
-		regs[BPF_REG_0].type = PTR_TO_SOCK_COMMON_OR_NULL;
+		regs[BPF_REG_0].type = PTR_TO_SOCK_COMMON | ret_flag;
 	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_TCP_SOCK) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
-		regs[BPF_REG_0].type = PTR_TO_TCP_SOCK_OR_NULL;
+		regs[BPF_REG_0].type = PTR_TO_TCP_SOCK | ret_flag;
 	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_ALLOC_MEM) {
 		mark_reg_known_zero(env, regs, BPF_REG_0);
-		regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
+		regs[BPF_REG_0].type = PTR_TO_MEM | ret_flag;
 		regs[BPF_REG_0].mem_size = meta.mem_size;
 	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MEM_OR_BTF_ID) {
 		const struct btf_type *t;
@@ -6574,14 +6527,10 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 					tname, PTR_ERR(ret));
 				return -EINVAL;
 			}
-			regs[BPF_REG_0].type =
-				(ret_type & PTR_MAYBE_NULL) ?
-				PTR_TO_MEM_OR_NULL : PTR_TO_MEM;
+			regs[BPF_REG_0].type = PTR_TO_MEM | ret_flag;
 			regs[BPF_REG_0].mem_size = tsize;
 		} else {
-			regs[BPF_REG_0].type =
-				(ret_type & PTR_MAYBE_NULL) ?
-				PTR_TO_BTF_ID_OR_NULL : PTR_TO_BTF_ID;
+			regs[BPF_REG_0].type = PTR_TO_BTF_ID | ret_flag;
 			regs[BPF_REG_0].btf = meta.ret_btf;
 			regs[BPF_REG_0].btf_id = meta.ret_btf_id;
 		}
@@ -6589,9 +6538,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 		int ret_btf_id;
 
 		mark_reg_known_zero(env, regs, BPF_REG_0);
-		regs[BPF_REG_0].type = (ret_type & PTR_MAYBE_NULL) ?
-						     PTR_TO_BTF_ID_OR_NULL :
-						     PTR_TO_BTF_ID;
+		regs[BPF_REG_0].type = PTR_TO_BTF_ID | ret_flag;
 		ret_btf_id = *fn->ret_btf_id;
 		if (ret_btf_id == 0) {
 			verbose(env, "invalid return type %d of func %s#%d\n",
@@ -6819,25 +6766,25 @@ static bool check_reg_sane_offset(struct bpf_verifier_env *env,
 
 	if (known && (val >= BPF_MAX_VAR_OFF || val <= -BPF_MAX_VAR_OFF)) {
 		verbose(env, "math between %s pointer and %lld is not allowed\n",
-			reg_type_str[type], val);
+			reg_type_str(type), val);
 		return false;
 	}
 
 	if (reg->off >= BPF_MAX_VAR_OFF || reg->off <= -BPF_MAX_VAR_OFF) {
 		verbose(env, "%s pointer offset %d is not allowed\n",
-			reg_type_str[type], reg->off);
+			reg_type_str(type), reg->off);
 		return false;
 	}
 
 	if (smin == S64_MIN) {
 		verbose(env, "math between %s pointer and register with unbounded min value is not allowed\n",
-			reg_type_str[type]);
+			reg_type_str(type));
 		return false;
 	}
 
 	if (smin >= BPF_MAX_VAR_OFF || smin <= -BPF_MAX_VAR_OFF) {
 		verbose(env, "value %lld makes %s pointer be out of bounds\n",
-			smin, reg_type_str[type]);
+			smin, reg_type_str(type));
 		return false;
 	}
 
@@ -7214,11 +7161,13 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
-	switch (ptr_reg->type) {
-	case PTR_TO_MAP_VALUE_OR_NULL:
+	if (ptr_reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
 		verbose(env, "R%d pointer arithmetic on %s prohibited, null-check it first\n",
-			dst, reg_type_str[ptr_reg->type]);
+			dst, reg_type_str(ptr_reg->type));
 		return -EACCES;
+	}
+
+	switch (BPF_BASE_TYPE(ptr_reg->type)) {
 	case CONST_PTR_TO_MAP:
 		/* smin_val represents the known value */
 		if (known && smin_val == 0 && opcode == BPF_ADD)
@@ -7226,14 +7175,11 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
 		fallthrough;
 	case PTR_TO_PACKET_END:
 	case PTR_TO_SOCKET:
-	case PTR_TO_SOCKET_OR_NULL:
 	case PTR_TO_SOCK_COMMON:
-	case PTR_TO_SOCK_COMMON_OR_NULL:
 	case PTR_TO_TCP_SOCK:
-	case PTR_TO_TCP_SOCK_OR_NULL:
 	case PTR_TO_XDP_SOCK:
 		verbose(env, "R%d pointer arithmetic on %s prohibited\n",
-			dst, reg_type_str[ptr_reg->type]);
+			dst, reg_type_str(ptr_reg->type));
 		return -EACCES;
 	default:
 		break;
@@ -9584,7 +9530,7 @@ static int check_return_code(struct bpf_verifier_env *env)
 		/* enforce return zero from async callbacks like timer */
 		if (reg->type != SCALAR_VALUE) {
 			verbose(env, "In async callback the register R0 is not a known value (%s)\n",
-				reg_type_str[reg->type]);
+				reg_type_str(reg->type));
 			return -EINVAL;
 		}
 
@@ -9598,7 +9544,7 @@ static int check_return_code(struct bpf_verifier_env *env)
 	if (is_subprog) {
 		if (reg->type != SCALAR_VALUE) {
 			verbose(env, "At subprogram exit the register R0 is not a scalar value (%s)\n",
-				reg_type_str[reg->type]);
+				reg_type_str(reg->type));
 			return -EINVAL;
 		}
 		return 0;
@@ -9662,7 +9608,7 @@ static int check_return_code(struct bpf_verifier_env *env)
 
 	if (reg->type != SCALAR_VALUE) {
 		verbose(env, "At program exit the register R0 is not a known value (%s)\n",
-			reg_type_str[reg->type]);
+			reg_type_str(reg->type));
 		return -EINVAL;
 	}
 
@@ -10443,7 +10389,7 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
 		return true;
 	if (rcur->type == NOT_INIT)
 		return false;
-	switch (rold->type) {
+	switch (BPF_BASE_TYPE(rold->type)) {
 	case SCALAR_VALUE:
 		if (env->explore_alu_limits)
 			return false;
@@ -10465,6 +10411,22 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
 		}
 	case PTR_TO_MAP_KEY:
 	case PTR_TO_MAP_VALUE:
+		/* a PTR_TO_MAP_VALUE could be safe to use as a
+		 * PTR_TO_MAP_VALUE_OR_NULL into the same map.
+		 * However, if the old PTR_TO_MAP_VALUE_OR_NULL then got NULL-
+		 * checked, doing so could have affected others with the same
+		 * id, and we can't check for that because we lost the id when
+		 * we converted to a PTR_TO_MAP_VALUE.
+		 */
+		if (reg_type_may_be_null(rold->type)) {
+			if (!reg_type_may_be_null(rcur->type))
+				return false;
+			if (memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)))
+				return false;
+			/* Check our ids match any regs they're supposed to */
+			return check_ids(rold->id, rcur->id, idmap);
+		}
+
 		/* If the new min/max/var_off satisfy the old ones and
 		 * everything else matches, we are OK.
 		 * 'id' is not compared, since it's only used for maps with
@@ -10476,20 +10438,6 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
 		return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
 		       range_within(rold, rcur) &&
 		       tnum_in(rold->var_off, rcur->var_off);
-	case PTR_TO_MAP_VALUE_OR_NULL:
-		/* a PTR_TO_MAP_VALUE could be safe to use as a
-		 * PTR_TO_MAP_VALUE_OR_NULL into the same map.
-		 * However, if the old PTR_TO_MAP_VALUE_OR_NULL then got NULL-
-		 * checked, doing so could have affected others with the same
-		 * id, and we can't check for that because we lost the id when
-		 * we converted to a PTR_TO_MAP_VALUE.
-		 */
-		if (rcur->type != PTR_TO_MAP_VALUE_OR_NULL)
-			return false;
-		if (memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)))
-			return false;
-		/* Check our ids match any regs they're supposed to */
-		return check_ids(rold->id, rcur->id, idmap);
 	case PTR_TO_PACKET_META:
 	case PTR_TO_PACKET:
 		if (rcur->type != rold->type)
@@ -10518,11 +10466,8 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
 	case PTR_TO_PACKET_END:
 	case PTR_TO_FLOW_KEYS:
 	case PTR_TO_SOCKET:
-	case PTR_TO_SOCKET_OR_NULL:
 	case PTR_TO_SOCK_COMMON:
-	case PTR_TO_SOCK_COMMON_OR_NULL:
 	case PTR_TO_TCP_SOCK:
-	case PTR_TO_TCP_SOCK_OR_NULL:
 	case PTR_TO_XDP_SOCK:
 		/* Only valid matches are exact, which memcmp() above
 		 * would have accepted
@@ -11048,17 +10993,13 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
 /* Return true if it's OK to have the same insn return a different type. */
 static bool reg_type_mismatch_ok(enum bpf_reg_type type)
 {
-	switch (type) {
+	switch (BPF_BASE_TYPE(type)) {
 	case PTR_TO_CTX:
 	case PTR_TO_SOCKET:
-	case PTR_TO_SOCKET_OR_NULL:
 	case PTR_TO_SOCK_COMMON:
-	case PTR_TO_SOCK_COMMON_OR_NULL:
 	case PTR_TO_TCP_SOCK:
-	case PTR_TO_TCP_SOCK_OR_NULL:
 	case PTR_TO_XDP_SOCK:
 	case PTR_TO_BTF_ID:
-	case PTR_TO_BTF_ID_OR_NULL:
 		return false;
 	default:
 		return true;
@@ -11282,7 +11223,7 @@ static int do_check(struct bpf_verifier_env *env)
 			if (is_ctx_reg(env, insn->dst_reg)) {
 				verbose(env, "BPF_ST stores into R%d %s is not allowed\n",
 					insn->dst_reg,
-					reg_type_str[reg_state(env, insn->dst_reg)->type]);
+					reg_type_str(reg_state(env, insn->dst_reg)->type));
 				return -EACCES;
 			}
 
diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index 68d2cbf8331a..4cb5ef8eddbc 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -929,7 +929,7 @@ static struct bpf_iter_reg bpf_sk_storage_map_reg_info = {
 		{ offsetof(struct bpf_iter__bpf_sk_storage_map, sk),
 		  PTR_TO_BTF_ID_OR_NULL },
 		{ offsetof(struct bpf_iter__bpf_sk_storage_map, value),
-		  PTR_TO_RDWR_BUF_OR_NULL },
+		  PTR_TO_RDWR_BUF | PTR_MAYBE_NULL },
 	},
 	.seq_info		= &iter_seq_info,
 };
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index f39ef79ced67..8b2632be3771 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -1559,7 +1559,7 @@ static struct bpf_iter_reg sock_map_iter_reg = {
 	.ctx_arg_info_size	= 2,
 	.ctx_arg_info		= {
 		{ offsetof(struct bpf_iter__sockmap, key),
-		  PTR_TO_RDONLY_BUF_OR_NULL },
+		  PTR_TO_RDONLY_BUF | PTR_MAYBE_NULL },
 		{ offsetof(struct bpf_iter__sockmap, sk),
 		  PTR_TO_BTF_ID_OR_NULL },
 	},
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 5/9] bpf: Introduce MEM_RDONLY flag
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
                   ` (3 preceding siblings ...)
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX " Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 6/9] bpf: Convert PTR_TO_MEM_OR_NULL to composable types Hao Luo
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

This patch introduce another flag MEM_RDONLY to tag a reg value is
pointing to a read-only memory. It makes the following changes:

1. PTR_TO_RDWR_BUF -> PTR_TO_BUF
2. PTR_TO_RDONLY_BUF -> PTR_TO_BUF | MEM_RDONLY

Signed-off-by: Hao Luo <haoluo@google.com>
---
 include/linux/bpf.h       |  8 +++--
 kernel/bpf/btf.c          |  3 +-
 kernel/bpf/map_iter.c     |  4 +--
 kernel/bpf/verifier.c     | 76 +++++++++++++++++++++++----------------
 net/core/bpf_sk_storage.c |  2 +-
 net/core/sock_map.c       |  2 +-
 6 files changed, 55 insertions(+), 40 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d484f6637e60..61b72dbaeae8 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -311,7 +311,10 @@ enum bpf_type_flag {
 	/* PTR may be NULL. */
 	PTR_MAYBE_NULL		= BIT(0 + BPF_BASE_TYPE_BITS),
 
-	__BPF_TYPE_LAST_FLAG	= PTR_MAYBE_NULL,
+	/* MEM is read-only. */
+	MEM_RDONLY		= BIT(1 + BPF_BASE_TYPE_BITS),
+
+	__BPF_TYPE_LAST_FLAG	= MEM_RDONLY,
 };
 
 #define BPF_BASE_TYPE_MASK	GENMASK(BPF_BASE_TYPE_BITS, 0)
@@ -499,8 +502,7 @@ enum bpf_reg_type {
 	 * an explicit null check is required for this struct.
 	 */
 	PTR_TO_MEM,		 /* reg points to valid memory region */
-	PTR_TO_RDONLY_BUF,	 /* reg points to a readonly buffer */
-	PTR_TO_RDWR_BUF,	 /* reg points to a read/write buffer */
+	PTR_TO_BUF,		 /* reg points to a read/write buffer */
 	PTR_TO_PERCPU_BTF_ID,	 /* reg points to a percpu kernel variable */
 	PTR_TO_FUNC,		 /* reg points to a bpf program function */
 	PTR_TO_MAP_KEY,		 /* reg points to a map element key */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 62a86db7d8ec..19ddd6fe5663 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4946,8 +4946,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 
 		type = BPF_BASE_TYPE(ctx_arg_info->reg_type);
 		flag = BPF_TYPE_FLAG(ctx_arg_info->reg_type);
-		if (ctx_arg_info->offset == off &&
-		    (type == PTR_TO_RDWR_BUF || type == PTR_TO_RDONLY_BUF) &&
+		if (ctx_arg_info->offset == off && type == PTR_TO_BUF &&
 		    (flag & PTR_MAYBE_NULL)) {
 			info->reg_type = ctx_arg_info->reg_type;
 			return true;
diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
index 631f0e44b7a9..b0fa190b0979 100644
--- a/kernel/bpf/map_iter.c
+++ b/kernel/bpf/map_iter.c
@@ -174,9 +174,9 @@ static const struct bpf_iter_reg bpf_map_elem_reg_info = {
 	.ctx_arg_info_size	= 2,
 	.ctx_arg_info		= {
 		{ offsetof(struct bpf_iter__bpf_map_elem, key),
-		  PTR_TO_RDONLY_BUF | PTR_MAYBE_NULL },
+		  PTR_TO_BUF | PTR_MAYBE_NULL | MEM_RDONLY },
 		{ offsetof(struct bpf_iter__bpf_map_elem, value),
-		  PTR_TO_RDWR_BUF | PTR_MAYBE_NULL },
+		  PTR_TO_BUF | PTR_MAYBE_NULL },
 	},
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 01a564a23562..27f3440f4b18 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -457,6 +457,11 @@ static bool reg_type_may_be_refcounted_or_null(enum bpf_reg_type type)
 		BPF_BASE_TYPE(type) == PTR_TO_MEM;
 }
 
+static bool reg_type_is_rdonly_mem(enum bpf_reg_type type)
+{
+	return BPF_TYPE_FLAG(type) & MEM_RDONLY;
+}
+
 static bool arg_type_may_be_refcounted(enum bpf_arg_type type)
 {
 	return type == ARG_PTR_TO_SOCK_COMMON;
@@ -551,8 +556,7 @@ static const char * const reg_type_str(enum bpf_reg_type type)
 		[PTR_TO_BTF_ID]		= "ptr_",
 		[PTR_TO_PERCPU_BTF_ID]	= "percpu_ptr_",
 		[PTR_TO_MEM]		= "mem",
-		[PTR_TO_RDONLY_BUF]	= "rdonly_buf",
-		[PTR_TO_RDWR_BUF]	= "rdwr_buf",
+		[PTR_TO_BUF]		= "rdwr_buf",
 		[PTR_TO_FUNC]		= "func",
 		[PTR_TO_MAP_KEY]	= "map_key",
 	};
@@ -2679,8 +2683,7 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
 	case PTR_TO_TCP_SOCK:
 	case PTR_TO_XDP_SOCK:
 	case PTR_TO_BTF_ID:
-	case PTR_TO_RDONLY_BUF:
-	case PTR_TO_RDWR_BUF:
+	case PTR_TO_BUF:
 	case PTR_TO_PERCPU_BTF_ID:
 	case PTR_TO_MEM:
 	case PTR_TO_FUNC:
@@ -4418,23 +4421,30 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 	} else if (reg->type == CONST_PTR_TO_MAP) {
 		err = check_ptr_to_map_access(env, regs, regno, off, size, t,
 					      value_regno);
-	} else if (reg->type == PTR_TO_RDONLY_BUF) {
-		if (t == BPF_WRITE) {
-			verbose(env, "R%d cannot write into %s\n",
-				regno, reg_type_str(reg->type));
-			return -EACCES;
+	} else if (BPF_BASE_TYPE(reg->type) == PTR_TO_BUF) {
+		bool rdonly_mem = reg_type_is_rdonly_mem(reg->type);
+		const char *buf_info;
+		u32 *max_access;
+
+		if (rdonly_mem) {
+			if (t == BPF_WRITE) {
+				verbose(env, "R%d cannot write into rdonly %s\n",
+					regno, reg_type_str(reg->type));
+				return -EACCES;
+			}
+			buf_info = "rdonly";
+			max_access = &env->prog->aux->max_rdonly_access;
+		} else {
+			buf_info = "rdwr";
+			max_access = &env->prog->aux->max_rdwr_access;
 		}
+
 		err = check_buffer_access(env, reg, regno, off, size, false,
-					  "rdonly",
-					  &env->prog->aux->max_rdonly_access);
+					  buf_info, max_access);
+
 		if (!err && value_regno >= 0)
-			mark_reg_unknown(env, regs, value_regno);
-	} else if (reg->type == PTR_TO_RDWR_BUF) {
-		err = check_buffer_access(env, reg, regno, off, size, false,
-					  "rdwr",
-					  &env->prog->aux->max_rdwr_access);
-		if (!err && t == BPF_READ && value_regno >= 0)
-			mark_reg_unknown(env, regs, value_regno);
+			if (rdonly_mem || t == BPF_READ)
+				mark_reg_unknown(env, regs, value_regno);
 	} else {
 		verbose(env, "R%d invalid mem access '%s'\n", regno,
 			reg_type_str(reg->type));
@@ -4681,8 +4691,10 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
 				   struct bpf_call_arg_meta *meta)
 {
 	struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
+	const char *buf_info;
+	u32 *max_access;
 
-	switch (reg->type) {
+	switch (BPF_BASE_TYPE(reg->type)) {
 	case PTR_TO_PACKET:
 	case PTR_TO_PACKET_META:
 		return check_packet_access(env, regno, reg->off, access_size,
@@ -4701,18 +4713,20 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
 		return check_mem_region_access(env, regno, reg->off,
 					       access_size, reg->mem_size,
 					       zero_size_allowed);
-	case PTR_TO_RDONLY_BUF:
-		if (meta && meta->raw_mode)
-			return -EACCES;
-		return check_buffer_access(env, reg, regno, reg->off,
-					   access_size, zero_size_allowed,
-					   "rdonly",
-					   &env->prog->aux->max_rdonly_access);
-	case PTR_TO_RDWR_BUF:
+	case PTR_TO_BUF:
+		if (reg_type_is_rdonly_mem(reg->type)) {
+			if (meta && meta->raw_mode)
+				return -EACCES;
+
+			buf_info = "rdonly";
+			max_access = &env->prog->aux->max_rdonly_access;
+		} else {
+			buf_info = "rdwr";
+			max_access = &env->prog->aux->max_rdwr_access;
+		}
 		return check_buffer_access(env, reg, regno, reg->off,
 					   access_size, zero_size_allowed,
-					   "rdwr",
-					   &env->prog->aux->max_rdwr_access);
+					   buf_info, max_access);
 	case PTR_TO_STACK:
 		return check_stack_range_initialized(
 				env,
@@ -4991,8 +5005,8 @@ static const struct bpf_reg_types mem_types = {
 		PTR_TO_MAP_KEY,
 		PTR_TO_MAP_VALUE,
 		PTR_TO_MEM,
-		PTR_TO_RDONLY_BUF,
-		PTR_TO_RDWR_BUF,
+		PTR_TO_BUF,
+		PTR_TO_BUF | MEM_RDONLY,
 	},
 };
 
diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index 4cb5ef8eddbc..ea61dfe19c86 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -929,7 +929,7 @@ static struct bpf_iter_reg bpf_sk_storage_map_reg_info = {
 		{ offsetof(struct bpf_iter__bpf_sk_storage_map, sk),
 		  PTR_TO_BTF_ID_OR_NULL },
 		{ offsetof(struct bpf_iter__bpf_sk_storage_map, value),
-		  PTR_TO_RDWR_BUF | PTR_MAYBE_NULL },
+		  PTR_TO_BUF | PTR_MAYBE_NULL },
 	},
 	.seq_info		= &iter_seq_info,
 };
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 8b2632be3771..005bf58b1148 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -1559,7 +1559,7 @@ static struct bpf_iter_reg sock_map_iter_reg = {
 	.ctx_arg_info_size	= 2,
 	.ctx_arg_info		= {
 		{ offsetof(struct bpf_iter__sockmap, key),
-		  PTR_TO_RDONLY_BUF | PTR_MAYBE_NULL },
+		  PTR_TO_BUF | PTR_MAYBE_NULL | MEM_RDONLY },
 		{ offsetof(struct bpf_iter__sockmap, sk),
 		  PTR_TO_BTF_ID_OR_NULL },
 	},
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 6/9] bpf: Convert PTR_TO_MEM_OR_NULL to composable types.
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
                   ` (4 preceding siblings ...)
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 5/9] bpf: Introduce MEM_RDONLY flag Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 7/9] bpf: Make per_cpu_ptr return rdonly PTR_TO_MEM Hao Luo
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

Remove PTR_TO_MEM_OR_NULL and replace it with PTR_TO_MEM combined with
flag PTR_MAYBE_NULL.

Signed-off-by: Hao Luo <haoluo@google.com>
---
 include/linux/bpf.h   | 1 -
 kernel/bpf/btf.c      | 2 +-
 kernel/bpf/verifier.c | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 61b72dbaeae8..19d27d5a1d94 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -514,7 +514,6 @@ enum bpf_reg_type {
 	PTR_TO_SOCK_COMMON_OR_NULL	= PTR_MAYBE_NULL | PTR_TO_SOCK_COMMON,
 	PTR_TO_TCP_SOCK_OR_NULL		= PTR_MAYBE_NULL | PTR_TO_TCP_SOCK,
 	PTR_TO_BTF_ID_OR_NULL		= PTR_MAYBE_NULL | PTR_TO_BTF_ID,
-	PTR_TO_MEM_OR_NULL		= PTR_MAYBE_NULL | PTR_TO_MEM,
 
 	/* This must be the last entry. Its purpose is to ensure the enum is
 	 * wide enough to hold the higher bits reserved for bpf_type_flag.
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 19ddd6fe5663..776b90de0971 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5861,7 +5861,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog,
 				return -EINVAL;
 			}
 
-			reg->type = PTR_TO_MEM_OR_NULL;
+			reg->type = PTR_TO_MEM | PTR_MAYBE_NULL;
 			reg->id = ++env->id_gen;
 
 			continue;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 27f3440f4b18..72c7135bee3e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13313,7 +13313,7 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
 				mark_reg_known_zero(env, regs, i);
 			else if (regs[i].type == SCALAR_VALUE)
 				mark_reg_unknown(env, regs, i);
-			else if (regs[i].type == PTR_TO_MEM_OR_NULL) {
+			else if (BPF_BASE_TYPE(regs[i].type) == PTR_TO_MEM) {
 				const u32 mem_size = regs[i].mem_size;
 
 				mark_reg_known_zero(env, regs, i);
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 7/9] bpf: Make per_cpu_ptr return rdonly PTR_TO_MEM.
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
                   ` (5 preceding siblings ...)
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 6/9] bpf: Convert PTR_TO_MEM_OR_NULL to composable types Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem Hao Luo
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 9/9] bpf/selftests: Test PTR_TO_RDONLY_MEM Hao Luo
  8 siblings, 0 replies; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

Tag the return type of {per, this}_cpu_ptr with RDONLY_MEM. The
returned value of this pair of helpers is kernel object, which
can not be updated by bpf programs. In addition to tagging these two
helpers, the verifier is now able to differentiate RDONLY MEM from
RDWR MEM.

Fixes: 63d9b80dcf2c ("bpf: Introducte bpf_this_cpu_ptr()")
Fixes: eaa6bcb71ef6 ("bpf: Introduce bpf_per_cpu_ptr()")
Fixes: 4976b718c355 ("bpf: Introduce pseudo_btf_id")
Signed-off-by: Hao Luo <haoluo@google.com>
---
 kernel/bpf/helpers.c  |  4 ++--
 kernel/bpf/verifier.c | 24 ++++++++++++++++++++----
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 293d9314ec7f..a5e349c9d3e3 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -667,7 +667,7 @@ BPF_CALL_2(bpf_per_cpu_ptr, const void *, ptr, u32, cpu)
 const struct bpf_func_proto bpf_per_cpu_ptr_proto = {
 	.func		= bpf_per_cpu_ptr,
 	.gpl_only	= false,
-	.ret_type	= RET_PTR_TO_MEM_OR_BTF_ID | PTR_MAYBE_NULL,
+	.ret_type	= RET_PTR_TO_MEM_OR_BTF_ID | PTR_MAYBE_NULL | MEM_RDONLY,
 	.arg1_type	= ARG_PTR_TO_PERCPU_BTF_ID,
 	.arg2_type	= ARG_ANYTHING,
 };
@@ -680,7 +680,7 @@ BPF_CALL_1(bpf_this_cpu_ptr, const void *, percpu_ptr)
 const struct bpf_func_proto bpf_this_cpu_ptr_proto = {
 	.func		= bpf_this_cpu_ptr,
 	.gpl_only	= false,
-	.ret_type	= RET_PTR_TO_MEM_OR_BTF_ID,
+	.ret_type	= RET_PTR_TO_MEM_OR_BTF_ID | MEM_RDONLY,
 	.arg1_type	= ARG_PTR_TO_PERCPU_BTF_ID,
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 72c7135bee3e..e2663544362a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4306,16 +4306,32 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 				mark_reg_unknown(env, regs, value_regno);
 			}
 		}
-	} else if (reg->type == PTR_TO_MEM) {
+	} else if (BPF_BASE_TYPE(reg->type) == PTR_TO_MEM) {
+		bool rdonly_mem = reg_type_is_rdonly_mem(reg->type);
+
+		if (reg_type_may_be_null(reg->type)) {
+			verbose(env, "R%d invalid mem access '%s'\n", regno,
+				reg_type_str(reg->type));
+			return -EACCES;
+		}
+
+		if (t == BPF_WRITE && rdonly_mem) {
+			verbose(env, "R%d cannot write into rdonly %s\n",
+				regno, reg_type_str(reg->type));
+			return -EACCES;
+		}
+
 		if (t == BPF_WRITE && value_regno >= 0 &&
 		    is_pointer_value(env, value_regno)) {
 			verbose(env, "R%d leaks addr into mem\n", value_regno);
 			return -EACCES;
 		}
+
 		err = check_mem_region_access(env, regno, off, size,
 					      reg->mem_size, false);
 		if (!err && t == BPF_READ && value_regno >= 0)
-			mark_reg_unknown(env, regs, value_regno);
+			if (t == BPF_READ || rdonly_mem)
+				mark_reg_unknown(env, regs, value_regno);
 	} else if (reg->type == PTR_TO_CTX) {
 		enum bpf_reg_type reg_type = SCALAR_VALUE;
 		struct btf *btf = NULL;
@@ -9345,7 +9361,7 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
 		mark_reg_known_zero(env, regs, insn->dst_reg);
 
 		dst_reg->type = aux->btf_var.reg_type;
-		switch (dst_reg->type) {
+		switch (BPF_BASE_TYPE(dst_reg->type)) {
 		case PTR_TO_MEM:
 			dst_reg->mem_size = aux->btf_var.mem_size;
 			break;
@@ -11489,7 +11505,7 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
 			err = -EINVAL;
 			goto err_put;
 		}
-		aux->btf_var.reg_type = PTR_TO_MEM;
+		aux->btf_var.reg_type = PTR_TO_MEM | MEM_RDONLY;
 		aux->btf_var.mem_size = tsize;
 	} else {
 		aux->btf_var.reg_type = PTR_TO_BTF_ID;
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
                   ` (6 preceding siblings ...)
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 7/9] bpf: Make per_cpu_ptr return rdonly PTR_TO_MEM Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  2021-12-01 20:34   ` Alexei Starovoitov
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 9/9] bpf/selftests: Test PTR_TO_RDONLY_MEM Hao Luo
  8 siblings, 1 reply; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

Some helper functions may modify its arguments, for example,
bpf_d_path, bpf_get_stack etc. Previously, their argument types
were marked as ARG_PTR_TO_MEM, which is compatible with read-only
mem types, such as PTR_TO_RDONLY_BUF. Therefore it's legitimate
to modify a read-only memory by passing it into one of such helper
functions.

This patch tags the args that point to immutable memory with
MEM_RDONLY flag. The arguments that don't have this flag will be
only compatible with mutable memory types. The arguments that have
MEM_RDONLY flag are compatible with both mutable memory and immutable
memory.

Signed-off-by: Hao Luo <haoluo@google.com>
---
 kernel/bpf/btf.c         |  2 +-
 kernel/bpf/cgroup.c      |  2 +-
 kernel/bpf/helpers.c     |  8 ++---
 kernel/bpf/ringbuf.c     |  2 +-
 kernel/bpf/syscall.c     |  2 +-
 kernel/bpf/verifier.c    | 28 +++++++++++++++++-
 kernel/trace/bpf_trace.c | 26 ++++++++--------
 net/core/filter.c        | 64 ++++++++++++++++++++--------------------
 8 files changed, 80 insertions(+), 54 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 776b90de0971..f0da2f997937 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6351,7 +6351,7 @@ const struct bpf_func_proto bpf_btf_find_by_name_kind_proto = {
 	.func		= bpf_btf_find_by_name_kind,
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
-	.arg1_type	= ARG_PTR_TO_MEM,
+	.arg1_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg2_type	= ARG_CONST_SIZE,
 	.arg3_type	= ARG_ANYTHING,
 	.arg4_type	= ARG_ANYTHING,
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 2ca643af9a54..b8fe671af13c 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1789,7 +1789,7 @@ static const struct bpf_func_proto bpf_sysctl_set_new_value_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 };
 
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a5e349c9d3e3..66b466903a4e 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -530,7 +530,7 @@ const struct bpf_func_proto bpf_strtol_proto = {
 	.func		= bpf_strtol,
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
-	.arg1_type	= ARG_PTR_TO_MEM,
+	.arg1_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg2_type	= ARG_CONST_SIZE,
 	.arg3_type	= ARG_ANYTHING,
 	.arg4_type	= ARG_PTR_TO_LONG,
@@ -558,7 +558,7 @@ const struct bpf_func_proto bpf_strtoul_proto = {
 	.func		= bpf_strtoul,
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
-	.arg1_type	= ARG_PTR_TO_MEM,
+	.arg1_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg2_type	= ARG_CONST_SIZE,
 	.arg3_type	= ARG_ANYTHING,
 	.arg4_type	= ARG_PTR_TO_LONG,
@@ -630,7 +630,7 @@ const struct bpf_func_proto bpf_event_output_data_proto =  {
 	.arg1_type      = ARG_PTR_TO_CTX,
 	.arg2_type      = ARG_CONST_MAP_PTR,
 	.arg3_type      = ARG_ANYTHING,
-	.arg4_type      = ARG_PTR_TO_MEM,
+	.arg4_type      = ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type      = ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -1011,7 +1011,7 @@ const struct bpf_func_proto bpf_snprintf_proto = {
 	.arg1_type	= ARG_PTR_TO_MEM_OR_NULL,
 	.arg2_type	= ARG_CONST_SIZE_OR_ZERO,
 	.arg3_type	= ARG_PTR_TO_CONST_STR,
-	.arg4_type	= ARG_PTR_TO_MEM_OR_NULL,
+	.arg4_type	= ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
index 9e0c10c6892a..638d7fd7b375 100644
--- a/kernel/bpf/ringbuf.c
+++ b/kernel/bpf/ringbuf.c
@@ -444,7 +444,7 @@ const struct bpf_func_proto bpf_ringbuf_output_proto = {
 	.func		= bpf_ringbuf_output,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_CONST_MAP_PTR,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE_OR_ZERO,
 	.arg4_type	= ARG_ANYTHING,
 };
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 50f96ea4452a..301afb44e47f 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4757,7 +4757,7 @@ static const struct bpf_func_proto bpf_sys_bpf_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_ANYTHING,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e2663544362a..11ec26c1caa4 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4976,9 +4976,15 @@ static int resolve_map_arg_type(struct bpf_verifier_env *env,
 	return 0;
 }
 
+
 struct bpf_reg_types {
 	const enum bpf_reg_type types[10];
 	u32 *btf_id;
+
+	/* Certain types require customized type matching function. */
+	bool (*type_match_fn)(enum bpf_arg_type arg_type,
+			      enum bpf_reg_type type,
+			      enum bpf_reg_type expected);
 };
 
 static const struct bpf_reg_types map_key_value_types = {
@@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = {
 };
 #endif
 
+static bool mem_type_match(enum bpf_arg_type arg_type,
+			   enum bpf_reg_type type, enum bpf_reg_type expected)
+{
+	/* If arg_type is tagged with MEM_RDONLY, type is compatible with both
+	 * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before
+	 * comparison.
+	 */
+	if ((arg_type & MEM_RDONLY) != 0)
+		type &= ~MEM_RDONLY;
+
+	return type == expected;
+}
+
 static const struct bpf_reg_types mem_types = {
 	.types = {
 		PTR_TO_STACK,
@@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = {
 		PTR_TO_MAP_VALUE,
 		PTR_TO_MEM,
 		PTR_TO_BUF,
-		PTR_TO_BUF | MEM_RDONLY,
 	},
+	.type_match_fn = mem_type_match,
 };
 
 static const struct bpf_reg_types int_ptr_types = {
@@ -5097,6 +5116,13 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
 		if (expected == NOT_INIT)
 			break;
 
+		if (compatible->type_match_fn) {
+			if (compatible->type_match_fn(arg_type, type, expected))
+				goto found;
+
+			continue;
+		}
+
 		if (type == expected)
 			goto found;
 	}
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 25ea521fb8f1..79404049b70f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -345,7 +345,7 @@ static const struct bpf_func_proto bpf_probe_write_user_proto = {
 	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_ANYTHING,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 };
 
@@ -394,7 +394,7 @@ static const struct bpf_func_proto bpf_trace_printk_proto = {
 	.func		= bpf_trace_printk,
 	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
-	.arg1_type	= ARG_PTR_TO_MEM,
+	.arg1_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg2_type	= ARG_CONST_SIZE,
 };
 
@@ -450,9 +450,9 @@ static const struct bpf_func_proto bpf_trace_vprintk_proto = {
 	.func		= bpf_trace_vprintk,
 	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
-	.arg1_type	= ARG_PTR_TO_MEM,
+	.arg1_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg2_type	= ARG_CONST_SIZE,
-	.arg3_type	= ARG_PTR_TO_MEM_OR_NULL,
+	.arg3_type	= ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY,
 	.arg4_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -492,9 +492,9 @@ static const struct bpf_func_proto bpf_seq_printf_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_BTF_ID,
 	.arg1_btf_id	= &btf_seq_file_ids[0],
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
-	.arg4_type      = ARG_PTR_TO_MEM_OR_NULL,
+	.arg4_type      = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY,
 	.arg5_type      = ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -509,7 +509,7 @@ static const struct bpf_func_proto bpf_seq_write_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_BTF_ID,
 	.arg1_btf_id	= &btf_seq_file_ids[0],
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -533,7 +533,7 @@ static const struct bpf_func_proto bpf_seq_printf_btf_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_BTF_ID,
 	.arg1_btf_id	= &btf_seq_file_ids[0],
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE_OR_ZERO,
 	.arg4_type	= ARG_ANYTHING,
 };
@@ -694,7 +694,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_CONST_MAP_PTR,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -1004,7 +1004,7 @@ const struct bpf_func_proto bpf_snprintf_btf_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_MEM,
 	.arg2_type	= ARG_CONST_SIZE,
-	.arg3_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg4_type	= ARG_CONST_SIZE,
 	.arg5_type	= ARG_ANYTHING,
 };
@@ -1289,7 +1289,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto_tp = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_CONST_MAP_PTR,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -1515,7 +1515,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto_raw_tp = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_CONST_MAP_PTR,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -1569,7 +1569,7 @@ static const struct bpf_func_proto bpf_get_stack_proto_raw_tp = {
 	.gpl_only	= true,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE_OR_ZERO,
 	.arg4_type	= ARG_ANYTHING,
 };
diff --git a/net/core/filter.c b/net/core/filter.c
index 26e0276aa00d..eadca10b436d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1713,7 +1713,7 @@ static const struct bpf_func_proto bpf_skb_store_bytes_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_ANYTHING,
-	.arg3_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg4_type	= ARG_CONST_SIZE,
 	.arg5_type	= ARG_ANYTHING,
 };
@@ -2018,9 +2018,9 @@ static const struct bpf_func_proto bpf_csum_diff_proto = {
 	.gpl_only	= false,
 	.pkt_access	= true,
 	.ret_type	= RET_INTEGER,
-	.arg1_type	= ARG_PTR_TO_MEM_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY,
 	.arg2_type	= ARG_CONST_SIZE_OR_ZERO,
-	.arg3_type	= ARG_PTR_TO_MEM_OR_NULL,
+	.arg3_type	= ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY,
 	.arg4_type	= ARG_CONST_SIZE_OR_ZERO,
 	.arg5_type	= ARG_ANYTHING,
 };
@@ -2541,7 +2541,7 @@ static const struct bpf_func_proto bpf_redirect_neigh_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_ANYTHING,
-	.arg2_type      = ARG_PTR_TO_MEM_OR_NULL,
+	.arg2_type      = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY,
 	.arg3_type      = ARG_CONST_SIZE_OR_ZERO,
 	.arg4_type	= ARG_ANYTHING,
 };
@@ -4174,7 +4174,7 @@ static const struct bpf_func_proto bpf_skb_event_output_proto = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_CONST_MAP_PTR,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -4188,7 +4188,7 @@ const struct bpf_func_proto bpf_skb_output_proto = {
 	.arg1_btf_id	= &bpf_skb_output_btf_ids[0],
 	.arg2_type	= ARG_CONST_MAP_PTR,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -4371,7 +4371,7 @@ static const struct bpf_func_proto bpf_skb_set_tunnel_key_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 	.arg4_type	= ARG_ANYTHING,
 };
@@ -4397,7 +4397,7 @@ static const struct bpf_func_proto bpf_skb_set_tunnel_opt_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 };
 
@@ -4567,7 +4567,7 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_CONST_MAP_PTR,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -4581,7 +4581,7 @@ const struct bpf_func_proto bpf_xdp_output_proto = {
 	.arg1_btf_id	= &bpf_xdp_output_btf_ids[0],
 	.arg2_type	= ARG_CONST_MAP_PTR,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE_OR_ZERO,
 };
 
@@ -5067,7 +5067,7 @@ const struct bpf_func_proto bpf_sk_setsockopt_proto = {
 	.arg1_type	= ARG_PTR_TO_BTF_ID_SOCK_COMMON,
 	.arg2_type	= ARG_ANYTHING,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE,
 };
 
@@ -5101,7 +5101,7 @@ static const struct bpf_func_proto bpf_sock_addr_setsockopt_proto = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_ANYTHING,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE,
 };
 
@@ -5135,7 +5135,7 @@ static const struct bpf_func_proto bpf_sock_ops_setsockopt_proto = {
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_ANYTHING,
 	.arg3_type	= ARG_ANYTHING,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE,
 };
 
@@ -5310,7 +5310,7 @@ static const struct bpf_func_proto bpf_bind_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 };
 
@@ -5898,7 +5898,7 @@ static const struct bpf_func_proto bpf_lwt_in_push_encap_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_ANYTHING,
-	.arg3_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg4_type	= ARG_CONST_SIZE
 };
 
@@ -5908,7 +5908,7 @@ static const struct bpf_func_proto bpf_lwt_xmit_push_encap_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_ANYTHING,
-	.arg3_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg4_type	= ARG_CONST_SIZE
 };
 
@@ -5951,7 +5951,7 @@ static const struct bpf_func_proto bpf_lwt_seg6_store_bytes_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_ANYTHING,
-	.arg3_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg4_type	= ARG_CONST_SIZE
 };
 
@@ -6039,7 +6039,7 @@ static const struct bpf_func_proto bpf_lwt_seg6_action_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
 	.arg2_type	= ARG_ANYTHING,
-	.arg3_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg4_type	= ARG_CONST_SIZE
 };
 
@@ -6264,7 +6264,7 @@ static const struct bpf_func_proto bpf_skc_lookup_tcp_proto = {
 	.pkt_access	= true,
 	.ret_type	= RET_PTR_TO_SOCK_COMMON_OR_NULL,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 	.arg4_type	= ARG_ANYTHING,
 	.arg5_type	= ARG_ANYTHING,
@@ -6283,7 +6283,7 @@ static const struct bpf_func_proto bpf_sk_lookup_tcp_proto = {
 	.pkt_access	= true,
 	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 	.arg4_type	= ARG_ANYTHING,
 	.arg5_type	= ARG_ANYTHING,
@@ -6302,7 +6302,7 @@ static const struct bpf_func_proto bpf_sk_lookup_udp_proto = {
 	.pkt_access	= true,
 	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 	.arg4_type	= ARG_ANYTHING,
 	.arg5_type	= ARG_ANYTHING,
@@ -6339,7 +6339,7 @@ static const struct bpf_func_proto bpf_xdp_sk_lookup_udp_proto = {
 	.pkt_access     = true,
 	.ret_type       = RET_PTR_TO_SOCKET_OR_NULL,
 	.arg1_type      = ARG_PTR_TO_CTX,
-	.arg2_type      = ARG_PTR_TO_MEM,
+	.arg2_type      = ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type      = ARG_CONST_SIZE,
 	.arg4_type      = ARG_ANYTHING,
 	.arg5_type      = ARG_ANYTHING,
@@ -6362,7 +6362,7 @@ static const struct bpf_func_proto bpf_xdp_skc_lookup_tcp_proto = {
 	.pkt_access     = true,
 	.ret_type       = RET_PTR_TO_SOCK_COMMON_OR_NULL,
 	.arg1_type      = ARG_PTR_TO_CTX,
-	.arg2_type      = ARG_PTR_TO_MEM,
+	.arg2_type      = ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type      = ARG_CONST_SIZE,
 	.arg4_type      = ARG_ANYTHING,
 	.arg5_type      = ARG_ANYTHING,
@@ -6385,7 +6385,7 @@ static const struct bpf_func_proto bpf_xdp_sk_lookup_tcp_proto = {
 	.pkt_access     = true,
 	.ret_type       = RET_PTR_TO_SOCKET_OR_NULL,
 	.arg1_type      = ARG_PTR_TO_CTX,
-	.arg2_type      = ARG_PTR_TO_MEM,
+	.arg2_type      = ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type      = ARG_CONST_SIZE,
 	.arg4_type      = ARG_ANYTHING,
 	.arg5_type      = ARG_ANYTHING,
@@ -6404,7 +6404,7 @@ static const struct bpf_func_proto bpf_sock_addr_skc_lookup_tcp_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_PTR_TO_SOCK_COMMON_OR_NULL,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 	.arg4_type	= ARG_ANYTHING,
 	.arg5_type	= ARG_ANYTHING,
@@ -6423,7 +6423,7 @@ static const struct bpf_func_proto bpf_sock_addr_sk_lookup_tcp_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 	.arg4_type	= ARG_ANYTHING,
 	.arg5_type	= ARG_ANYTHING,
@@ -6442,7 +6442,7 @@ static const struct bpf_func_proto bpf_sock_addr_sk_lookup_udp_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 	.arg4_type	= ARG_ANYTHING,
 	.arg5_type	= ARG_ANYTHING,
@@ -6755,9 +6755,9 @@ static const struct bpf_func_proto bpf_tcp_check_syncookie_proto = {
 	.pkt_access	= true,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_BTF_ID_SOCK_COMMON,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE,
 };
 
@@ -6824,9 +6824,9 @@ static const struct bpf_func_proto bpf_tcp_gen_syncookie_proto = {
 	.pkt_access	= true,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_BTF_ID_SOCK_COMMON,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
-	.arg4_type	= ARG_PTR_TO_MEM,
+	.arg4_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg5_type	= ARG_CONST_SIZE,
 };
 
@@ -7055,7 +7055,7 @@ static const struct bpf_func_proto bpf_sock_ops_store_hdr_opt_proto = {
 	.gpl_only	= false,
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_CTX,
-	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg2_type	= ARG_PTR_TO_MEM | MEM_RDONLY,
 	.arg3_type	= ARG_CONST_SIZE,
 	.arg4_type	= ARG_ANYTHING,
 };
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RFC PATCH bpf-next v2 9/9] bpf/selftests: Test PTR_TO_RDONLY_MEM
  2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
                   ` (7 preceding siblings ...)
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem Hao Luo
@ 2021-11-30  1:29 ` Hao Luo
  8 siblings, 0 replies; 28+ messages in thread
From: Hao Luo @ 2021-11-30  1:29 UTC (permalink / raw)
  To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf, Hao Luo

This test verifies that a ksym of non-struct can not be directly
updated.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Hao Luo <haoluo@google.com>
---
 .../selftests/bpf/prog_tests/ksyms_btf.c      | 14 +++++++++
 .../bpf/progs/test_ksyms_btf_write_check.c    | 29 +++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_btf_write_check.c

diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c b/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c
index 79f6bd1e50d6..f6933b06daf8 100644
--- a/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c
@@ -8,6 +8,7 @@
 #include "test_ksyms_btf_null_check.skel.h"
 #include "test_ksyms_weak.skel.h"
 #include "test_ksyms_weak.lskel.h"
+#include "test_ksyms_btf_write_check.skel.h"
 
 static int duration;
 
@@ -137,6 +138,16 @@ static void test_weak_syms_lskel(void)
 	test_ksyms_weak_lskel__destroy(skel);
 }
 
+static void test_write_check(void)
+{
+	struct test_ksyms_btf_write_check *skel;
+
+	skel = test_ksyms_btf_write_check__open_and_load();
+	ASSERT_ERR_PTR(skel, "unexpected load of a prog writing to ksym memory\n");
+
+	test_ksyms_btf_write_check__destroy(skel);
+}
+
 void test_ksyms_btf(void)
 {
 	int percpu_datasec;
@@ -167,4 +178,7 @@ void test_ksyms_btf(void)
 
 	if (test__start_subtest("weak_ksyms_lskel"))
 		test_weak_syms_lskel();
+
+	if (test__start_subtest("write_check"))
+		test_write_check();
 }
diff --git a/tools/testing/selftests/bpf/progs/test_ksyms_btf_write_check.c b/tools/testing/selftests/bpf/progs/test_ksyms_btf_write_check.c
new file mode 100644
index 000000000000..2180c41cd890
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_ksyms_btf_write_check.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Google */
+
+#include "vmlinux.h"
+
+#include <bpf/bpf_helpers.h>
+
+extern const int bpf_prog_active __ksym; /* int type global var. */
+
+SEC("raw_tp/sys_enter")
+int handler(const void *ctx)
+{
+	int *active;
+	__u32 cpu;
+
+	cpu = bpf_get_smp_processor_id();
+	active = (int *)bpf_per_cpu_ptr(&bpf_prog_active, cpu);
+	if (active) {
+		/* Kernel memory obtained from bpf_{per,this}_cpu_ptr
+		 * is read-only, should _not_ pass verification.
+		 */
+		/* WRITE_ONCE */
+		*(volatile int *)active = -1;
+	}
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.34.0.384.gca35af8252-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX " Hao Luo
@ 2021-11-30  2:59   ` kernel test robot
  2021-11-30  3:40     ` kernel test robot
  2021-12-01 20:30   ` Alexei Starovoitov
  2 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-11-30  2:59 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 24756 bytes --]

Hi Hao,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Hao-Luo/Introduce-composable-bpf-types/20211130-093143
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20211130/202111301041.Xvx6affN-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/5af019e76ba5485e0b56b5b4607c9d2e30ca6138
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hao-Luo/Introduce-composable-bpf-types/20211130-093143
        git checkout 5af019e76ba5485e0b56b5b4607c9d2e30ca6138
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash kernel/bpf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   kernel/bpf/verifier.c: In function 'check_helper_call':
>> kernel/bpf/verifier.c:6597:39: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=]
    6597 |    verbose(env, "invalid return type %d of func %s#%d\n",
         |                                      ~^
         |                                       |
         |                                       int
         |                                      %ld
   kernel/bpf/verifier.c:6608:38: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=]
    6608 |   verbose(env, "unknown return type %d of func %s#%d\n",
         |                                     ~^
         |                                      |
         |                                      int
         |                                     %ld


vim +6597 kernel/bpf/verifier.c

9b99edcae5c80c Jiri Olsa           2021-07-14  6373  
69c087ba6225b5 Yonghong Song       2021-02-26  6374  static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
69c087ba6225b5 Yonghong Song       2021-02-26  6375  			     int *insn_idx_p)
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6376  {
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6377  	const struct bpf_func_proto *fn = NULL;
5af019e76ba548 Hao Luo             2021-11-29  6378  	enum bpf_return_type ret_type;
638f5b90d46016 Alexei Starovoitov  2017-10-31  6379  	struct bpf_reg_state *regs;
33ff9823c569f3 Daniel Borkmann     2016-04-13  6380  	struct bpf_call_arg_meta meta;
69c087ba6225b5 Yonghong Song       2021-02-26  6381  	int insn_idx = *insn_idx_p;
969bf05eb3cedd Alexei Starovoitov  2016-05-05  6382  	bool changes_data;
69c087ba6225b5 Yonghong Song       2021-02-26  6383  	int i, err, func_id;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6384  
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6385  	/* find function prototype */
69c087ba6225b5 Yonghong Song       2021-02-26  6386  	func_id = insn->imm;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6387  	if (func_id < 0 || func_id >= __BPF_FUNC_MAX_ID) {
61bd5218eef349 Jakub Kicinski      2017-10-09  6388  		verbose(env, "invalid func %s#%d\n", func_id_name(func_id),
61bd5218eef349 Jakub Kicinski      2017-10-09  6389  			func_id);
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6390  		return -EINVAL;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6391  	}
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6392  
00176a34d9e27a Jakub Kicinski      2017-10-16  6393  	if (env->ops->get_func_proto)
5e43f899b03a34 Andrey Ignatov      2018-03-30  6394  		fn = env->ops->get_func_proto(func_id, env->prog);
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6395  	if (!fn) {
61bd5218eef349 Jakub Kicinski      2017-10-09  6396  		verbose(env, "unknown func %s#%d\n", func_id_name(func_id),
61bd5218eef349 Jakub Kicinski      2017-10-09  6397  			func_id);
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6398  		return -EINVAL;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6399  	}
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6400  
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6401  	/* eBPF programs must be GPL compatible to use GPL-ed functions */
24701ecea76b0b Daniel Borkmann     2015-03-01  6402  	if (!env->prog->gpl_compatible && fn->gpl_only) {
3fe2867cdf088f Daniel Borkmann     2018-06-02  6403  		verbose(env, "cannot call GPL-restricted function from non-GPL compatible program\n");
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6404  		return -EINVAL;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6405  	}
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6406  
eae2e83e62633a Jiri Olsa           2020-08-25  6407  	if (fn->allowed && !fn->allowed(env->prog)) {
eae2e83e62633a Jiri Olsa           2020-08-25  6408  		verbose(env, "helper call is not allowed in probe\n");
eae2e83e62633a Jiri Olsa           2020-08-25  6409  		return -EINVAL;
eae2e83e62633a Jiri Olsa           2020-08-25  6410  	}
eae2e83e62633a Jiri Olsa           2020-08-25  6411  
04514d13222f2c Daniel Borkmann     2017-12-14  6412  	/* With LD_ABS/IND some JITs save/restore skb from r1. */
17bedab2723145 Martin KaFai Lau    2016-12-07  6413  	changes_data = bpf_helper_changes_pkt_data(fn->func);
04514d13222f2c Daniel Borkmann     2017-12-14  6414  	if (changes_data && fn->arg1_type != ARG_PTR_TO_CTX) {
04514d13222f2c Daniel Borkmann     2017-12-14  6415  		verbose(env, "kernel subsystem misconfigured func %s#%d: r1 != ctx\n",
04514d13222f2c Daniel Borkmann     2017-12-14  6416  			func_id_name(func_id), func_id);
04514d13222f2c Daniel Borkmann     2017-12-14  6417  		return -EINVAL;
04514d13222f2c Daniel Borkmann     2017-12-14  6418  	}
969bf05eb3cedd Alexei Starovoitov  2016-05-05  6419  
33ff9823c569f3 Daniel Borkmann     2016-04-13  6420  	memset(&meta, 0, sizeof(meta));
36bbef52c7eb64 Daniel Borkmann     2016-09-20  6421  	meta.pkt_access = fn->pkt_access;
33ff9823c569f3 Daniel Borkmann     2016-04-13  6422  
1b986589680a2a Martin KaFai Lau    2019-03-12  6423  	err = check_func_proto(fn, func_id);
435faee1aae9c1 Daniel Borkmann     2016-04-13  6424  	if (err) {
61bd5218eef349 Jakub Kicinski      2017-10-09  6425  		verbose(env, "kernel subsystem misconfigured func %s#%d\n",
ebb676daa1a340 Thomas Graf         2016-10-27  6426  			func_id_name(func_id), func_id);
435faee1aae9c1 Daniel Borkmann     2016-04-13  6427  		return err;
435faee1aae9c1 Daniel Borkmann     2016-04-13  6428  	}
435faee1aae9c1 Daniel Borkmann     2016-04-13  6429  
d83525ca62cf8e Alexei Starovoitov  2019-01-31  6430  	meta.func_id = func_id;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6431  	/* check args */
523a4cf491b3c9 Dmitrii Banshchikov 2021-02-26  6432  	for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) {
af7ec13833619e Yonghong Song       2020-06-23  6433  		err = check_func_arg(env, i, &meta, fn);
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6434  		if (err)
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6435  			return err;
a7658e1a4164ce Alexei Starovoitov  2019-10-15  6436  	}
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6437  
c93552c443ebc6 Daniel Borkmann     2018-05-24  6438  	err = record_func_map(env, &meta, func_id, insn_idx);
c93552c443ebc6 Daniel Borkmann     2018-05-24  6439  	if (err)
c93552c443ebc6 Daniel Borkmann     2018-05-24  6440  		return err;
c93552c443ebc6 Daniel Borkmann     2018-05-24  6441  
d2e4c1e6c29472 Daniel Borkmann     2019-11-22  6442  	err = record_func_key(env, &meta, func_id, insn_idx);
d2e4c1e6c29472 Daniel Borkmann     2019-11-22  6443  	if (err)
d2e4c1e6c29472 Daniel Borkmann     2019-11-22  6444  		return err;
d2e4c1e6c29472 Daniel Borkmann     2019-11-22  6445  
435faee1aae9c1 Daniel Borkmann     2016-04-13  6446  	/* Mark slots with STACK_MISC in case of raw mode, stack offset
435faee1aae9c1 Daniel Borkmann     2016-04-13  6447  	 * is inferred from register state.
435faee1aae9c1 Daniel Borkmann     2016-04-13  6448  	 */
435faee1aae9c1 Daniel Borkmann     2016-04-13  6449  	for (i = 0; i < meta.access_size; i++) {
ca36960211eb22 Daniel Borkmann     2018-02-23  6450  		err = check_mem_access(env, insn_idx, meta.regno, i, BPF_B,
ca36960211eb22 Daniel Borkmann     2018-02-23  6451  				       BPF_WRITE, -1, false);
435faee1aae9c1 Daniel Borkmann     2016-04-13  6452  		if (err)
435faee1aae9c1 Daniel Borkmann     2016-04-13  6453  			return err;
435faee1aae9c1 Daniel Borkmann     2016-04-13  6454  	}
435faee1aae9c1 Daniel Borkmann     2016-04-13  6455  
fd978bf7fd3125 Joe Stringer        2018-10-02  6456  	if (func_id == BPF_FUNC_tail_call) {
fd978bf7fd3125 Joe Stringer        2018-10-02  6457  		err = check_reference_leak(env);
fd978bf7fd3125 Joe Stringer        2018-10-02  6458  		if (err) {
fd978bf7fd3125 Joe Stringer        2018-10-02  6459  			verbose(env, "tail_call would lead to reference leak\n");
fd978bf7fd3125 Joe Stringer        2018-10-02  6460  			return err;
fd978bf7fd3125 Joe Stringer        2018-10-02  6461  		}
fd978bf7fd3125 Joe Stringer        2018-10-02  6462  	} else if (is_release_function(func_id)) {
1b986589680a2a Martin KaFai Lau    2019-03-12  6463  		err = release_reference(env, meta.ref_obj_id);
46f8bc92758c62 Martin KaFai Lau    2019-02-09  6464  		if (err) {
46f8bc92758c62 Martin KaFai Lau    2019-02-09  6465  			verbose(env, "func %s#%d reference has not been acquired before\n",
46f8bc92758c62 Martin KaFai Lau    2019-02-09  6466  				func_id_name(func_id), func_id);
fd978bf7fd3125 Joe Stringer        2018-10-02  6467  			return err;
fd978bf7fd3125 Joe Stringer        2018-10-02  6468  		}
46f8bc92758c62 Martin KaFai Lau    2019-02-09  6469  	}
fd978bf7fd3125 Joe Stringer        2018-10-02  6470  
638f5b90d46016 Alexei Starovoitov  2017-10-31  6471  	regs = cur_regs(env);
cd339431765383 Roman Gushchin      2018-08-02  6472  
cd339431765383 Roman Gushchin      2018-08-02  6473  	/* check that flags argument in get_local_storage(map, flags) is 0,
cd339431765383 Roman Gushchin      2018-08-02  6474  	 * this is required because get_local_storage() can't return an error.
cd339431765383 Roman Gushchin      2018-08-02  6475  	 */
cd339431765383 Roman Gushchin      2018-08-02  6476  	if (func_id == BPF_FUNC_get_local_storage &&
cd339431765383 Roman Gushchin      2018-08-02  6477  	    !register_is_null(&regs[BPF_REG_2])) {
cd339431765383 Roman Gushchin      2018-08-02  6478  		verbose(env, "get_local_storage() doesn't support non-zero flags\n");
cd339431765383 Roman Gushchin      2018-08-02  6479  		return -EINVAL;
cd339431765383 Roman Gushchin      2018-08-02  6480  	}
cd339431765383 Roman Gushchin      2018-08-02  6481  
69c087ba6225b5 Yonghong Song       2021-02-26  6482  	if (func_id == BPF_FUNC_for_each_map_elem) {
69c087ba6225b5 Yonghong Song       2021-02-26  6483  		err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
69c087ba6225b5 Yonghong Song       2021-02-26  6484  					set_map_elem_callback_state);
69c087ba6225b5 Yonghong Song       2021-02-26  6485  		if (err < 0)
69c087ba6225b5 Yonghong Song       2021-02-26  6486  			return -EINVAL;
69c087ba6225b5 Yonghong Song       2021-02-26  6487  	}
69c087ba6225b5 Yonghong Song       2021-02-26  6488  
b00628b1c7d595 Alexei Starovoitov  2021-07-14  6489  	if (func_id == BPF_FUNC_timer_set_callback) {
b00628b1c7d595 Alexei Starovoitov  2021-07-14  6490  		err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
b00628b1c7d595 Alexei Starovoitov  2021-07-14  6491  					set_timer_callback_state);
b00628b1c7d595 Alexei Starovoitov  2021-07-14  6492  		if (err < 0)
b00628b1c7d595 Alexei Starovoitov  2021-07-14  6493  			return -EINVAL;
b00628b1c7d595 Alexei Starovoitov  2021-07-14  6494  	}
b00628b1c7d595 Alexei Starovoitov  2021-07-14  6495  
7c7e3d31e7856a Song Liu            2021-11-05  6496  	if (func_id == BPF_FUNC_find_vma) {
7c7e3d31e7856a Song Liu            2021-11-05  6497  		err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
7c7e3d31e7856a Song Liu            2021-11-05  6498  					set_find_vma_callback_state);
7c7e3d31e7856a Song Liu            2021-11-05  6499  		if (err < 0)
7c7e3d31e7856a Song Liu            2021-11-05  6500  			return -EINVAL;
7c7e3d31e7856a Song Liu            2021-11-05  6501  	}
7c7e3d31e7856a Song Liu            2021-11-05  6502  
7b15523a989b63 Florent Revest      2021-04-19  6503  	if (func_id == BPF_FUNC_snprintf) {
7b15523a989b63 Florent Revest      2021-04-19  6504  		err = check_bpf_snprintf_call(env, regs);
7b15523a989b63 Florent Revest      2021-04-19  6505  		if (err < 0)
7b15523a989b63 Florent Revest      2021-04-19  6506  			return err;
7b15523a989b63 Florent Revest      2021-04-19  6507  	}
7b15523a989b63 Florent Revest      2021-04-19  6508  
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6509  	/* reset caller saved regs */
dc503a8ad98474 Edward Cree         2017-08-15  6510  	for (i = 0; i < CALLER_SAVED_REGS; i++) {
61bd5218eef349 Jakub Kicinski      2017-10-09  6511  		mark_reg_not_init(env, regs, caller_saved[i]);
dc503a8ad98474 Edward Cree         2017-08-15  6512  		check_reg_arg(env, caller_saved[i], DST_OP_NO_MARK);
dc503a8ad98474 Edward Cree         2017-08-15  6513  	}
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6514  
5327ed3d44b754 Jiong Wang          2019-05-24  6515  	/* helper call returns 64-bit value. */
5327ed3d44b754 Jiong Wang          2019-05-24  6516  	regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
5327ed3d44b754 Jiong Wang          2019-05-24  6517  
dc503a8ad98474 Edward Cree         2017-08-15  6518  	/* update return register (already marked as written above) */
5af019e76ba548 Hao Luo             2021-11-29  6519  	ret_type = fn->ret_type;
5af019e76ba548 Hao Luo             2021-11-29  6520  	if (ret_type == RET_INTEGER) {
f1174f77b50c94 Edward Cree         2017-08-07  6521  		/* sets type to SCALAR_VALUE */
61bd5218eef349 Jakub Kicinski      2017-10-09  6522  		mark_reg_unknown(env, regs, BPF_REG_0);
5af019e76ba548 Hao Luo             2021-11-29  6523  	} else if (ret_type == RET_VOID) {
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6524  		regs[BPF_REG_0].type = NOT_INIT;
5af019e76ba548 Hao Luo             2021-11-29  6525  	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MAP_VALUE) {
f1174f77b50c94 Edward Cree         2017-08-07  6526  		/* There is no offset yet applied, variable or fixed */
61bd5218eef349 Jakub Kicinski      2017-10-09  6527  		mark_reg_known_zero(env, regs, BPF_REG_0);
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6528  		/* remember map_ptr, so that check_map_access()
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6529  		 * can check 'value_size' boundary of memory access
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6530  		 * to map element returned from bpf_map_lookup_elem()
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6531  		 */
33ff9823c569f3 Daniel Borkmann     2016-04-13  6532  		if (meta.map_ptr == NULL) {
61bd5218eef349 Jakub Kicinski      2017-10-09  6533  			verbose(env,
61bd5218eef349 Jakub Kicinski      2017-10-09  6534  				"kernel subsystem misconfigured verifier\n");
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6535  			return -EINVAL;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6536  		}
33ff9823c569f3 Daniel Borkmann     2016-04-13  6537  		regs[BPF_REG_0].map_ptr = meta.map_ptr;
3e8ce29850f183 Alexei Starovoitov  2021-07-14  6538  		regs[BPF_REG_0].map_uid = meta.map_uid;
5af019e76ba548 Hao Luo             2021-11-29  6539  		if (ret_type_may_be_null(fn->ret_type)) {
5af019e76ba548 Hao Luo             2021-11-29  6540  			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
5af019e76ba548 Hao Luo             2021-11-29  6541  		} else {
4d31f30148cea6 Daniel Borkmann     2018-11-01  6542  			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;
e16d2f1ab96849 Alexei Starovoitov  2019-01-31  6543  			if (map_value_has_spin_lock(meta.map_ptr))
e16d2f1ab96849 Alexei Starovoitov  2019-01-31  6544  				regs[BPF_REG_0].id = ++env->id_gen;
4d31f30148cea6 Daniel Borkmann     2018-11-01  6545  		}
5af019e76ba548 Hao Luo             2021-11-29  6546  	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCKET) {
46f8bc92758c62 Martin KaFai Lau    2019-02-09  6547  		mark_reg_known_zero(env, regs, BPF_REG_0);
46f8bc92758c62 Martin KaFai Lau    2019-02-09  6548  		regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
5af019e76ba548 Hao Luo             2021-11-29  6549  	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCK_COMMON) {
85a51f8c28b981 Lorenz Bauer        2019-03-22  6550  		mark_reg_known_zero(env, regs, BPF_REG_0);
85a51f8c28b981 Lorenz Bauer        2019-03-22  6551  		regs[BPF_REG_0].type = PTR_TO_SOCK_COMMON_OR_NULL;
5af019e76ba548 Hao Luo             2021-11-29  6552  	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_TCP_SOCK) {
655a51e536c09d Martin KaFai Lau    2019-02-09  6553  		mark_reg_known_zero(env, regs, BPF_REG_0);
655a51e536c09d Martin KaFai Lau    2019-02-09  6554  		regs[BPF_REG_0].type = PTR_TO_TCP_SOCK_OR_NULL;
5af019e76ba548 Hao Luo             2021-11-29  6555  	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_ALLOC_MEM) {
457f44363a8894 Andrii Nakryiko     2020-05-29  6556  		mark_reg_known_zero(env, regs, BPF_REG_0);
457f44363a8894 Andrii Nakryiko     2020-05-29  6557  		regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
457f44363a8894 Andrii Nakryiko     2020-05-29  6558  		regs[BPF_REG_0].mem_size = meta.mem_size;
5af019e76ba548 Hao Luo             2021-11-29  6559  	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MEM_OR_BTF_ID) {
eaa6bcb71ef6ed Hao Luo             2020-09-29  6560  		const struct btf_type *t;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6561  
eaa6bcb71ef6ed Hao Luo             2020-09-29  6562  		mark_reg_known_zero(env, regs, BPF_REG_0);
22dc4a0f5ed11b Andrii Nakryiko     2020-12-03  6563  		t = btf_type_skip_modifiers(meta.ret_btf, meta.ret_btf_id, NULL);
eaa6bcb71ef6ed Hao Luo             2020-09-29  6564  		if (!btf_type_is_struct(t)) {
eaa6bcb71ef6ed Hao Luo             2020-09-29  6565  			u32 tsize;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6566  			const struct btf_type *ret;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6567  			const char *tname;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6568  
eaa6bcb71ef6ed Hao Luo             2020-09-29  6569  			/* resolve the type size of ksym. */
22dc4a0f5ed11b Andrii Nakryiko     2020-12-03  6570  			ret = btf_resolve_size(meta.ret_btf, t, &tsize);
eaa6bcb71ef6ed Hao Luo             2020-09-29  6571  			if (IS_ERR(ret)) {
22dc4a0f5ed11b Andrii Nakryiko     2020-12-03  6572  				tname = btf_name_by_offset(meta.ret_btf, t->name_off);
eaa6bcb71ef6ed Hao Luo             2020-09-29  6573  				verbose(env, "unable to resolve the size of type '%s': %ld\n",
eaa6bcb71ef6ed Hao Luo             2020-09-29  6574  					tname, PTR_ERR(ret));
eaa6bcb71ef6ed Hao Luo             2020-09-29  6575  				return -EINVAL;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6576  			}
63d9b80dcf2c67 Hao Luo             2020-09-29  6577  			regs[BPF_REG_0].type =
5af019e76ba548 Hao Luo             2021-11-29  6578  				(ret_type & PTR_MAYBE_NULL) ?
5af019e76ba548 Hao Luo             2021-11-29  6579  				PTR_TO_MEM_OR_NULL : PTR_TO_MEM;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6580  			regs[BPF_REG_0].mem_size = tsize;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6581  		} else {
63d9b80dcf2c67 Hao Luo             2020-09-29  6582  			regs[BPF_REG_0].type =
5af019e76ba548 Hao Luo             2021-11-29  6583  				(ret_type & PTR_MAYBE_NULL) ?
5af019e76ba548 Hao Luo             2021-11-29  6584  				PTR_TO_BTF_ID_OR_NULL : PTR_TO_BTF_ID;
22dc4a0f5ed11b Andrii Nakryiko     2020-12-03  6585  			regs[BPF_REG_0].btf = meta.ret_btf;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6586  			regs[BPF_REG_0].btf_id = meta.ret_btf_id;
eaa6bcb71ef6ed Hao Luo             2020-09-29  6587  		}
5af019e76ba548 Hao Luo             2021-11-29  6588  	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_BTF_ID) {
af7ec13833619e Yonghong Song       2020-06-23  6589  		int ret_btf_id;
af7ec13833619e Yonghong Song       2020-06-23  6590  
af7ec13833619e Yonghong Song       2020-06-23  6591  		mark_reg_known_zero(env, regs, BPF_REG_0);
5af019e76ba548 Hao Luo             2021-11-29  6592  		regs[BPF_REG_0].type = (ret_type & PTR_MAYBE_NULL) ?
5af019e76ba548 Hao Luo             2021-11-29  6593  						     PTR_TO_BTF_ID_OR_NULL :
5af019e76ba548 Hao Luo             2021-11-29  6594  						     PTR_TO_BTF_ID;
af7ec13833619e Yonghong Song       2020-06-23  6595  		ret_btf_id = *fn->ret_btf_id;
af7ec13833619e Yonghong Song       2020-06-23  6596  		if (ret_btf_id == 0) {
af7ec13833619e Yonghong Song       2020-06-23 @6597  			verbose(env, "invalid return type %d of func %s#%d\n",
5af019e76ba548 Hao Luo             2021-11-29  6598  				BPF_BASE_TYPE(ret_type), func_id_name(func_id),
5af019e76ba548 Hao Luo             2021-11-29  6599  				func_id);
af7ec13833619e Yonghong Song       2020-06-23  6600  			return -EINVAL;
af7ec13833619e Yonghong Song       2020-06-23  6601  		}
22dc4a0f5ed11b Andrii Nakryiko     2020-12-03  6602  		/* current BPF helper definitions are only coming from
22dc4a0f5ed11b Andrii Nakryiko     2020-12-03  6603  		 * built-in code with type IDs from  vmlinux BTF
22dc4a0f5ed11b Andrii Nakryiko     2020-12-03  6604  		 */
22dc4a0f5ed11b Andrii Nakryiko     2020-12-03  6605  		regs[BPF_REG_0].btf = btf_vmlinux;
af7ec13833619e Yonghong Song       2020-06-23  6606  		regs[BPF_REG_0].btf_id = ret_btf_id;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6607  	} else {
61bd5218eef349 Jakub Kicinski      2017-10-09  6608  		verbose(env, "unknown return type %d of func %s#%d\n",
5af019e76ba548 Hao Luo             2021-11-29  6609  			BPF_BASE_TYPE(ret_type), func_id_name(func_id), func_id);
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6610  		return -EINVAL;
17a5267067f3c3 Alexei Starovoitov  2014-09-26  6611  	}
04fd61ab36ec06 Alexei Starovoitov  2015-05-19  6612  
93c230e3f5bd6e Martin KaFai Lau    2020-10-19  6613  	if (reg_type_may_be_null(regs[BPF_REG_0].type))
93c230e3f5bd6e Martin KaFai Lau    2020-10-19  6614  		regs[BPF_REG_0].id = ++env->id_gen;
93c230e3f5bd6e Martin KaFai Lau    2020-10-19  6615  
0f3adc288df8ba Lorenz Bauer        2019-03-22  6616  	if (is_ptr_cast_function(func_id)) {
1b986589680a2a Martin KaFai Lau    2019-03-12  6617  		/* For release_reference() */
1b986589680a2a Martin KaFai Lau    2019-03-12  6618  		regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
64d85290d79c06 Jakub Sitnicki      2020-04-29  6619  	} else if (is_acquire_function(func_id, meta.map_ptr)) {
0f3adc288df8ba Lorenz Bauer        2019-03-22  6620  		int id = acquire_reference_state(env, insn_idx);
0f3adc288df8ba Lorenz Bauer        2019-03-22  6621  
0f3adc288df8ba Lorenz Bauer        2019-03-22  6622  		if (id < 0)
0f3adc288df8ba Lorenz Bauer        2019-03-22  6623  			return id;
0f3adc288df8ba Lorenz Bauer        2019-03-22  6624  		/* For mark_ptr_or_null_reg() */
0f3adc288df8ba Lorenz Bauer        2019-03-22  6625  		regs[BPF_REG_0].id = id;
0f3adc288df8ba Lorenz Bauer        2019-03-22  6626  		/* For release_reference() */
0f3adc288df8ba Lorenz Bauer        2019-03-22  6627  		regs[BPF_REG_0].ref_obj_id = id;
0f3adc288df8ba Lorenz Bauer        2019-03-22  6628  	}
1b986589680a2a Martin KaFai Lau    2019-03-12  6629  
849fa50662fbc8 Yonghong Song       2018-04-28  6630  	do_refine_retval_range(regs, fn->ret_type, func_id, &meta);
849fa50662fbc8 Yonghong Song       2018-04-28  6631  
61bd5218eef349 Jakub Kicinski      2017-10-09  6632  	err = check_map_func_compatibility(env, meta.map_ptr, func_id);
35578d79840030 Kaixu Xia           2015-08-06  6633  	if (err)
35578d79840030 Kaixu Xia           2015-08-06  6634  		return err;
04fd61ab36ec06 Alexei Starovoitov  2015-05-19  6635  
fa28dcb82a38f8 Song Liu            2020-06-29  6636  	if ((func_id == BPF_FUNC_get_stack ||
fa28dcb82a38f8 Song Liu            2020-06-29  6637  	     func_id == BPF_FUNC_get_task_stack) &&
fa28dcb82a38f8 Song Liu            2020-06-29  6638  	    !env->prog->has_callchain_buf) {
c195651e565ae7 Yonghong Song       2018-04-28  6639  		const char *err_str;
c195651e565ae7 Yonghong Song       2018-04-28  6640  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX " Hao Luo
@ 2021-11-30  3:30   ` kernel test robot
  2021-11-30  4:21     ` kernel test robot
  2021-11-30  4:31   ` kernel test robot
  2 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-11-30  3:30 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3554 bytes --]

Hi Hao,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Hao-Luo/Introduce-composable-bpf-types/20211130-093143
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20211130/202111301155.7gf5aKBg-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/9e92c0a723fc173ac102b3bb27df479a01f32896
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hao-Luo/Introduce-composable-bpf-types/20211130-093143
        git checkout 9e92c0a723fc173ac102b3bb27df479a01f32896
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash kernel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> kernel/bpf/verifier.c:533:8: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
     533 | static const char * const reg_type_str(enum bpf_reg_type type)
         |        ^~~~~
   kernel/bpf/verifier.c: In function 'check_helper_call':
   kernel/bpf/verifier.c:6544:39: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=]
    6544 |    verbose(env, "invalid return type %d of func %s#%d\n",
         |                                      ~^
         |                                       |
         |                                       int
         |                                      %ld
   kernel/bpf/verifier.c:6555:38: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=]
    6555 |   verbose(env, "unknown return type %d of func %s#%d\n",
         |                                     ~^
         |                                      |
         |                                      int
         |                                     %ld


vim +533 kernel/bpf/verifier.c

   531	
   532	/* string representation of 'enum bpf_reg_type' */
 > 533	static const char * const reg_type_str(enum bpf_reg_type type)
   534	{
   535		static const char * const str[] = {
   536			[NOT_INIT]		= "?",
   537			[SCALAR_VALUE]		= "inv",
   538			[PTR_TO_CTX]		= "ctx",
   539			[CONST_PTR_TO_MAP]	= "map_ptr",
   540			[PTR_TO_MAP_VALUE]	= "map_value",
   541			[PTR_TO_STACK]		= "fp",
   542			[PTR_TO_PACKET]		= "pkt",
   543			[PTR_TO_PACKET_META]	= "pkt_meta",
   544			[PTR_TO_PACKET_END]	= "pkt_end",
   545			[PTR_TO_FLOW_KEYS]	= "flow_keys",
   546			[PTR_TO_SOCKET]		= "sock",
   547			[PTR_TO_SOCK_COMMON]	= "sock_common",
   548			[PTR_TO_TCP_SOCK]	= "tcp_sock",
   549			[PTR_TO_TP_BUFFER]	= "tp_buffer",
   550			[PTR_TO_XDP_SOCK]	= "xdp_sock",
   551			[PTR_TO_BTF_ID]		= "ptr_",
   552			[PTR_TO_PERCPU_BTF_ID]	= "percpu_ptr_",
   553			[PTR_TO_MEM]		= "mem",
   554			[PTR_TO_RDONLY_BUF]	= "rdonly_buf",
   555			[PTR_TO_RDWR_BUF]	= "rdwr_buf",
   556			[PTR_TO_FUNC]		= "func",
   557			[PTR_TO_MAP_KEY]	= "map_key",
   558		};
   559	
   560		return str[BPF_BASE_TYPE(type)];
   561	}
   562	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX " Hao Luo
@ 2021-11-30  3:40     ` kernel test robot
  2021-11-30  3:40     ` kernel test robot
  2021-12-01 20:30   ` Alexei Starovoitov
  2 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-11-30  3:40 UTC (permalink / raw)
  To: Hao Luo; +Cc: llvm, kbuild-all

Hi Hao,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Hao-Luo/Introduce-composable-bpf-types/20211130-093143
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arm-buildonly-randconfig-r005-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301101.rEYY4B1t-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/5af019e76ba5485e0b56b5b4607c9d2e30ca6138
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hao-Luo/Introduce-composable-bpf-types/20211130-093143
        git checkout 5af019e76ba5485e0b56b5b4607c9d2e30ca6138
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash kernel/bpf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> kernel/bpf/verifier.c:6598:5: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
                                   BPF_BASE_TYPE(ret_type), func_id_name(func_id),
                                   ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:326:26: note: expanded from macro 'BPF_BASE_TYPE'
   #define BPF_BASE_TYPE(x)        ((x) & BPF_BASE_TYPE_MASK)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:6609:4: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
                           BPF_BASE_TYPE(ret_type), func_id_name(func_id), func_id);
                           ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:326:26: note: expanded from macro 'BPF_BASE_TYPE'
   #define BPF_BASE_TYPE(x)        ((x) & BPF_BASE_TYPE_MASK)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   2 warnings generated.


vim +6598 kernel/bpf/verifier.c

  6373	
  6374	static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
  6375				     int *insn_idx_p)
  6376	{
  6377		const struct bpf_func_proto *fn = NULL;
  6378		enum bpf_return_type ret_type;
  6379		struct bpf_reg_state *regs;
  6380		struct bpf_call_arg_meta meta;
  6381		int insn_idx = *insn_idx_p;
  6382		bool changes_data;
  6383		int i, err, func_id;
  6384	
  6385		/* find function prototype */
  6386		func_id = insn->imm;
  6387		if (func_id < 0 || func_id >= __BPF_FUNC_MAX_ID) {
  6388			verbose(env, "invalid func %s#%d\n", func_id_name(func_id),
  6389				func_id);
  6390			return -EINVAL;
  6391		}
  6392	
  6393		if (env->ops->get_func_proto)
  6394			fn = env->ops->get_func_proto(func_id, env->prog);
  6395		if (!fn) {
  6396			verbose(env, "unknown func %s#%d\n", func_id_name(func_id),
  6397				func_id);
  6398			return -EINVAL;
  6399		}
  6400	
  6401		/* eBPF programs must be GPL compatible to use GPL-ed functions */
  6402		if (!env->prog->gpl_compatible && fn->gpl_only) {
  6403			verbose(env, "cannot call GPL-restricted function from non-GPL compatible program\n");
  6404			return -EINVAL;
  6405		}
  6406	
  6407		if (fn->allowed && !fn->allowed(env->prog)) {
  6408			verbose(env, "helper call is not allowed in probe\n");
  6409			return -EINVAL;
  6410		}
  6411	
  6412		/* With LD_ABS/IND some JITs save/restore skb from r1. */
  6413		changes_data = bpf_helper_changes_pkt_data(fn->func);
  6414		if (changes_data && fn->arg1_type != ARG_PTR_TO_CTX) {
  6415			verbose(env, "kernel subsystem misconfigured func %s#%d: r1 != ctx\n",
  6416				func_id_name(func_id), func_id);
  6417			return -EINVAL;
  6418		}
  6419	
  6420		memset(&meta, 0, sizeof(meta));
  6421		meta.pkt_access = fn->pkt_access;
  6422	
  6423		err = check_func_proto(fn, func_id);
  6424		if (err) {
  6425			verbose(env, "kernel subsystem misconfigured func %s#%d\n",
  6426				func_id_name(func_id), func_id);
  6427			return err;
  6428		}
  6429	
  6430		meta.func_id = func_id;
  6431		/* check args */
  6432		for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) {
  6433			err = check_func_arg(env, i, &meta, fn);
  6434			if (err)
  6435				return err;
  6436		}
  6437	
  6438		err = record_func_map(env, &meta, func_id, insn_idx);
  6439		if (err)
  6440			return err;
  6441	
  6442		err = record_func_key(env, &meta, func_id, insn_idx);
  6443		if (err)
  6444			return err;
  6445	
  6446		/* Mark slots with STACK_MISC in case of raw mode, stack offset
  6447		 * is inferred from register state.
  6448		 */
  6449		for (i = 0; i < meta.access_size; i++) {
  6450			err = check_mem_access(env, insn_idx, meta.regno, i, BPF_B,
  6451					       BPF_WRITE, -1, false);
  6452			if (err)
  6453				return err;
  6454		}
  6455	
  6456		if (func_id == BPF_FUNC_tail_call) {
  6457			err = check_reference_leak(env);
  6458			if (err) {
  6459				verbose(env, "tail_call would lead to reference leak\n");
  6460				return err;
  6461			}
  6462		} else if (is_release_function(func_id)) {
  6463			err = release_reference(env, meta.ref_obj_id);
  6464			if (err) {
  6465				verbose(env, "func %s#%d reference has not been acquired before\n",
  6466					func_id_name(func_id), func_id);
  6467				return err;
  6468			}
  6469		}
  6470	
  6471		regs = cur_regs(env);
  6472	
  6473		/* check that flags argument in get_local_storage(map, flags) is 0,
  6474		 * this is required because get_local_storage() can't return an error.
  6475		 */
  6476		if (func_id == BPF_FUNC_get_local_storage &&
  6477		    !register_is_null(&regs[BPF_REG_2])) {
  6478			verbose(env, "get_local_storage() doesn't support non-zero flags\n");
  6479			return -EINVAL;
  6480		}
  6481	
  6482		if (func_id == BPF_FUNC_for_each_map_elem) {
  6483			err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
  6484						set_map_elem_callback_state);
  6485			if (err < 0)
  6486				return -EINVAL;
  6487		}
  6488	
  6489		if (func_id == BPF_FUNC_timer_set_callback) {
  6490			err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
  6491						set_timer_callback_state);
  6492			if (err < 0)
  6493				return -EINVAL;
  6494		}
  6495	
  6496		if (func_id == BPF_FUNC_find_vma) {
  6497			err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
  6498						set_find_vma_callback_state);
  6499			if (err < 0)
  6500				return -EINVAL;
  6501		}
  6502	
  6503		if (func_id == BPF_FUNC_snprintf) {
  6504			err = check_bpf_snprintf_call(env, regs);
  6505			if (err < 0)
  6506				return err;
  6507		}
  6508	
  6509		/* reset caller saved regs */
  6510		for (i = 0; i < CALLER_SAVED_REGS; i++) {
  6511			mark_reg_not_init(env, regs, caller_saved[i]);
  6512			check_reg_arg(env, caller_saved[i], DST_OP_NO_MARK);
  6513		}
  6514	
  6515		/* helper call returns 64-bit value. */
  6516		regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
  6517	
  6518		/* update return register (already marked as written above) */
  6519		ret_type = fn->ret_type;
  6520		if (ret_type == RET_INTEGER) {
  6521			/* sets type to SCALAR_VALUE */
  6522			mark_reg_unknown(env, regs, BPF_REG_0);
  6523		} else if (ret_type == RET_VOID) {
  6524			regs[BPF_REG_0].type = NOT_INIT;
  6525		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MAP_VALUE) {
  6526			/* There is no offset yet applied, variable or fixed */
  6527			mark_reg_known_zero(env, regs, BPF_REG_0);
  6528			/* remember map_ptr, so that check_map_access()
  6529			 * can check 'value_size' boundary of memory access
  6530			 * to map element returned from bpf_map_lookup_elem()
  6531			 */
  6532			if (meta.map_ptr == NULL) {
  6533				verbose(env,
  6534					"kernel subsystem misconfigured verifier\n");
  6535				return -EINVAL;
  6536			}
  6537			regs[BPF_REG_0].map_ptr = meta.map_ptr;
  6538			regs[BPF_REG_0].map_uid = meta.map_uid;
  6539			if (ret_type_may_be_null(fn->ret_type)) {
  6540				regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
  6541			} else {
  6542				regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;
  6543				if (map_value_has_spin_lock(meta.map_ptr))
  6544					regs[BPF_REG_0].id = ++env->id_gen;
  6545			}
  6546		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCKET) {
  6547			mark_reg_known_zero(env, regs, BPF_REG_0);
  6548			regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
  6549		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCK_COMMON) {
  6550			mark_reg_known_zero(env, regs, BPF_REG_0);
  6551			regs[BPF_REG_0].type = PTR_TO_SOCK_COMMON_OR_NULL;
  6552		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_TCP_SOCK) {
  6553			mark_reg_known_zero(env, regs, BPF_REG_0);
  6554			regs[BPF_REG_0].type = PTR_TO_TCP_SOCK_OR_NULL;
  6555		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_ALLOC_MEM) {
  6556			mark_reg_known_zero(env, regs, BPF_REG_0);
  6557			regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
  6558			regs[BPF_REG_0].mem_size = meta.mem_size;
  6559		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MEM_OR_BTF_ID) {
  6560			const struct btf_type *t;
  6561	
  6562			mark_reg_known_zero(env, regs, BPF_REG_0);
  6563			t = btf_type_skip_modifiers(meta.ret_btf, meta.ret_btf_id, NULL);
  6564			if (!btf_type_is_struct(t)) {
  6565				u32 tsize;
  6566				const struct btf_type *ret;
  6567				const char *tname;
  6568	
  6569				/* resolve the type size of ksym. */
  6570				ret = btf_resolve_size(meta.ret_btf, t, &tsize);
  6571				if (IS_ERR(ret)) {
  6572					tname = btf_name_by_offset(meta.ret_btf, t->name_off);
  6573					verbose(env, "unable to resolve the size of type '%s': %ld\n",
  6574						tname, PTR_ERR(ret));
  6575					return -EINVAL;
  6576				}
  6577				regs[BPF_REG_0].type =
  6578					(ret_type & PTR_MAYBE_NULL) ?
  6579					PTR_TO_MEM_OR_NULL : PTR_TO_MEM;
  6580				regs[BPF_REG_0].mem_size = tsize;
  6581			} else {
  6582				regs[BPF_REG_0].type =
  6583					(ret_type & PTR_MAYBE_NULL) ?
  6584					PTR_TO_BTF_ID_OR_NULL : PTR_TO_BTF_ID;
  6585				regs[BPF_REG_0].btf = meta.ret_btf;
  6586				regs[BPF_REG_0].btf_id = meta.ret_btf_id;
  6587			}
  6588		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_BTF_ID) {
  6589			int ret_btf_id;
  6590	
  6591			mark_reg_known_zero(env, regs, BPF_REG_0);
  6592			regs[BPF_REG_0].type = (ret_type & PTR_MAYBE_NULL) ?
  6593							     PTR_TO_BTF_ID_OR_NULL :
  6594							     PTR_TO_BTF_ID;
  6595			ret_btf_id = *fn->ret_btf_id;
  6596			if (ret_btf_id == 0) {
  6597				verbose(env, "invalid return type %d of func %s#%d\n",
> 6598					BPF_BASE_TYPE(ret_type), func_id_name(func_id),
  6599					func_id);
  6600				return -EINVAL;
  6601			}
  6602			/* current BPF helper definitions are only coming from
  6603			 * built-in code with type IDs from  vmlinux BTF
  6604			 */
  6605			regs[BPF_REG_0].btf = btf_vmlinux;
  6606			regs[BPF_REG_0].btf_id = ret_btf_id;
  6607		} else {
  6608			verbose(env, "unknown return type %d of func %s#%d\n",
  6609				BPF_BASE_TYPE(ret_type), func_id_name(func_id), func_id);
  6610			return -EINVAL;
  6611		}
  6612	
  6613		if (reg_type_may_be_null(regs[BPF_REG_0].type))
  6614			regs[BPF_REG_0].id = ++env->id_gen;
  6615	
  6616		if (is_ptr_cast_function(func_id)) {
  6617			/* For release_reference() */
  6618			regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
  6619		} else if (is_acquire_function(func_id, meta.map_ptr)) {
  6620			int id = acquire_reference_state(env, insn_idx);
  6621	
  6622			if (id < 0)
  6623				return id;
  6624			/* For mark_ptr_or_null_reg() */
  6625			regs[BPF_REG_0].id = id;
  6626			/* For release_reference() */
  6627			regs[BPF_REG_0].ref_obj_id = id;
  6628		}
  6629	
  6630		do_refine_retval_range(regs, fn->ret_type, func_id, &meta);
  6631	
  6632		err = check_map_func_compatibility(env, meta.map_ptr, func_id);
  6633		if (err)
  6634			return err;
  6635	
  6636		if ((func_id == BPF_FUNC_get_stack ||
  6637		     func_id == BPF_FUNC_get_task_stack) &&
  6638		    !env->prog->has_callchain_buf) {
  6639			const char *err_str;
  6640	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX | PTR_MAYBE_NULL
@ 2021-11-30  3:40     ` kernel test robot
  0 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-11-30  3:40 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 12876 bytes --]

Hi Hao,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Hao-Luo/Introduce-composable-bpf-types/20211130-093143
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arm-buildonly-randconfig-r005-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301101.rEYY4B1t-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/5af019e76ba5485e0b56b5b4607c9d2e30ca6138
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hao-Luo/Introduce-composable-bpf-types/20211130-093143
        git checkout 5af019e76ba5485e0b56b5b4607c9d2e30ca6138
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash kernel/bpf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> kernel/bpf/verifier.c:6598:5: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
                                   BPF_BASE_TYPE(ret_type), func_id_name(func_id),
                                   ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:326:26: note: expanded from macro 'BPF_BASE_TYPE'
   #define BPF_BASE_TYPE(x)        ((x) & BPF_BASE_TYPE_MASK)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:6609:4: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
                           BPF_BASE_TYPE(ret_type), func_id_name(func_id), func_id);
                           ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:326:26: note: expanded from macro 'BPF_BASE_TYPE'
   #define BPF_BASE_TYPE(x)        ((x) & BPF_BASE_TYPE_MASK)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   2 warnings generated.


vim +6598 kernel/bpf/verifier.c

  6373	
  6374	static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
  6375				     int *insn_idx_p)
  6376	{
  6377		const struct bpf_func_proto *fn = NULL;
  6378		enum bpf_return_type ret_type;
  6379		struct bpf_reg_state *regs;
  6380		struct bpf_call_arg_meta meta;
  6381		int insn_idx = *insn_idx_p;
  6382		bool changes_data;
  6383		int i, err, func_id;
  6384	
  6385		/* find function prototype */
  6386		func_id = insn->imm;
  6387		if (func_id < 0 || func_id >= __BPF_FUNC_MAX_ID) {
  6388			verbose(env, "invalid func %s#%d\n", func_id_name(func_id),
  6389				func_id);
  6390			return -EINVAL;
  6391		}
  6392	
  6393		if (env->ops->get_func_proto)
  6394			fn = env->ops->get_func_proto(func_id, env->prog);
  6395		if (!fn) {
  6396			verbose(env, "unknown func %s#%d\n", func_id_name(func_id),
  6397				func_id);
  6398			return -EINVAL;
  6399		}
  6400	
  6401		/* eBPF programs must be GPL compatible to use GPL-ed functions */
  6402		if (!env->prog->gpl_compatible && fn->gpl_only) {
  6403			verbose(env, "cannot call GPL-restricted function from non-GPL compatible program\n");
  6404			return -EINVAL;
  6405		}
  6406	
  6407		if (fn->allowed && !fn->allowed(env->prog)) {
  6408			verbose(env, "helper call is not allowed in probe\n");
  6409			return -EINVAL;
  6410		}
  6411	
  6412		/* With LD_ABS/IND some JITs save/restore skb from r1. */
  6413		changes_data = bpf_helper_changes_pkt_data(fn->func);
  6414		if (changes_data && fn->arg1_type != ARG_PTR_TO_CTX) {
  6415			verbose(env, "kernel subsystem misconfigured func %s#%d: r1 != ctx\n",
  6416				func_id_name(func_id), func_id);
  6417			return -EINVAL;
  6418		}
  6419	
  6420		memset(&meta, 0, sizeof(meta));
  6421		meta.pkt_access = fn->pkt_access;
  6422	
  6423		err = check_func_proto(fn, func_id);
  6424		if (err) {
  6425			verbose(env, "kernel subsystem misconfigured func %s#%d\n",
  6426				func_id_name(func_id), func_id);
  6427			return err;
  6428		}
  6429	
  6430		meta.func_id = func_id;
  6431		/* check args */
  6432		for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) {
  6433			err = check_func_arg(env, i, &meta, fn);
  6434			if (err)
  6435				return err;
  6436		}
  6437	
  6438		err = record_func_map(env, &meta, func_id, insn_idx);
  6439		if (err)
  6440			return err;
  6441	
  6442		err = record_func_key(env, &meta, func_id, insn_idx);
  6443		if (err)
  6444			return err;
  6445	
  6446		/* Mark slots with STACK_MISC in case of raw mode, stack offset
  6447		 * is inferred from register state.
  6448		 */
  6449		for (i = 0; i < meta.access_size; i++) {
  6450			err = check_mem_access(env, insn_idx, meta.regno, i, BPF_B,
  6451					       BPF_WRITE, -1, false);
  6452			if (err)
  6453				return err;
  6454		}
  6455	
  6456		if (func_id == BPF_FUNC_tail_call) {
  6457			err = check_reference_leak(env);
  6458			if (err) {
  6459				verbose(env, "tail_call would lead to reference leak\n");
  6460				return err;
  6461			}
  6462		} else if (is_release_function(func_id)) {
  6463			err = release_reference(env, meta.ref_obj_id);
  6464			if (err) {
  6465				verbose(env, "func %s#%d reference has not been acquired before\n",
  6466					func_id_name(func_id), func_id);
  6467				return err;
  6468			}
  6469		}
  6470	
  6471		regs = cur_regs(env);
  6472	
  6473		/* check that flags argument in get_local_storage(map, flags) is 0,
  6474		 * this is required because get_local_storage() can't return an error.
  6475		 */
  6476		if (func_id == BPF_FUNC_get_local_storage &&
  6477		    !register_is_null(&regs[BPF_REG_2])) {
  6478			verbose(env, "get_local_storage() doesn't support non-zero flags\n");
  6479			return -EINVAL;
  6480		}
  6481	
  6482		if (func_id == BPF_FUNC_for_each_map_elem) {
  6483			err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
  6484						set_map_elem_callback_state);
  6485			if (err < 0)
  6486				return -EINVAL;
  6487		}
  6488	
  6489		if (func_id == BPF_FUNC_timer_set_callback) {
  6490			err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
  6491						set_timer_callback_state);
  6492			if (err < 0)
  6493				return -EINVAL;
  6494		}
  6495	
  6496		if (func_id == BPF_FUNC_find_vma) {
  6497			err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
  6498						set_find_vma_callback_state);
  6499			if (err < 0)
  6500				return -EINVAL;
  6501		}
  6502	
  6503		if (func_id == BPF_FUNC_snprintf) {
  6504			err = check_bpf_snprintf_call(env, regs);
  6505			if (err < 0)
  6506				return err;
  6507		}
  6508	
  6509		/* reset caller saved regs */
  6510		for (i = 0; i < CALLER_SAVED_REGS; i++) {
  6511			mark_reg_not_init(env, regs, caller_saved[i]);
  6512			check_reg_arg(env, caller_saved[i], DST_OP_NO_MARK);
  6513		}
  6514	
  6515		/* helper call returns 64-bit value. */
  6516		regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
  6517	
  6518		/* update return register (already marked as written above) */
  6519		ret_type = fn->ret_type;
  6520		if (ret_type == RET_INTEGER) {
  6521			/* sets type to SCALAR_VALUE */
  6522			mark_reg_unknown(env, regs, BPF_REG_0);
  6523		} else if (ret_type == RET_VOID) {
  6524			regs[BPF_REG_0].type = NOT_INIT;
  6525		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MAP_VALUE) {
  6526			/* There is no offset yet applied, variable or fixed */
  6527			mark_reg_known_zero(env, regs, BPF_REG_0);
  6528			/* remember map_ptr, so that check_map_access()
  6529			 * can check 'value_size' boundary of memory access
  6530			 * to map element returned from bpf_map_lookup_elem()
  6531			 */
  6532			if (meta.map_ptr == NULL) {
  6533				verbose(env,
  6534					"kernel subsystem misconfigured verifier\n");
  6535				return -EINVAL;
  6536			}
  6537			regs[BPF_REG_0].map_ptr = meta.map_ptr;
  6538			regs[BPF_REG_0].map_uid = meta.map_uid;
  6539			if (ret_type_may_be_null(fn->ret_type)) {
  6540				regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
  6541			} else {
  6542				regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;
  6543				if (map_value_has_spin_lock(meta.map_ptr))
  6544					regs[BPF_REG_0].id = ++env->id_gen;
  6545			}
  6546		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCKET) {
  6547			mark_reg_known_zero(env, regs, BPF_REG_0);
  6548			regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
  6549		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_SOCK_COMMON) {
  6550			mark_reg_known_zero(env, regs, BPF_REG_0);
  6551			regs[BPF_REG_0].type = PTR_TO_SOCK_COMMON_OR_NULL;
  6552		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_TCP_SOCK) {
  6553			mark_reg_known_zero(env, regs, BPF_REG_0);
  6554			regs[BPF_REG_0].type = PTR_TO_TCP_SOCK_OR_NULL;
  6555		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_ALLOC_MEM) {
  6556			mark_reg_known_zero(env, regs, BPF_REG_0);
  6557			regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
  6558			regs[BPF_REG_0].mem_size = meta.mem_size;
  6559		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MEM_OR_BTF_ID) {
  6560			const struct btf_type *t;
  6561	
  6562			mark_reg_known_zero(env, regs, BPF_REG_0);
  6563			t = btf_type_skip_modifiers(meta.ret_btf, meta.ret_btf_id, NULL);
  6564			if (!btf_type_is_struct(t)) {
  6565				u32 tsize;
  6566				const struct btf_type *ret;
  6567				const char *tname;
  6568	
  6569				/* resolve the type size of ksym. */
  6570				ret = btf_resolve_size(meta.ret_btf, t, &tsize);
  6571				if (IS_ERR(ret)) {
  6572					tname = btf_name_by_offset(meta.ret_btf, t->name_off);
  6573					verbose(env, "unable to resolve the size of type '%s': %ld\n",
  6574						tname, PTR_ERR(ret));
  6575					return -EINVAL;
  6576				}
  6577				regs[BPF_REG_0].type =
  6578					(ret_type & PTR_MAYBE_NULL) ?
  6579					PTR_TO_MEM_OR_NULL : PTR_TO_MEM;
  6580				regs[BPF_REG_0].mem_size = tsize;
  6581			} else {
  6582				regs[BPF_REG_0].type =
  6583					(ret_type & PTR_MAYBE_NULL) ?
  6584					PTR_TO_BTF_ID_OR_NULL : PTR_TO_BTF_ID;
  6585				regs[BPF_REG_0].btf = meta.ret_btf;
  6586				regs[BPF_REG_0].btf_id = meta.ret_btf_id;
  6587			}
  6588		} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_BTF_ID) {
  6589			int ret_btf_id;
  6590	
  6591			mark_reg_known_zero(env, regs, BPF_REG_0);
  6592			regs[BPF_REG_0].type = (ret_type & PTR_MAYBE_NULL) ?
  6593							     PTR_TO_BTF_ID_OR_NULL :
  6594							     PTR_TO_BTF_ID;
  6595			ret_btf_id = *fn->ret_btf_id;
  6596			if (ret_btf_id == 0) {
  6597				verbose(env, "invalid return type %d of func %s#%d\n",
> 6598					BPF_BASE_TYPE(ret_type), func_id_name(func_id),
  6599					func_id);
  6600				return -EINVAL;
  6601			}
  6602			/* current BPF helper definitions are only coming from
  6603			 * built-in code with type IDs from  vmlinux BTF
  6604			 */
  6605			regs[BPF_REG_0].btf = btf_vmlinux;
  6606			regs[BPF_REG_0].btf_id = ret_btf_id;
  6607		} else {
  6608			verbose(env, "unknown return type %d of func %s#%d\n",
  6609				BPF_BASE_TYPE(ret_type), func_id_name(func_id), func_id);
  6610			return -EINVAL;
  6611		}
  6612	
  6613		if (reg_type_may_be_null(regs[BPF_REG_0].type))
  6614			regs[BPF_REG_0].id = ++env->id_gen;
  6615	
  6616		if (is_ptr_cast_function(func_id)) {
  6617			/* For release_reference() */
  6618			regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
  6619		} else if (is_acquire_function(func_id, meta.map_ptr)) {
  6620			int id = acquire_reference_state(env, insn_idx);
  6621	
  6622			if (id < 0)
  6623				return id;
  6624			/* For mark_ptr_or_null_reg() */
  6625			regs[BPF_REG_0].id = id;
  6626			/* For release_reference() */
  6627			regs[BPF_REG_0].ref_obj_id = id;
  6628		}
  6629	
  6630		do_refine_retval_range(regs, fn->ret_type, func_id, &meta);
  6631	
  6632		err = check_map_func_compatibility(env, meta.map_ptr, func_id);
  6633		if (err)
  6634			return err;
  6635	
  6636		if ((func_id == BPF_FUNC_get_stack ||
  6637		     func_id == BPF_FUNC_get_task_stack) &&
  6638		    !env->prog->has_callchain_buf) {
  6639			const char *err_str;
  6640	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX " Hao Luo
@ 2021-11-30  4:21     ` kernel test robot
  2021-11-30  4:21     ` kernel test robot
  2021-11-30  4:31   ` kernel test robot
  2 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-11-30  4:21 UTC (permalink / raw)
  To: Hao Luo; +Cc: llvm, kbuild-all

Hi Hao,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Hao-Luo/Introduce-composable-bpf-types/20211130-093143
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arm-buildonly-randconfig-r005-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301214.JYWypTyw-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/9e92c0a723fc173ac102b3bb27df479a01f32896
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hao-Luo/Introduce-composable-bpf-types/20211130-093143
        git checkout 9e92c0a723fc173ac102b3bb27df479a01f32896
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash kernel/bpf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> kernel/bpf/verifier.c:533:21: warning: 'const' type qualifier on return type has no effect [-Wignored-qualifiers]
   static const char * const reg_type_str(enum bpf_reg_type type)
                       ^~~~~~
   kernel/bpf/verifier.c:6545:5: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
                                   BPF_BASE_TYPE(ret_type), func_id_name(func_id),
                                   ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:326:26: note: expanded from macro 'BPF_BASE_TYPE'
   #define BPF_BASE_TYPE(x)        ((x) & BPF_BASE_TYPE_MASK)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:6556:4: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
                           BPF_BASE_TYPE(ret_type), func_id_name(func_id), func_id);
                           ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:326:26: note: expanded from macro 'BPF_BASE_TYPE'
   #define BPF_BASE_TYPE(x)        ((x) & BPF_BASE_TYPE_MASK)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.


vim +/const +533 kernel/bpf/verifier.c

   531	
   532	/* string representation of 'enum bpf_reg_type' */
 > 533	static const char * const reg_type_str(enum bpf_reg_type type)
   534	{
   535		static const char * const str[] = {
   536			[NOT_INIT]		= "?",
   537			[SCALAR_VALUE]		= "inv",
   538			[PTR_TO_CTX]		= "ctx",
   539			[CONST_PTR_TO_MAP]	= "map_ptr",
   540			[PTR_TO_MAP_VALUE]	= "map_value",
   541			[PTR_TO_STACK]		= "fp",
   542			[PTR_TO_PACKET]		= "pkt",
   543			[PTR_TO_PACKET_META]	= "pkt_meta",
   544			[PTR_TO_PACKET_END]	= "pkt_end",
   545			[PTR_TO_FLOW_KEYS]	= "flow_keys",
   546			[PTR_TO_SOCKET]		= "sock",
   547			[PTR_TO_SOCK_COMMON]	= "sock_common",
   548			[PTR_TO_TCP_SOCK]	= "tcp_sock",
   549			[PTR_TO_TP_BUFFER]	= "tp_buffer",
   550			[PTR_TO_XDP_SOCK]	= "xdp_sock",
   551			[PTR_TO_BTF_ID]		= "ptr_",
   552			[PTR_TO_PERCPU_BTF_ID]	= "percpu_ptr_",
   553			[PTR_TO_MEM]		= "mem",
   554			[PTR_TO_RDONLY_BUF]	= "rdonly_buf",
   555			[PTR_TO_RDWR_BUF]	= "rdwr_buf",
   556			[PTR_TO_FUNC]		= "func",
   557			[PTR_TO_MAP_KEY]	= "map_key",
   558		};
   559	
   560		return str[BPF_BASE_TYPE(type)];
   561	}
   562	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL
@ 2021-11-30  4:21     ` kernel test robot
  0 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-11-30  4:21 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4017 bytes --]

Hi Hao,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Hao-Luo/Introduce-composable-bpf-types/20211130-093143
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arm-buildonly-randconfig-r005-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301214.JYWypTyw-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/9e92c0a723fc173ac102b3bb27df479a01f32896
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hao-Luo/Introduce-composable-bpf-types/20211130-093143
        git checkout 9e92c0a723fc173ac102b3bb27df479a01f32896
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash kernel/bpf/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> kernel/bpf/verifier.c:533:21: warning: 'const' type qualifier on return type has no effect [-Wignored-qualifiers]
   static const char * const reg_type_str(enum bpf_reg_type type)
                       ^~~~~~
   kernel/bpf/verifier.c:6545:5: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
                                   BPF_BASE_TYPE(ret_type), func_id_name(func_id),
                                   ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:326:26: note: expanded from macro 'BPF_BASE_TYPE'
   #define BPF_BASE_TYPE(x)        ((x) & BPF_BASE_TYPE_MASK)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:6556:4: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
                           BPF_BASE_TYPE(ret_type), func_id_name(func_id), func_id);
                           ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:326:26: note: expanded from macro 'BPF_BASE_TYPE'
   #define BPF_BASE_TYPE(x)        ((x) & BPF_BASE_TYPE_MASK)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.


vim +/const +533 kernel/bpf/verifier.c

   531	
   532	/* string representation of 'enum bpf_reg_type' */
 > 533	static const char * const reg_type_str(enum bpf_reg_type type)
   534	{
   535		static const char * const str[] = {
   536			[NOT_INIT]		= "?",
   537			[SCALAR_VALUE]		= "inv",
   538			[PTR_TO_CTX]		= "ctx",
   539			[CONST_PTR_TO_MAP]	= "map_ptr",
   540			[PTR_TO_MAP_VALUE]	= "map_value",
   541			[PTR_TO_STACK]		= "fp",
   542			[PTR_TO_PACKET]		= "pkt",
   543			[PTR_TO_PACKET_META]	= "pkt_meta",
   544			[PTR_TO_PACKET_END]	= "pkt_end",
   545			[PTR_TO_FLOW_KEYS]	= "flow_keys",
   546			[PTR_TO_SOCKET]		= "sock",
   547			[PTR_TO_SOCK_COMMON]	= "sock_common",
   548			[PTR_TO_TCP_SOCK]	= "tcp_sock",
   549			[PTR_TO_TP_BUFFER]	= "tp_buffer",
   550			[PTR_TO_XDP_SOCK]	= "xdp_sock",
   551			[PTR_TO_BTF_ID]		= "ptr_",
   552			[PTR_TO_PERCPU_BTF_ID]	= "percpu_ptr_",
   553			[PTR_TO_MEM]		= "mem",
   554			[PTR_TO_RDONLY_BUF]	= "rdonly_buf",
   555			[PTR_TO_RDWR_BUF]	= "rdwr_buf",
   556			[PTR_TO_FUNC]		= "func",
   557			[PTR_TO_MAP_KEY]	= "map_key",
   558		};
   559	
   560		return str[BPF_BASE_TYPE(type)];
   561	}
   562	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX " Hao Luo
  2021-11-30  3:30   ` kernel test robot
  2021-11-30  4:21     ` kernel test robot
@ 2021-11-30  4:31   ` kernel test robot
  2 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-11-30  4:31 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4563 bytes --]

Hi Hao,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Hao-Luo/Introduce-composable-bpf-types/20211130-093143
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20211130/202111301226.380sF7wF-lkp(a)intel.com/config)
compiler: powerpc-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9e92c0a723fc173ac102b3bb27df479a01f32896
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hao-Luo/Introduce-composable-bpf-types/20211130-093143
        git checkout 9e92c0a723fc173ac102b3bb27df479a01f32896
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/net/ethernet/netronome/nfp/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from <command-line>:
   drivers/net/ethernet/netronome/nfp/bpf/verifier.c: In function 'nfp_bpf_check_helper_call':
>> include/linux/compiler_types.h:335:45: error: call to '__compiletime_assert_631' declared with attribute error: BUILD_BUG_ON failed: NFP_BPF_SCALAR_VALUE != SCALAR_VALUE || NFP_BPF_MAP_VALUE != PTR_TO_MAP_VALUE || NFP_BPF_STACK != PTR_TO_STACK || NFP_BPF_PACKET_DATA != PTR_TO_PACKET
     335 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/compiler_types.h:316:25: note: in definition of macro '__compiletime_assert'
     316 |                         prefix ## suffix();                             \
         |                         ^~~~~~
   include/linux/compiler_types.h:335:9: note: in expansion of macro '_compiletime_assert'
     335 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
      50 |         BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
         |         ^~~~~~~~~~~~~~~~
   drivers/net/ethernet/netronome/nfp/bpf/verifier.c:234:17: note: in expansion of macro 'BUILD_BUG_ON'
     234 |                 BUILD_BUG_ON(NFP_BPF_SCALAR_VALUE != SCALAR_VALUE ||
         |                 ^~~~~~~~~~~~


vim +/__compiletime_assert_631 +335 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  321  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  322  #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  323  	__compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  324  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  325  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  326   * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  327   * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  328   * @msg:       a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  329   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  330   * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  331   * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  332   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  333   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  334  #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @335  	_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  336  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 1/9] bpf: Introduce composable reg, ret and arg types.
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 1/9] bpf: Introduce composable reg, ret and arg types Hao Luo
@ 2021-12-01 20:29   ` Alexei Starovoitov
  2021-12-01 22:36     ` Hao Luo
  0 siblings, 1 reply; 28+ messages in thread
From: Alexei Starovoitov @ 2021-12-01 20:29 UTC (permalink / raw)
  To: Hao Luo
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Mon, Nov 29, 2021 at 05:29:40PM -0800, Hao Luo wrote:
> There are some common properties shared between bpf reg, ret and arg
> values. For instance, a value may be a NULL pointer, or a pointer to
> a read-only memory. Previously, to express these properties, enumeration
> was used. For example, in order to test whether a reg value can be NULL,
> reg_type_may_be_null() simply enumerates all types that are possibly
> NULL. The problem of this approach is that it's not scalable and causes
> a lot of duplication. These properties can be combined, for example, a
> type could be either MAYBE_NULL or RDONLY, or both.
> 
> This patch series rewrites the layout of reg_type, arg_type and
> ret_type, so that common properties can be extracted and represented as
> composable flag. For example, one can write
> 
>  ARG_PTR_TO_MEM | PTR_MAYBE_NULL
> 
> which is equivalent to the previous
> 
>  ARG_PTR_TO_MEM_OR_NULL
> 
> The type ARG_PTR_TO_MEM are called "base types" in this patch. Base
> types can be extended with flags. A flag occupies the higher bits while
> base types sits in the lower bits.
> 
> This patch in particular sets up a set of macro for this purpose. The
> followed patches rewrites arg_types, ret_types and reg_types
> respectively.
> 
> Signed-off-by: Hao Luo <haoluo@google.com>
> ---
>  include/linux/bpf.h | 50 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index cc7a0c36e7df..b592b3f7d223 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -297,6 +297,37 @@ bool bpf_map_meta_equal(const struct bpf_map *meta0,
>  
>  extern const struct bpf_map_ops bpf_map_offload_ops;
>  
> +/* bpf_type_flag contains a set of flags that are applicable to the values of
> + * arg_type, ret_type and reg_type. For example, a pointer value may be null,
> + * or a memory is read-only. We classify types into two categories: base types
> + * and extended types. Extended types are base types combined with a type flag.
> + *
> + * Currently there are no more than 32 base types in arg_type, ret_type and
> + * reg_types.
> + */
> +#define BPF_BASE_TYPE_BITS	8
> +
> +enum bpf_type_flag {
> +	/* PTR may be NULL. */
> +	PTR_MAYBE_NULL		= BIT(0 + BPF_BASE_TYPE_BITS),
> +
> +	__BPF_TYPE_LAST_FLAG	= PTR_MAYBE_NULL,
> +};
> +
> +#define BPF_BASE_TYPE_MASK	GENMASK(BPF_BASE_TYPE_BITS, 0)
> +
> +/* Max number of base types. */
> +#define BPF_BASE_TYPE_LIMIT	(1UL << BPF_BASE_TYPE_BITS)
> +
> +/* Max number of all types. */
> +#define BPF_TYPE_LIMIT		(__BPF_TYPE_LAST_FLAG | (__BPF_TYPE_LAST_FLAG - 1))
> +
> +/* extract base type. */
> +#define BPF_BASE_TYPE(x)	((x) & BPF_BASE_TYPE_MASK)
> +
> +/* extract flags from an extended type. */
> +#define BPF_TYPE_FLAG(x)	((enum bpf_type_flag)((x) & ~BPF_BASE_TYPE_MASK))

Overall I think it's really great.
The only suggestion is to use:
static inline u32 base_type(u32 x)
{
  return x & BPF_BASE_TYPE_MASK;
}
and
static inline u32 type_flag(u32 x) ..

The capital letter macros are too loud.

wdyt?


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX | PTR_MAYBE_NULL
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX " Hao Luo
  2021-11-30  2:59   ` kernel test robot
  2021-11-30  3:40     ` kernel test robot
@ 2021-12-01 20:30   ` Alexei Starovoitov
  2021-12-01 22:40     ` Hao Luo
  2 siblings, 1 reply; 28+ messages in thread
From: Alexei Starovoitov @ 2021-12-01 20:30 UTC (permalink / raw)
  To: Hao Luo
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Mon, Nov 29, 2021 at 05:29:42PM -0800, Hao Luo wrote:
>  	/* update return register (already marked as written above) */
> -	if (fn->ret_type == RET_INTEGER) {
> +	ret_type = fn->ret_type;
> +	if (ret_type == RET_INTEGER) {
>  		/* sets type to SCALAR_VALUE */
>  		mark_reg_unknown(env, regs, BPF_REG_0);
> -	} else if (fn->ret_type == RET_VOID) {
> +	} else if (ret_type == RET_VOID) {
>  		regs[BPF_REG_0].type = NOT_INIT;
> -	} else if (fn->ret_type == RET_PTR_TO_MAP_VALUE_OR_NULL ||
> -		   fn->ret_type == RET_PTR_TO_MAP_VALUE) {
> +	} else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MAP_VALUE) {
>  		/* There is no offset yet applied, variable or fixed */
>  		mark_reg_known_zero(env, regs, BPF_REG_0);
>  		/* remember map_ptr, so that check_map_access()
> @@ -6530,28 +6536,27 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>  		}
>  		regs[BPF_REG_0].map_ptr = meta.map_ptr;
>  		regs[BPF_REG_0].map_uid = meta.map_uid;
> -		if (fn->ret_type == RET_PTR_TO_MAP_VALUE) {
> +		if (ret_type_may_be_null(fn->ret_type)) {

it should have been ret_type here?

> +			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
> +		} else {
>  			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.
  2021-11-30  1:29 ` [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem Hao Luo
@ 2021-12-01 20:34   ` Alexei Starovoitov
  2021-12-01 22:21     ` Hao Luo
  0 siblings, 1 reply; 28+ messages in thread
From: Alexei Starovoitov @ 2021-12-01 20:34 UTC (permalink / raw)
  To: Hao Luo
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote:
>  
> +
>  struct bpf_reg_types {
>  	const enum bpf_reg_type types[10];
>  	u32 *btf_id;
> +
> +	/* Certain types require customized type matching function. */
> +	bool (*type_match_fn)(enum bpf_arg_type arg_type,
> +			      enum bpf_reg_type type,
> +			      enum bpf_reg_type expected);
>  };
>  
>  static const struct bpf_reg_types map_key_value_types = {
> @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = {
>  };
>  #endif
>  
> +static bool mem_type_match(enum bpf_arg_type arg_type,
> +			   enum bpf_reg_type type, enum bpf_reg_type expected)
> +{
> +	/* If arg_type is tagged with MEM_RDONLY, type is compatible with both
> +	 * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before
> +	 * comparison.
> +	 */
> +	if ((arg_type & MEM_RDONLY) != 0)
> +		type &= ~MEM_RDONLY;
> +
> +	return type == expected;
> +}
> +
>  static const struct bpf_reg_types mem_types = {
>  	.types = {
>  		PTR_TO_STACK,
> @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = {
>  		PTR_TO_MAP_VALUE,
>  		PTR_TO_MEM,
>  		PTR_TO_BUF,
> -		PTR_TO_BUF | MEM_RDONLY,
>  	},
> +	.type_match_fn = mem_type_match,

why add a callback for this logic?
Isn't it a universal rule for MEM_RDONLY?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.
  2021-12-01 20:34   ` Alexei Starovoitov
@ 2021-12-01 22:21     ` Hao Luo
  2021-12-02  3:53       ` Alexei Starovoitov
  0 siblings, 1 reply; 28+ messages in thread
From: Hao Luo @ 2021-12-01 22:21 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote:
> >
> > +
> >  struct bpf_reg_types {
> >       const enum bpf_reg_type types[10];
> >       u32 *btf_id;
> > +
> > +     /* Certain types require customized type matching function. */
> > +     bool (*type_match_fn)(enum bpf_arg_type arg_type,
> > +                           enum bpf_reg_type type,
> > +                           enum bpf_reg_type expected);
> >  };
> >
> >  static const struct bpf_reg_types map_key_value_types = {
> > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = {
> >  };
> >  #endif
> >
> > +static bool mem_type_match(enum bpf_arg_type arg_type,
> > +                        enum bpf_reg_type type, enum bpf_reg_type expected)
> > +{
> > +     /* If arg_type is tagged with MEM_RDONLY, type is compatible with both
> > +      * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before
> > +      * comparison.
> > +      */
> > +     if ((arg_type & MEM_RDONLY) != 0)
> > +             type &= ~MEM_RDONLY;
> > +
> > +     return type == expected;
> > +}
> > +
> >  static const struct bpf_reg_types mem_types = {
> >       .types = {
> >               PTR_TO_STACK,
> > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = {
> >               PTR_TO_MAP_VALUE,
> >               PTR_TO_MEM,
> >               PTR_TO_BUF,
> > -             PTR_TO_BUF | MEM_RDONLY,
> >       },
> > +     .type_match_fn = mem_type_match,
>
> why add a callback for this logic?
> Isn't it a universal rule for MEM_RDONLY?

Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but
all flags can be checked in the same way? Like the following

 static const struct bpf_reg_types int_ptr_types = {
@@ -5097,6 +5116,13 @@ static int check_reg_type(struct
bpf_verifier_env *env, u32 regno,
                if (expected == NOT_INIT)
                        break;

+               flag = bpf_type_flag(arg_type);

-               if (type == expected)
+               if ((type & ~flag) == expected)
                        goto found;
        }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 1/9] bpf: Introduce composable reg, ret and arg types.
  2021-12-01 20:29   ` Alexei Starovoitov
@ 2021-12-01 22:36     ` Hao Luo
  0 siblings, 0 replies; 28+ messages in thread
From: Hao Luo @ 2021-12-01 22:36 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Wed, Dec 1, 2021 at 12:29 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Nov 29, 2021 at 05:29:40PM -0800, Hao Luo wrote:
> > There are some common properties shared between bpf reg, ret and arg
> > values. For instance, a value may be a NULL pointer, or a pointer to
> > a read-only memory. Previously, to express these properties, enumeration
> > was used. For example, in order to test whether a reg value can be NULL,
> > reg_type_may_be_null() simply enumerates all types that are possibly
> > NULL. The problem of this approach is that it's not scalable and causes
> > a lot of duplication. These properties can be combined, for example, a
> > type could be either MAYBE_NULL or RDONLY, or both.
> >
> > This patch series rewrites the layout of reg_type, arg_type and
> > ret_type, so that common properties can be extracted and represented as
> > composable flag. For example, one can write
> >
> >  ARG_PTR_TO_MEM | PTR_MAYBE_NULL
> >
> > which is equivalent to the previous
> >
> >  ARG_PTR_TO_MEM_OR_NULL
> >
> > The type ARG_PTR_TO_MEM are called "base types" in this patch. Base
> > types can be extended with flags. A flag occupies the higher bits while
> > base types sits in the lower bits.
> >
> > This patch in particular sets up a set of macro for this purpose. The
> > followed patches rewrites arg_types, ret_types and reg_types
> > respectively.
> >
> > Signed-off-by: Hao Luo <haoluo@google.com>
> > ---
> >  include/linux/bpf.h | 50 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index cc7a0c36e7df..b592b3f7d223 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -297,6 +297,37 @@ bool bpf_map_meta_equal(const struct bpf_map *meta0,
> >
> >  extern const struct bpf_map_ops bpf_map_offload_ops;
> >
> > +/* bpf_type_flag contains a set of flags that are applicable to the values of
> > + * arg_type, ret_type and reg_type. For example, a pointer value may be null,
> > + * or a memory is read-only. We classify types into two categories: base types
> > + * and extended types. Extended types are base types combined with a type flag.
> > + *
> > + * Currently there are no more than 32 base types in arg_type, ret_type and
> > + * reg_types.
> > + */
> > +#define BPF_BASE_TYPE_BITS   8
> > +
> > +enum bpf_type_flag {
> > +     /* PTR may be NULL. */
> > +     PTR_MAYBE_NULL          = BIT(0 + BPF_BASE_TYPE_BITS),
> > +
> > +     __BPF_TYPE_LAST_FLAG    = PTR_MAYBE_NULL,
> > +};
> > +
> > +#define BPF_BASE_TYPE_MASK   GENMASK(BPF_BASE_TYPE_BITS, 0)
> > +
> > +/* Max number of base types. */
> > +#define BPF_BASE_TYPE_LIMIT  (1UL << BPF_BASE_TYPE_BITS)
> > +
> > +/* Max number of all types. */
> > +#define BPF_TYPE_LIMIT               (__BPF_TYPE_LAST_FLAG | (__BPF_TYPE_LAST_FLAG - 1))
> > +
> > +/* extract base type. */
> > +#define BPF_BASE_TYPE(x)     ((x) & BPF_BASE_TYPE_MASK)
> > +
> > +/* extract flags from an extended type. */
> > +#define BPF_TYPE_FLAG(x)     ((enum bpf_type_flag)((x) & ~BPF_BASE_TYPE_MASK))
>
> Overall I think it's really great.
> The only suggestion is to use:
> static inline u32 base_type(u32 x)
> {
>   return x & BPF_BASE_TYPE_MASK;
> }
> and
> static inline u32 type_flag(u32 x) ..
>
> The capital letter macros are too loud.
>
> wdyt?
>

Sounds good to me. Will do in the next iteration.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX | PTR_MAYBE_NULL
  2021-12-01 20:30   ` Alexei Starovoitov
@ 2021-12-01 22:40     ` Hao Luo
  0 siblings, 0 replies; 28+ messages in thread
From: Hao Luo @ 2021-12-01 22:40 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Wed, Dec 1, 2021 at 12:30 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Nov 29, 2021 at 05:29:42PM -0800, Hao Luo wrote:
> >       /* update return register (already marked as written above) */
> > -     if (fn->ret_type == RET_INTEGER) {
> > +     ret_type = fn->ret_type;
> > +     if (ret_type == RET_INTEGER) {
> >               /* sets type to SCALAR_VALUE */
> >               mark_reg_unknown(env, regs, BPF_REG_0);
> > -     } else if (fn->ret_type == RET_VOID) {
> > +     } else if (ret_type == RET_VOID) {
> >               regs[BPF_REG_0].type = NOT_INIT;
> > -     } else if (fn->ret_type == RET_PTR_TO_MAP_VALUE_OR_NULL ||
> > -                fn->ret_type == RET_PTR_TO_MAP_VALUE) {
> > +     } else if (BPF_BASE_TYPE(ret_type) == RET_PTR_TO_MAP_VALUE) {
> >               /* There is no offset yet applied, variable or fixed */
> >               mark_reg_known_zero(env, regs, BPF_REG_0);
> >               /* remember map_ptr, so that check_map_access()
> > @@ -6530,28 +6536,27 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> >               }
> >               regs[BPF_REG_0].map_ptr = meta.map_ptr;
> >               regs[BPF_REG_0].map_uid = meta.map_uid;
> > -             if (fn->ret_type == RET_PTR_TO_MAP_VALUE) {
> > +             if (ret_type_may_be_null(fn->ret_type)) {
>
> it should have been ret_type here?
>

Yes. I think fn->ret_type and ret_type are the same here. I can
replace it with 'ret_type' in the next version.

> > +                     regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
> > +             } else {
> >                       regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.
  2021-12-01 22:21     ` Hao Luo
@ 2021-12-02  3:53       ` Alexei Starovoitov
  2021-12-02 18:42         ` Hao Luo
  0 siblings, 1 reply; 28+ messages in thread
From: Alexei Starovoitov @ 2021-12-02  3:53 UTC (permalink / raw)
  To: Hao Luo
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@google.com> wrote:
>
> On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote:
> > >
> > > +
> > >  struct bpf_reg_types {
> > >       const enum bpf_reg_type types[10];
> > >       u32 *btf_id;
> > > +
> > > +     /* Certain types require customized type matching function. */
> > > +     bool (*type_match_fn)(enum bpf_arg_type arg_type,
> > > +                           enum bpf_reg_type type,
> > > +                           enum bpf_reg_type expected);
> > >  };
> > >
> > >  static const struct bpf_reg_types map_key_value_types = {
> > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = {
> > >  };
> > >  #endif
> > >
> > > +static bool mem_type_match(enum bpf_arg_type arg_type,
> > > +                        enum bpf_reg_type type, enum bpf_reg_type expected)
> > > +{
> > > +     /* If arg_type is tagged with MEM_RDONLY, type is compatible with both
> > > +      * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before
> > > +      * comparison.
> > > +      */
> > > +     if ((arg_type & MEM_RDONLY) != 0)
> > > +             type &= ~MEM_RDONLY;
> > > +
> > > +     return type == expected;
> > > +}
> > > +
> > >  static const struct bpf_reg_types mem_types = {
> > >       .types = {
> > >               PTR_TO_STACK,
> > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = {
> > >               PTR_TO_MAP_VALUE,
> > >               PTR_TO_MEM,
> > >               PTR_TO_BUF,
> > > -             PTR_TO_BUF | MEM_RDONLY,
> > >       },
> > > +     .type_match_fn = mem_type_match,
> >
> > why add a callback for this logic?
> > Isn't it a universal rule for MEM_RDONLY?
>
> Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but
> all flags can be checked in the same way? Like the following
>
>  static const struct bpf_reg_types int_ptr_types = {
> @@ -5097,6 +5116,13 @@ static int check_reg_type(struct
> bpf_verifier_env *env, u32 regno,
>                 if (expected == NOT_INIT)
>                         break;
>
> +               flag = bpf_type_flag(arg_type);
>
> -               if (type == expected)
> +               if ((type & ~flag) == expected)
>                         goto found;
>         }

I think for MAYBE_NULL and MEM_RDONLY that would be correct,
but not necessarily apply to future flags.
I would open code it for these specific flags.
Also what do you think about dropping bpf_ prefix from type_flag()?
It won't conflict with anything and less verbose.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.
  2021-12-02  3:53       ` Alexei Starovoitov
@ 2021-12-02 18:42         ` Hao Luo
  2021-12-02 21:13           ` Alexei Starovoitov
  0 siblings, 1 reply; 28+ messages in thread
From: Hao Luo @ 2021-12-02 18:42 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Wed, Dec 1, 2021 at 7:53 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@google.com> wrote:
> >
> > On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote:
> > > >
> > > > +
> > > >  struct bpf_reg_types {
> > > >       const enum bpf_reg_type types[10];
> > > >       u32 *btf_id;
> > > > +
> > > > +     /* Certain types require customized type matching function. */
> > > > +     bool (*type_match_fn)(enum bpf_arg_type arg_type,
> > > > +                           enum bpf_reg_type type,
> > > > +                           enum bpf_reg_type expected);
> > > >  };
> > > >
> > > >  static const struct bpf_reg_types map_key_value_types = {
> > > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = {
> > > >  };
> > > >  #endif
> > > >
> > > > +static bool mem_type_match(enum bpf_arg_type arg_type,
> > > > +                        enum bpf_reg_type type, enum bpf_reg_type expected)
> > > > +{
> > > > +     /* If arg_type is tagged with MEM_RDONLY, type is compatible with both
> > > > +      * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before
> > > > +      * comparison.
> > > > +      */
> > > > +     if ((arg_type & MEM_RDONLY) != 0)
> > > > +             type &= ~MEM_RDONLY;
> > > > +
> > > > +     return type == expected;
> > > > +}
> > > > +
> > > >  static const struct bpf_reg_types mem_types = {
> > > >       .types = {
> > > >               PTR_TO_STACK,
> > > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = {
> > > >               PTR_TO_MAP_VALUE,
> > > >               PTR_TO_MEM,
> > > >               PTR_TO_BUF,
> > > > -             PTR_TO_BUF | MEM_RDONLY,
> > > >       },
> > > > +     .type_match_fn = mem_type_match,
> > >
> > > why add a callback for this logic?
> > > Isn't it a universal rule for MEM_RDONLY?
> >
> > Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but
> > all flags can be checked in the same way? Like the following
> >
> >  static const struct bpf_reg_types int_ptr_types = {
> > @@ -5097,6 +5116,13 @@ static int check_reg_type(struct
> > bpf_verifier_env *env, u32 regno,
> >                 if (expected == NOT_INIT)
> >                         break;
> >
> > +               flag = bpf_type_flag(arg_type);
> >
> > -               if (type == expected)
> > +               if ((type & ~flag) == expected)
> >                         goto found;
> >         }
>
> I think for MAYBE_NULL and MEM_RDONLY that would be correct,
> but not necessarily apply to future flags.
> I would open code it for these specific flags.

ack.

> Also what do you think about dropping bpf_ prefix from type_flag()?
> It won't conflict with anything and less verbose.

Sounds good as long as it won't conflict. IMO it would be good to have
an internal header in kernel/bpf. It appears to me that we put
everything in linux/bpf.h now. In sched, there is a
kernel/sched/sched.h used internally in kernel/sched and a
linux/sched.h that is public to other subsystems.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.
  2021-12-02 18:42         ` Hao Luo
@ 2021-12-02 21:13           ` Alexei Starovoitov
  2021-12-03  0:14             ` Hao Luo
  0 siblings, 1 reply; 28+ messages in thread
From: Alexei Starovoitov @ 2021-12-02 21:13 UTC (permalink / raw)
  To: Hao Luo
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Thu, Dec 2, 2021 at 10:42 AM Hao Luo <haoluo@google.com> wrote:
>
> On Wed, Dec 1, 2021 at 7:53 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@google.com> wrote:
> > >
> > > On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote:
> > > > >
> > > > > +
> > > > >  struct bpf_reg_types {
> > > > >       const enum bpf_reg_type types[10];
> > > > >       u32 *btf_id;
> > > > > +
> > > > > +     /* Certain types require customized type matching function. */
> > > > > +     bool (*type_match_fn)(enum bpf_arg_type arg_type,
> > > > > +                           enum bpf_reg_type type,
> > > > > +                           enum bpf_reg_type expected);
> > > > >  };
> > > > >
> > > > >  static const struct bpf_reg_types map_key_value_types = {
> > > > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = {
> > > > >  };
> > > > >  #endif
> > > > >
> > > > > +static bool mem_type_match(enum bpf_arg_type arg_type,
> > > > > +                        enum bpf_reg_type type, enum bpf_reg_type expected)
> > > > > +{
> > > > > +     /* If arg_type is tagged with MEM_RDONLY, type is compatible with both
> > > > > +      * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before
> > > > > +      * comparison.
> > > > > +      */
> > > > > +     if ((arg_type & MEM_RDONLY) != 0)
> > > > > +             type &= ~MEM_RDONLY;
> > > > > +
> > > > > +     return type == expected;
> > > > > +}
> > > > > +
> > > > >  static const struct bpf_reg_types mem_types = {
> > > > >       .types = {
> > > > >               PTR_TO_STACK,
> > > > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = {
> > > > >               PTR_TO_MAP_VALUE,
> > > > >               PTR_TO_MEM,
> > > > >               PTR_TO_BUF,
> > > > > -             PTR_TO_BUF | MEM_RDONLY,
> > > > >       },
> > > > > +     .type_match_fn = mem_type_match,
> > > >
> > > > why add a callback for this logic?
> > > > Isn't it a universal rule for MEM_RDONLY?
> > >
> > > Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but
> > > all flags can be checked in the same way? Like the following
> > >
> > >  static const struct bpf_reg_types int_ptr_types = {
> > > @@ -5097,6 +5116,13 @@ static int check_reg_type(struct
> > > bpf_verifier_env *env, u32 regno,
> > >                 if (expected == NOT_INIT)
> > >                         break;
> > >
> > > +               flag = bpf_type_flag(arg_type);
> > >
> > > -               if (type == expected)
> > > +               if ((type & ~flag) == expected)
> > >                         goto found;
> > >         }
> >
> > I think for MAYBE_NULL and MEM_RDONLY that would be correct,
> > but not necessarily apply to future flags.
> > I would open code it for these specific flags.
>
> ack.
>
> > Also what do you think about dropping bpf_ prefix from type_flag()?
> > It won't conflict with anything and less verbose.
>
> Sounds good as long as it won't conflict. IMO it would be good to have
> an internal header in kernel/bpf. It appears to me that we put
> everything in linux/bpf.h now. In sched, there is a
> kernel/sched/sched.h used internally in kernel/sched and a
> linux/sched.h that is public to other subsystems.

yeah. That's long overdue. We have include/linux/bpf_verifier.h
that might work for this case too.
Or even directly in verifier.c (if possible).

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem.
  2021-12-02 21:13           ` Alexei Starovoitov
@ 2021-12-03  0:14             ` Hao Luo
  0 siblings, 0 replies; 28+ messages in thread
From: Hao Luo @ 2021-12-03  0:14 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh, bpf

On Thu, Dec 2, 2021 at 1:13 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Dec 2, 2021 at 10:42 AM Hao Luo <haoluo@google.com> wrote:
> >
> > On Wed, Dec 1, 2021 at 7:53 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@google.com> wrote:
> > > >
> > > > On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote:
> > > > > >
> > > > > > +
> > > > > >  struct bpf_reg_types {
> > > > > >       const enum bpf_reg_type types[10];
> > > > > >       u32 *btf_id;
> > > > > > +
> > > > > > +     /* Certain types require customized type matching function. */
> > > > > > +     bool (*type_match_fn)(enum bpf_arg_type arg_type,
> > > > > > +                           enum bpf_reg_type type,
> > > > > > +                           enum bpf_reg_type expected);
> > > > > >  };
> > > > > >
> > > > > >  static const struct bpf_reg_types map_key_value_types = {
> > > > > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = {
> > > > > >  };
> > > > > >  #endif
> > > > > >
> > > > > > +static bool mem_type_match(enum bpf_arg_type arg_type,
> > > > > > +                        enum bpf_reg_type type, enum bpf_reg_type expected)
> > > > > > +{
> > > > > > +     /* If arg_type is tagged with MEM_RDONLY, type is compatible with both
> > > > > > +      * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before
> > > > > > +      * comparison.
> > > > > > +      */
> > > > > > +     if ((arg_type & MEM_RDONLY) != 0)
> > > > > > +             type &= ~MEM_RDONLY;
> > > > > > +
> > > > > > +     return type == expected;
> > > > > > +}
> > > > > > +
> > > > > >  static const struct bpf_reg_types mem_types = {
> > > > > >       .types = {
> > > > > >               PTR_TO_STACK,
> > > > > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = {
> > > > > >               PTR_TO_MAP_VALUE,
> > > > > >               PTR_TO_MEM,
> > > > > >               PTR_TO_BUF,
> > > > > > -             PTR_TO_BUF | MEM_RDONLY,
> > > > > >       },
> > > > > > +     .type_match_fn = mem_type_match,
> > > > >
> > > > > why add a callback for this logic?
> > > > > Isn't it a universal rule for MEM_RDONLY?
> > > >
> > > > Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but
> > > > all flags can be checked in the same way? Like the following
> > > >
> > > >  static const struct bpf_reg_types int_ptr_types = {
> > > > @@ -5097,6 +5116,13 @@ static int check_reg_type(struct
> > > > bpf_verifier_env *env, u32 regno,
> > > >                 if (expected == NOT_INIT)
> > > >                         break;
> > > >
> > > > +               flag = bpf_type_flag(arg_type);
> > > >
> > > > -               if (type == expected)
> > > > +               if ((type & ~flag) == expected)
> > > >                         goto found;
> > > >         }
> > >
> > > I think for MAYBE_NULL and MEM_RDONLY that would be correct,
> > > but not necessarily apply to future flags.
> > > I would open code it for these specific flags.
> >
> > ack.
> >
> > > Also what do you think about dropping bpf_ prefix from type_flag()?
> > > It won't conflict with anything and less verbose.
> >
> > Sounds good as long as it won't conflict. IMO it would be good to have
> > an internal header in kernel/bpf. It appears to me that we put
> > everything in linux/bpf.h now. In sched, there is a
> > kernel/sched/sched.h used internally in kernel/sched and a
> > linux/sched.h that is public to other subsystems.
>
> yeah. That's long overdue. We have include/linux/bpf_verifier.h
> that might work for this case too.
> Or even directly in verifier.c (if possible).

Ack. Directly in verifier.c would be best, but bpf_verifier.h works.
There is only one place outside verifier.c (i.e. btf_ctx_access() in
btf.c) that needs base_type, the rest are all in verifier.c. Anyway, I
am going to put these functions in bpf_verifer.h for now.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL
@ 2021-12-02 23:50 kernel test robot
  0 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-12-02 23:50 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 17367 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211130012948.380602-5-haoluo@google.com>
References: <20211130012948.380602-5-haoluo@google.com>
TO: Hao Luo <haoluo@google.com>

Hi Hao,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Hao-Luo/Introduce-composable-bpf-types/20211130-093143
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: x86_64-randconfig-c007-20211202 (https://download.01.org/0day-ci/archive/20211203/202112030712.qtXfXqLl-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 4b553297ef3ee4dc2119d5429adf3072e90fac38)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9e92c0a723fc173ac102b3bb27df479a01f32896
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hao-Luo/Introduce-composable-bpf-types/20211130-093143
        git checkout 9e92c0a723fc173ac102b3bb27df479a01f32896
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
   include/linux/rcupdate.h:390:48: note: expanded from macro '__rcu_dereference_check'
           typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
                                                         ^
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:2: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ^
   include/linux/compiler_types.h:323:2: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ^
   include/linux/compiler_types.h:307:2: note: expanded from macro '__compiletime_assert'
           do {                                                            \
           ^
   drivers/pci/p2pdma.c:536:11: note: Dereference of null pointer
           p2pdma = rcu_dereference(provider->p2pdma);
                    ^
   include/linux/rcupdate.h:597:28: note: expanded from macro 'rcu_dereference'
   #define rcu_dereference(p) rcu_dereference_check(p, 0)
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/rcupdate.h:529:2: note: expanded from macro 'rcu_dereference_check'
           __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu)
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/rcupdate.h:390:48: note: expanded from macro '__rcu_dereference_check'
           typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
                                                         ^~~~~~~~~~~~
   include/asm-generic/rwonce.h:50:2: note: expanded from macro 'READ_ONCE'
           __READ_ONCE(x);                                                 \
           ^~~~~~~~~~~~~~
   include/asm-generic/rwonce.h:44:24: note: expanded from macro '__READ_ONCE'
   #define __READ_ONCE(x)  (*(const volatile __unqual_scalar_typeof(x) *)&(x))
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   Suppressed 3 warnings (3 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   9 warnings generated.
   kernel/trace/trace.c:361:2: warning: Access to field 'next' results in a dereference of a null pointer [clang-analyzer-core.NullDereference]
           rcu_assign_pointer(*p, (*p)->next);
           ^
   include/linux/rcupdate.h:447:35: note: expanded from macro 'rcu_assign_pointer'
           uintptr_t _r_a_p__v = (uintptr_t)(v);                                 \
                                            ^
   kernel/trace/trace.c:404:2: note: Value assigned to 'ftrace_exports_list'
           mutex_lock(&ftrace_export_lock);
           ^
   include/linux/mutex.h:187:26: note: expanded from macro 'mutex_lock'
   #define mutex_lock(lock) mutex_lock_nested(lock, 0)
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/trace/trace.c:406:8: note: Calling 'rm_ftrace_export'
           ret = rm_ftrace_export(&ftrace_exports_list, export);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/trace/trace.c:379:8: note: Calling 'rm_trace_export'
           ret = rm_trace_export(list, export);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/trace/trace.c:354:17: note: Assuming the condition is false
           for (p = list; *p != NULL; p = &(*p)->next)
                          ^~~~~~~~~~
   kernel/trace/trace.c:354:17: note: Assuming pointer value is null
           for (p = list; *p != NULL; p = &(*p)->next)
                          ^~~~~~~~~~
   kernel/trace/trace.c:354:2: note: Loop condition is false. Execution continues on line 358
           for (p = list; *p != NULL; p = &(*p)->next)
           ^
   kernel/trace/trace.c:358:6: note: Assuming the condition is false
           if (*p != export)
               ^~~~~~~~~~~~
   kernel/trace/trace.c:358:2: note: Taking false branch
           if (*p != export)
           ^
   kernel/trace/trace.c:361:2: note: Access to field 'next' results in a dereference of a null pointer
           rcu_assign_pointer(*p, (*p)->next);
           ^
   include/linux/rcupdate.h:447:35: note: expanded from macro 'rcu_assign_pointer'
           uintptr_t _r_a_p__v = (uintptr_t)(v);                                 \
                                            ^~~
   kernel/trace/trace.c:2420:3: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
                   strcpy(comm, "<idle>");
                   ^~~~~~
   kernel/trace/trace.c:2420:3: note: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
                   strcpy(comm, "<idle>");
                   ^~~~~~
   kernel/trace/trace.c:2425:3: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
                   strcpy(comm, "<XXX>");
                   ^~~~~~
   kernel/trace/trace.c:2425:3: note: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
                   strcpy(comm, "<XXX>");
                   ^~~~~~
   kernel/trace/trace.c:2438:2: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
           strcpy(comm, "<...>");
           ^~~~~~
   kernel/trace/trace.c:2438:2: note: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
           strcpy(comm, "<...>");
           ^~~~~~
   kernel/trace/trace.c:3854:4: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
                           strcpy(iter->fmt, "%s");
                           ^~~~~~
   kernel/trace/trace.c:3854:4: note: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
                           strcpy(iter->fmt, "%s");
                           ^~~~~~
   Suppressed 4 warnings (3 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   13 warnings generated.
>> kernel/bpf/verifier.c:560:2: warning: Undefined or garbage value returned to caller [clang-analyzer-core.uninitialized.UndefReturn]
           return str[BPF_BASE_TYPE(type)];
           ^
   kernel/bpf/verifier.c:11029:17: note: Assuming the condition is false
           bool pop_log = !(env->log.level & BPF_LOG_LEVEL2);
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11037:2: note: Loop condition is true.  Entering loop body
           for (;;) {
           ^
   kernel/bpf/verifier.c:11043:7: note: Assuming 'insn_cnt' is > field 'insn_idx'
                   if (env->insn_idx >= insn_cnt) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11043:3: note: Taking false branch
                   if (env->insn_idx >= insn_cnt) {
                   ^
   kernel/bpf/verifier.c:11052:7: note: Assuming the condition is false
                   if (++env->insn_processed > BPF_COMPLEXITY_LIMIT_INSNS) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11052:3: note: Taking false branch
                   if (++env->insn_processed > BPF_COMPLEXITY_LIMIT_INSNS) {
                   ^
   kernel/bpf/verifier.c:11060:7: note: Assuming 'err' is >= 0
                   if (err < 0)
                       ^~~~~~~
   kernel/bpf/verifier.c:11060:3: note: Taking false branch
                   if (err < 0)
                   ^
   kernel/bpf/verifier.c:11062:7: note: Assuming 'err' is not equal to 1
                   if (err == 1) {
                       ^~~~~~~~
   kernel/bpf/verifier.c:11062:3: note: Taking false branch
                   if (err == 1) {
                   ^
   kernel/bpf/verifier.c:11076:7: note: Assuming the condition is false
                   if (signal_pending(current))
                       ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11076:3: note: Taking false branch
                   if (signal_pending(current))
                   ^
   kernel/bpf/verifier.c:11079:7: note: Assuming the condition is false
                   if (need_resched())
                       ^~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11079:3: note: Taking false branch
                   if (need_resched())
                   ^
   kernel/bpf/verifier.c:11082:7: note: Assuming the condition is false
                   if (env->log.level & BPF_LOG_LEVEL2 ||
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11082:7: note: Left side of '||' is false
   kernel/bpf/verifier.c:11083:8: note: Assuming the condition is false
                       (env->log.level & BPF_LOG_LEVEL && do_print_state)) {
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11083:39: note: Left side of '&&' is false
                       (env->log.level & BPF_LOG_LEVEL && do_print_state)) {
                                                       ^
   kernel/bpf/verifier.c:11095:3: note: Taking false branch
                   if (env->log.level & BPF_LOG_LEVEL) {
                   ^
   kernel/bpf/verifier.c:11107:7: note: Assuming the condition is false
                   if (bpf_prog_is_dev_bound(env->prog->aux)) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11107:3: note: Taking false branch
                   if (bpf_prog_is_dev_bound(env->prog->aux)) {
                   ^
   kernel/bpf/verifier.c:11118:7: note: Assuming 'class' is not equal to BPF_ALU
                   if (class == BPF_ALU || class == BPF_ALU64) {
                       ^~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11118:7: note: Left side of '||' is false
   kernel/bpf/verifier.c:11118:27: note: Assuming 'class' is equal to BPF_ALU64
                   if (class == BPF_ALU || class == BPF_ALU64) {
                                           ^~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:11118:3: note: Taking true branch
                   if (class == BPF_ALU || class == BPF_ALU64) {
                   ^
   kernel/bpf/verifier.c:11119:10: note: Calling 'check_alu_op'
                           err = check_alu_op(env, insn);
                                 ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:8174:6: note: Assuming 'opcode' is not equal to BPF_END
           if (opcode == BPF_END || opcode == BPF_NEG) {
               ^~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:8174:6: note: Left side of '||' is false
   kernel/bpf/verifier.c:8174:27: note: Assuming 'opcode' is not equal to BPF_NEG
           if (opcode == BPF_END || opcode == BPF_NEG) {
                                    ^~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:8174:2: note: Taking false branch
           if (opcode == BPF_END || opcode == BPF_NEG) {
           ^
   kernel/bpf/verifier.c:8207:13: note: Assuming 'opcode' is not equal to BPF_MOV
           } else if (opcode == BPF_MOV) {
                      ^~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:8207:9: note: Taking false branch
           } else if (opcode == BPF_MOV) {
                  ^
   kernel/bpf/verifier.c:8286:13: note: Assuming 'opcode' is <= BPF_END
           } else if (opcode > BPF_END) {
                      ^~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:8286:9: note: Taking false branch
           } else if (opcode > BPF_END) {
                  ^
   kernel/bpf/verifier.c:8292:7: note: Assuming the condition is false
                   if (BPF_SRC(insn->code) == BPF_X) {

vim +560 kernel/bpf/verifier.c

39491867ace594 Brendan Jackman    2021-03-04  531  
17a5267067f3c3 Alexei Starovoitov 2014-09-26  532  /* string representation of 'enum bpf_reg_type' */
9e92c0a723fc17 Hao Luo            2021-11-29  533  static const char * const reg_type_str(enum bpf_reg_type type)
9e92c0a723fc17 Hao Luo            2021-11-29  534  {
9e92c0a723fc17 Hao Luo            2021-11-29  535  	static const char * const str[] = {
17a5267067f3c3 Alexei Starovoitov 2014-09-26  536  		[NOT_INIT]		= "?",
f1174f77b50c94 Edward Cree        2017-08-07  537  		[SCALAR_VALUE]		= "inv",
17a5267067f3c3 Alexei Starovoitov 2014-09-26  538  		[PTR_TO_CTX]		= "ctx",
17a5267067f3c3 Alexei Starovoitov 2014-09-26  539  		[CONST_PTR_TO_MAP]	= "map_ptr",
17a5267067f3c3 Alexei Starovoitov 2014-09-26  540  		[PTR_TO_MAP_VALUE]	= "map_value",
17a5267067f3c3 Alexei Starovoitov 2014-09-26  541  		[PTR_TO_STACK]		= "fp",
969bf05eb3cedd Alexei Starovoitov 2016-05-05  542  		[PTR_TO_PACKET]		= "pkt",
de8f3a83b0a0fd Daniel Borkmann    2017-09-25  543  		[PTR_TO_PACKET_META]	= "pkt_meta",
969bf05eb3cedd Alexei Starovoitov 2016-05-05  544  		[PTR_TO_PACKET_END]	= "pkt_end",
d58e468b1112dc Petar Penkov       2018-09-14  545  		[PTR_TO_FLOW_KEYS]	= "flow_keys",
c64b7983288e63 Joe Stringer       2018-10-02  546  		[PTR_TO_SOCKET]		= "sock",
46f8bc92758c62 Martin KaFai Lau   2019-02-09  547  		[PTR_TO_SOCK_COMMON]	= "sock_common",
655a51e536c09d Martin KaFai Lau   2019-02-09  548  		[PTR_TO_TCP_SOCK]	= "tcp_sock",
9df1c28bb75217 Matt Mullins       2019-04-26  549  		[PTR_TO_TP_BUFFER]	= "tp_buffer",
fada7fdc83c0bf Jonathan Lemon     2019-06-06  550  		[PTR_TO_XDP_SOCK]	= "xdp_sock",
9e15db66136a14 Alexei Starovoitov 2019-10-15  551  		[PTR_TO_BTF_ID]		= "ptr_",
eaa6bcb71ef6ed Hao Luo            2020-09-29  552  		[PTR_TO_PERCPU_BTF_ID]	= "percpu_ptr_",
457f44363a8894 Andrii Nakryiko    2020-05-29  553  		[PTR_TO_MEM]		= "mem",
afbf21dce668ef Yonghong Song      2020-07-23  554  		[PTR_TO_RDONLY_BUF]	= "rdonly_buf",
afbf21dce668ef Yonghong Song      2020-07-23  555  		[PTR_TO_RDWR_BUF]	= "rdwr_buf",
69c087ba6225b5 Yonghong Song      2021-02-26  556  		[PTR_TO_FUNC]		= "func",
69c087ba6225b5 Yonghong Song      2021-02-26  557  		[PTR_TO_MAP_KEY]	= "map_key",
17a5267067f3c3 Alexei Starovoitov 2014-09-26  558  	};
17a5267067f3c3 Alexei Starovoitov 2014-09-26  559  
9e92c0a723fc17 Hao Luo            2021-11-29 @560  	return str[BPF_BASE_TYPE(type)];
9e92c0a723fc17 Hao Luo            2021-11-29  561  }
9e92c0a723fc17 Hao Luo            2021-11-29  562  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2021-12-03  0:14 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30  1:29 [RFC PATCH bpf-next v2 0/9] Introduce composable bpf types Hao Luo
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 1/9] bpf: Introduce composable reg, ret and arg types Hao Luo
2021-12-01 20:29   ` Alexei Starovoitov
2021-12-01 22:36     ` Hao Luo
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 2/9] bpf: Replace ARG_XXX_OR_NULL with ARG_XXX | PTR_MAYBE_NULL Hao Luo
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 3/9] bpf: Replace RET_XXX_OR_NULL with RET_XXX " Hao Luo
2021-11-30  2:59   ` kernel test robot
2021-11-30  3:40   ` kernel test robot
2021-11-30  3:40     ` kernel test robot
2021-12-01 20:30   ` Alexei Starovoitov
2021-12-01 22:40     ` Hao Luo
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX " Hao Luo
2021-11-30  3:30   ` kernel test robot
2021-11-30  4:21   ` kernel test robot
2021-11-30  4:21     ` kernel test robot
2021-11-30  4:31   ` kernel test robot
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 5/9] bpf: Introduce MEM_RDONLY flag Hao Luo
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 6/9] bpf: Convert PTR_TO_MEM_OR_NULL to composable types Hao Luo
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 7/9] bpf: Make per_cpu_ptr return rdonly PTR_TO_MEM Hao Luo
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 8/9] bpf: Add MEM_RDONLY for helper args that are pointers to rdonly mem Hao Luo
2021-12-01 20:34   ` Alexei Starovoitov
2021-12-01 22:21     ` Hao Luo
2021-12-02  3:53       ` Alexei Starovoitov
2021-12-02 18:42         ` Hao Luo
2021-12-02 21:13           ` Alexei Starovoitov
2021-12-03  0:14             ` Hao Luo
2021-11-30  1:29 ` [RFC PATCH bpf-next v2 9/9] bpf/selftests: Test PTR_TO_RDONLY_MEM Hao Luo
2021-12-02 23:50 [RFC PATCH bpf-next v2 4/9] bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.