[PATCH OLK-6.6 2/3] bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL"

4 Jan 2025

From: Kumar Kartikeya Dwivedi memxor@gmail.com
mainline inclusion
from mainline-v6.13-rc3
commit c00d738e1673ab801e1577e4e3c780ccf88b1a5b
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IBEDPP
CVE: CVE-2024-56702
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
This patch reverts commit
cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL"). The
patch was well-intended and meant to be as a stop-gap fixing branch
prediction when the pointer may actually be NULL at runtime. Eventually,
it was supposed to be replaced by an automated script or compiler pass
detecting possibly NULL arguments and marking them accordingly.
However, it caused two main issues observed for production programs and
failed to preserve backwards compatibility. First, programs relied on
the verifier not exploring == NULL branch when pointer is not NULL, thus
they started failing with a 'dereference of scalar' error.  Next,
allowing raw_tp arguments to be modified surfaced the warning in the
verifier that warns against reg->off when PTR_MAYBE_NULL is set.
More information, context, and discusson on both problems is available
in [0]. Overall, this approach had several shortcomings, and the fixes
would further complicate the verifier's logic, and the entire masking
scheme would have to be removed eventually anyway.
Hence, revert the patch in preparation of a better fix avoiding these
issues to replace this commit.
[0]: https://lore.kernel.org/bpf/20241206161053.809580-1-memxor@gmail.com
Reported-by: Manu Bretelle chantra@meta.com
Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
Signed-off-by: Kumar Kartikeya Dwivedi memxor@gmail.com
Link: https://lore.kernel.org/r/20241213221929.3495062-2-memxor@gmail.com
Signed-off-by: Alexei Starovoitov ast@kernel.org
Conflicts:
    kernel/bpf/btf.c
    include/linux/bpf.h
    kernel/bpf/verifier.c
    tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
[The conflicts were due to some minor issue]
Signed-off-by: Pu Lehui pulehui@huawei.com
---
 include/linux/bpf.h   |  6 ----
 kernel/bpf/btf.c      |  4 ---
 kernel/bpf/verifier.c | 77 +++++--------------------------------------
 3 files changed, 8 insertions(+), 79 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 7e7a2507a3e5..73ef8dda7ada 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3322,10 +3322,4 @@ static inline gfp_t bpf_memcg_flags(gfp_t flags)
    return flags;
 }
-static inline bool bpf_prog_is_raw_tp(const struct bpf_prog *prog)
-{
-	return prog->type == BPF_PROG_TYPE_TRACING &&
-	       prog->expected_attach_type == BPF_TRACE_RAW_TP;
-}
-
 #endif /* _LINUX_BPF_H */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index a54a7fdf99cd..7a82194ad51d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6076,10 +6076,6 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
    if (prog_args_trusted(prog))
    	info->reg_type |= PTR_TRUSTED;
-	/* Raw tracepoint arguments always get marked as maybe NULL */
-	if (bpf_prog_is_raw_tp(prog))
-		info->reg_type |= PTR_MAYBE_NULL;
-
    if (tgt_prog) {
    	enum bpf_prog_type tgt_type;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fe0c8bf97663..8fa7b37152c0 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -495,25 +495,6 @@ static struct btf_record *reg_btf_record(const struct bpf_reg_state *reg)
    return rec;
 }
-static bool mask_raw_tp_reg_cond(const struct bpf_verifier_env *env, struct bpf_reg_state *reg) {
-	return reg->type == (PTR_TO_BTF_ID | PTR_TRUSTED | PTR_MAYBE_NULL) &&
-	       bpf_prog_is_raw_tp(env->prog) && !reg->ref_obj_id;
-}
-
-static bool mask_raw_tp_reg(const struct bpf_verifier_env *env, struct bpf_reg_state *reg)
-{
-	if (!mask_raw_tp_reg_cond(env, reg))
-		return false;
-	reg->type &= ~PTR_MAYBE_NULL;
-	return true;
-}
-
-static void unmask_raw_tp_reg(struct bpf_reg_state *reg, bool result)
-{
-	if (result)
-		reg->type |= PTR_MAYBE_NULL;
-}
-
 static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
 {
    struct bpf_func_info_aux *aux = env->prog->aux->func_info_aux;
@@ -6477,7 +6458,6 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
    const char *field_name = NULL;
    enum bpf_type_flag flag = 0;
    u32 btf_id = 0;
-	bool mask;
    int ret;
if (!env->allow_ptr_leaks) {
@@ -6549,21 +6529,7 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
if (ret < 0)
    	return ret;
-	/* For raw_tp progs, we allow dereference of PTR_MAYBE_NULL
-	 * trusted PTR_TO_BTF_ID, these are the ones that are possibly
-	 * arguments to the raw_tp. Since internal checks in for trusted
-	 * reg in check_ptr_to_btf_access would consider PTR_MAYBE_NULL
-	 * modifier as problematic, mask it out temporarily for the
-	 * check. Don't apply this to pointers with ref_obj_id > 0, as
-	 * those won't be raw_tp args.
-	 *
-	 * We may end up applying this relaxation to other trusted
-	 * PTR_TO_BTF_ID with maybe null flag, since we cannot
-	 * distinguish PTR_MAYBE_NULL tagged for arguments vs normal
-	 * tagging, but that should expand allowed behavior, and not
-	 * cause regression for existing behavior.
-	 */
-	mask = mask_raw_tp_reg(env, reg);
+
    if (ret != PTR_TO_BTF_ID) {
    	/* just mark; */
@@ -6624,13 +6590,8 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
    	clear_trusted_flags(&flag);
    }
-	if (atype == BPF_READ && value_regno >= 0) {
+	if (atype == BPF_READ && value_regno >= 0)
    	mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, flag);
-		/* We've assigned a new type to regno, so don't undo masking. */
-		if (regno == value_regno)
-			mask = false;
-	}
-	unmask_raw_tp_reg(reg, mask);
return 0;
 }
@@ -7002,7 +6963,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
    	if (!err && t == BPF_READ && value_regno >= 0)
    		mark_reg_unknown(env, regs, value_regno);
    } else if (base_type(reg->type) == PTR_TO_BTF_ID &&
-		   (mask_raw_tp_reg_cond(env, reg) || !type_may_be_null(reg->type))) {
+		   !type_may_be_null(reg->type)) {
    	err = check_ptr_to_btf_access(env, regs, regno, off, size, t,
    				      value_regno);
    } else if (reg->type == CONST_PTR_TO_MAP) {
@@ -8554,7 +8515,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
    enum bpf_reg_type type = reg->type;
    u32 *arg_btf_id = NULL;
    int err = 0;
-	bool mask;
if (arg_type == ARG_DONTCARE)
    	return 0;
@@ -8595,11 +8555,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
        base_type(arg_type) == ARG_PTR_TO_SPIN_LOCK)
    	arg_btf_id = fn->arg_btf_id[arg];
-	mask = mask_raw_tp_reg(env, reg);
    err = check_reg_type(env, regno, arg_type, arg_btf_id, meta);
+	if (err)
+		return err;
-	err = err ?: check_func_arg_reg_off(env, reg, regno, arg_type);
-	unmask_raw_tp_reg(reg, mask);
+	err = check_func_arg_reg_off(env, reg, regno, arg_type);
    if (err)
    	return err;
@@ -11406,7 +11366,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
    	enum bpf_arg_type arg_type = ARG_DONTCARE;
    	u32 regno = i + 1, ref_id, type_size;
    	bool is_ret_buf_sz = false;
-		bool mask = false;
    	int kf_arg_type;
t = btf_type_skip_modifiers(btf, args[i].type, NULL);
@@ -11465,14 +11424,11 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
    		return -EINVAL;
    	}
-		mask = mask_raw_tp_reg(env, reg);
    	if ((is_kfunc_trusted_args(meta) || is_kfunc_rcu(meta)) &&
    	    (register_is_null(reg) || type_may_be_null(reg->type))) {
    		verbose(env, "Possibly NULL pointer passed to trusted arg%d\n", i);
-			unmask_raw_tp_reg(reg, mask);
    		return -EACCES;
    	}
-		unmask_raw_tp_reg(reg, mask);
if (reg->ref_obj_id) {
    		if (is_kfunc_release(meta) && meta->ref_obj_id) {
@@ -11499,24 +11455,17 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
    		if (!is_kfunc_trusted_args(meta) && !is_kfunc_rcu(meta))
    			break;
-			/* Allow passing maybe NULL raw_tp arguments to
-			 * kfuncs for compatibility. Don't apply this to
-			 * arguments with ref_obj_id > 0.
-			 */
-			mask = mask_raw_tp_reg(env, reg);
    		if (!is_trusted_reg(reg)) {
    			if (!is_kfunc_rcu(meta)) {
    				verbose(env, "R%d must be referenced or trusted\n", regno);
-					unmask_raw_tp_reg(reg, mask);
    				return -EINVAL;
    			}
    			if (!is_rcu_reg(reg)) {
    				verbose(env, "R%d must be a rcu pointer\n", regno);
-					unmask_raw_tp_reg(reg, mask);
    				return -EINVAL;
    			}
    		}
-			unmask_raw_tp_reg(reg, mask);
+
    		fallthrough;
    	case KF_ARG_PTR_TO_CTX:
    		/* Trusted arguments have the same offset checks as release arguments */
@@ -11541,9 +11490,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
if (is_kfunc_release(meta) && reg->ref_obj_id)
    		arg_type |= OBJ_RELEASE;
-		mask = mask_raw_tp_reg(env, reg);
    	ret = check_func_arg_reg_off(env, reg, regno, arg_type);
-		unmask_raw_tp_reg(reg, mask);
    	if (ret < 0)
    		return ret;
@@ -11704,7 +11651,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
    			return ret;
    		break;
    	case KF_ARG_PTR_TO_BTF_ID:
-			mask = mask_raw_tp_reg(env, reg);
    		/* Only base_type is checked, further checks are done here */
    		if ((base_type(reg->type) != PTR_TO_BTF_ID ||
    		     (bpf_type_has_unsafe_modifiers(reg->type) && !is_rcu_reg(reg))) &&
@@ -11713,11 +11659,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
    			verbose(env, "expected %s or socket\n",
    				reg_type_str(env, base_type(reg->type) |
    						  (type_flag(reg->type) & BPF_REG_TRUSTED_MODIFIERS)));
-				unmask_raw_tp_reg(reg, mask);
    			return -EINVAL;
    		}
    		ret = process_kf_arg_ptr_to_btf_id(env, reg, ref_t, ref_tname, ref_id, meta, i);
-			unmask_raw_tp_reg(reg, mask);
    		if (ret < 0)
    			return ret;
    		break;
@@ -12604,7 +12548,7 @@ static int sanitize_check_bounds(struct bpf_verifier_env *env,
  */
 static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
    			   struct bpf_insn *insn,
-				   struct bpf_reg_state *ptr_reg,
+				   const struct bpf_reg_state *ptr_reg,
    			   const struct bpf_reg_state *off_reg)
 {
    struct bpf_verifier_state *vstate = env->cur_state;
@@ -12618,7 +12562,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
    struct bpf_sanitize_info info = {};
    u8 opcode = BPF_OP(insn->code);
    u32 dst = insn->dst_reg;
-	bool mask;
    int ret;
dst_reg = &regs[dst];
@@ -12645,14 +12588,11 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
    	return -EACCES;
    }
-	mask = mask_raw_tp_reg(env, ptr_reg);
    if (ptr_reg->type & PTR_MAYBE_NULL) {
    	verbose(env, "R%d pointer arithmetic on %s prohibited, null-check it first\n",
    		dst, reg_type_str(env, ptr_reg->type));
-		unmask_raw_tp_reg(ptr_reg, mask);
    	return -EACCES;
    }
-	unmask_raw_tp_reg(ptr_reg, mask);
switch (base_type(ptr_reg->type)) {
    case PTR_TO_FLOW_KEYS:
@@ -18515,7 +18455,6 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
    	 * for this case.
    	 */
    	case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
-		case PTR_TO_BTF_ID | PTR_TRUSTED | PTR_MAYBE_NULL:
    		if (type == BPF_READ) {
    			if (BPF_MODE(insn->code) == BPF_MEM)
    				insn->code = BPF_LDX | BPF_PROBE_MEM |
-- 
2.34.1


    

2025

2024

2023

2022

2021

2020

2019

[PATCH OLK-6.6 2/3] bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL"