Handle possible NULL trusted raw_tp arguments

Arguments to a raw tracepoint are tagged as trusted, which carries the semantics that the pointer will be non-NULL. However, in certain cases, a raw tracepoint argument may end up being NULL. More context about this issue is available in [0].

Thus, there is a discrepancy between the reality, that raw_tp arguments can actually be NULL, and the verifier's knowledge, that they are never NULL, causing explicit NULL checks to be deleted, and accesses to such pointers potentially crashing the kernel.

To fix this, mark raw_tp arguments as PTR_MAYBE_NULL, and then special case the dereference and pointer arithmetic to permit it, and allow passing them into helpers/kfuncs; these exceptions are made for raw_tp programs only. Ensure that we don't do this when ref_obj_id > 0, as in that case this is an acquired object and doesn't need such adjustment.

The reason we do mask_raw_tp_trusted_reg logic is because other will recheck in places whether the register is a trusted_reg, and then consider our register as untrusted when detecting the presence of the PTR_MAYBE_NULL flag.

To allow safe dereference, we enable PROBE_MEM marking when we see loads into trusted pointers with PTR_MAYBE_NULL.

While trusted raw_tp arguments can also be passed into helpers or kfuncs where such broken assumption may cause issues, a future patch set will tackle their case separately, as PTR_TO_BTF_ID (without PTR_TRUSTED) can already be passed into helpers and causes similar problems. Thus, they are left alone for now.

It is possible that these checks also permit passing non-raw_tp args that are trusted PTR_TO_BTF_ID with null marking. In such a case, allowing dereference when pointer is NULL expands allowed behavior, so won't regress existing programs, and the case of passing these into helpers is the same as above and will be dealt with later.

Also update the failure case in tp_btf_nullable selftest to capture the new behavior, as the verifier will no longer cause an error when directly dereference a raw tracepoint argument marked as __nullable.

[0]: https://lore.kernel.org/bpf/[email protected]

Reviewed-by Jiri Olsa <[email protected]>

Reported-by Juri Lelli <[email protected]>

Tested-by Juri Lelli <[email protected]>

Fixes 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")

Signed-off-by Kumar Kartikeya Dwivedi <[email protected]>

---
 include/linux/bpf.h                           |  6 ++
 kernel/bpf/btf.c                              |  5 +-
 kernel/bpf/verifier.c                         | 79 +++++++++++++++++--
 .../bpf/progs/test_tp_btf_nullable.c          |  6 +-
 4 files changed, 87 insertions(+), 9 deletions(-)

kernel/bpf/verifier.c +1 -0

                        return -EACCES;
                }
+               unmask_raw_tp_reg(reg, mask);
 
                if (reg->ref_obj_id) {
                        if (is_kfunc_release(meta) && meta->ref_obj_id) {

@@ -12128,16 +12175,24 @@ 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;

kernel/bpf/verifier.c +5 -0

                                        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:
                case KF_ARG_PTR_TO_DYNPTR:

@@ -12160,7 +12215,9 @@ 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;
 

@@ -12337,6 +12394,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
                        ref_tname = btf_name_by_offset(btf, ref_t->name_off);
                        fallthrough;
                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))) &&

kernel/bpf/verifier.c +3 -1

                                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;

@@ -13320,7 +13380,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,
-                                  const struct bpf_reg_state *ptr_reg,
+                                  struct bpf_reg_state *ptr_reg,
                                   const struct bpf_reg_state *off_reg)
 {
        struct bpf_verifier_state *vstate = env->cur_state;

@@ -13334,6 +13394,7 @@ 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];

kernel/bpf/verifier.c +2 -0

                return -EACCES;
        }
+       unmask_raw_tp_reg(ptr_reg, mask);
 
        switch (base_type(ptr_reg->type)) {
        case PTR_TO_CTX:

@@ -19866,6 +19930,7 @@ 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 |

tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c +5 -2

index bba3e37f749b..5aaf2b065f86 100644
@@ -7,7 +7,11 @@
 #include "bpf_misc.h"
 
 SEC("tp_btf/bpf_testmod_test_nullable_bare")
-__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+/* This used to be a failure test, but raw_tp nullable arguments can now
+ * directly be dereferenced, whether they have nullable annotation or not,
+ * and don't need to be explicitly checked.
+ */
+__success
 int BPF_PROG(handle_tp_btf_nullable_bare1, struct bpf_testmod_test_read_ctx *nullable_ctx)
 {
        return nullable_ctx->len;
--

2.43.5

Quoted 55 lines · Kumar Kartikeya Dwivedi

Arguments to a raw tracepoint are tagged as trusted, which carries the semantics that the pointer will be non-NULL. However, in certain cases, a raw tracepoint argument may end up being NULL. More context about this issue is available in [0].

Thus, there is a discrepancy between the reality, that raw_tp arguments can actually be NULL, and the verifier's knowledge, that they are never NULL, causing explicit NULL checks to be deleted, and accesses to such pointers potentially crashing the kernel.

To fix this, mark raw_tp arguments as PTR_MAYBE_NULL, and then special case the dereference and pointer arithmetic to permit it, and allow passing them into helpers/kfuncs; these exceptions are made for raw_tp programs only. Ensure that we don't do this when ref_obj_id > 0, as in that case this is an acquired object and doesn't need such adjustment.

The reason we do mask_raw_tp_trusted_reg logic is because other will recheck in places whether the register is a trusted_reg, and then consider our register as untrusted when detecting the presence of the PTR_MAYBE_NULL flag.

To allow safe dereference, we enable PROBE_MEM marking when we see loads into trusted pointers with PTR_MAYBE_NULL.

While trusted raw_tp arguments can also be passed into helpers or kfuncs where such broken assumption may cause issues, a future patch set will tackle their case separately, as PTR_TO_BTF_ID (without PTR_TRUSTED) can already be passed into helpers and causes similar problems. Thus, they are left alone for now.

It is possible that these checks also permit passing non-raw_tp args that are trusted PTR_TO_BTF_ID with null marking. In such a case, allowing dereference when pointer is NULL expands allowed behavior, so won't regress existing programs, and the case of passing these into helpers is the same as above and will be dealt with later.

Also update the failure case in tp_btf_nullable selftest to capture the new behavior, as the verifier will no longer cause an error when directly dereference a raw tracepoint argument marked as __nullable.

[0]: https://lore.kernel.org/bpf/[email protected]

Reviewed-by Jiri Olsa <[email protected]>

Reported-by Juri Lelli <[email protected]>

Tested-by Juri Lelli <[email protected]>

Fixes 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")

Signed-off-by Kumar Kartikeya Dwivedi <[email protected]>

---
 include/linux/bpf.h                           |  6 ++
 kernel/bpf/btf.c                              |  5 +-
 kernel/bpf/verifier.c                         | 79 +++++++++++++++++--
 .../bpf/progs/test_tp_btf_nullable.c          |  6 +-
 4 files changed, 87 insertions(+), 9 deletions(-)

[...]

kernel/bpf/verifier.c static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ · line 12117

Quoted 10 lines

kernel/bpf/verifier.c +2 -0

@@ -12065,12 +12109,15 @@ 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)) &&
                        !is_kfunc_arg_nullable(meta->btf, &args[i])) {
                        verbose(env, "Possibly NULL pointer passed to trusted arg%d\n", i);
+                       unmask_raw_tp_reg(reg, mask);

Kumar,

Do we really need this unmask? We are already erroring out, restoring reg->type is probably not very important at this point?

[...]

Quoted 73 lines · Andrii Nakryiko

Quoted 55 lines · Kumar Kartikeya Dwivedi

Arguments to a raw tracepoint are tagged as trusted, which carries the semantics that the pointer will be non-NULL. However, in certain cases, a raw tracepoint argument may end up being NULL. More context about this issue is available in [0].

Thus, there is a discrepancy between the reality, that raw_tp arguments can actually be NULL, and the verifier's knowledge, that they are never NULL, causing explicit NULL checks to be deleted, and accesses to such pointers potentially crashing the kernel.

To fix this, mark raw_tp arguments as PTR_MAYBE_NULL, and then special case the dereference and pointer arithmetic to permit it, and allow passing them into helpers/kfuncs; these exceptions are made for raw_tp programs only. Ensure that we don't do this when ref_obj_id > 0, as in that case this is an acquired object and doesn't need such adjustment.

The reason we do mask_raw_tp_trusted_reg logic is because other will recheck in places whether the register is a trusted_reg, and then consider our register as untrusted when detecting the presence of the PTR_MAYBE_NULL flag.

To allow safe dereference, we enable PROBE_MEM marking when we see loads into trusted pointers with PTR_MAYBE_NULL.

While trusted raw_tp arguments can also be passed into helpers or kfuncs where such broken assumption may cause issues, a future patch set will tackle their case separately, as PTR_TO_BTF_ID (without PTR_TRUSTED) can already be passed into helpers and causes similar problems. Thus, they are left alone for now.

It is possible that these checks also permit passing non-raw_tp args that are trusted PTR_TO_BTF_ID with null marking. In such a case, allowing dereference when pointer is NULL expands allowed behavior, so won't regress existing programs, and the case of passing these into helpers is the same as above and will be dealt with later.

Also update the failure case in tp_btf_nullable selftest to capture the new behavior, as the verifier will no longer cause an error when directly dereference a raw tracepoint argument marked as __nullable.

[0]: https://lore.kernel.org/bpf/[email protected]

Reviewed-by Jiri Olsa <[email protected]>

Reported-by Juri Lelli <[email protected]>

Tested-by Juri Lelli <[email protected]>

Fixes 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")

Signed-off-by Kumar Kartikeya Dwivedi <[email protected]>

---
 include/linux/bpf.h                           |  6 ++
 kernel/bpf/btf.c                              |  5 +-
 kernel/bpf/verifier.c                         | 79 +++++++++++++++++--
 .../bpf/progs/test_tp_btf_nullable.c          |  6 +-
 4 files changed, 87 insertions(+), 9 deletions(-)

[...]

kernel/bpf/verifier.c static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ · line 12117

Quoted 10 lines

kernel/bpf/verifier.c +2 -0

@@ -12065,12 +12109,15 @@ 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)) &&
                        !is_kfunc_arg_nullable(meta->btf, &args[i])) {
                        verbose(env, "Possibly NULL pointer passed to trusted arg%d\n", i);
+                       unmask_raw_tp_reg(reg, mask);

Kumar,

Quoted 3 lines

Do we really need this unmask? We are already erroring out, restoring reg->type is probably not very important at this point?

Hello Andrii,
The reason I undid the masking was to ensure if the register type is printed for some reason,
it stays consistent and the masking isn't visible, but I guess the verifier state is printed _before_ an instruction is symbolically executed so it's not helping with anything.

I can send a follow up to remove the additional unmasking steps.