* [PATCH bpf v1 0/4] Explicit raw_tp NULL arguments
@ 2024-12-11 2:01 Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 1/4] bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL" Kumar Kartikeya Dwivedi
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2024-12-11 2:01 UTC (permalink / raw)
To: bpf
Cc: kkd, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, Manu Bretelle, Jiri Olsa,
Juri Lelli, kernel-team
This set reverts the raw_tp masking hack introduced in commit
cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
and replaces it wwith an explicit list of tracepoints and their
arguments which need to be annotated as PTR_MAYBE_NULL. More context
on the fallout caused by the masking fix and subsequent discussions can
be found in [0].
The set begins by reverting the fix and its associated selftest,
then introduces a new method of defining tracepoints with NULL
argument(s), and adds a script to autogenerate tests for all such
tracepoints. For tracepoints that are not available due to missing
CONFIG_ options, the testing is skipped by commenting them out.
However, to expand coverage for different cases, some additional config
options are introduced which do not introduce too many dependencies.
Kumar Kartikeya Dwivedi (4):
bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL"
selftests/bpf: Revert "selftests/bpf: Add tests for raw_tp null
handling"
bpf: Augment raw_tp arguments with PTR_MAYBE_NULL
selftests/bpf: Add autogenerated tests for raw_tp NULL args
include/linux/bpf.h | 6 -
kernel/bpf/btf.c | 134 +++++-
kernel/bpf/verifier.c | 79 +---
.../bpf/bpf_testmod/bpf_testmod-events.h | 8 -
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 2 -
tools/testing/selftests/bpf/config | 5 +
.../testing/selftests/bpf/gen_raw_tp_null.py | 58 +++
.../testing/selftests/bpf/gen_raw_tp_null.sh | 3 +
.../selftests/bpf/prog_tests/raw_tp_null.c | 19 +-
.../testing/selftests/bpf/progs/raw_tp_null.c | 431 +++++++++++++++++-
.../selftests/bpf/progs/raw_tp_scalar.c | 24 +
.../bpf/progs/test_tp_btf_nullable.c | 6 +-
12 files changed, 639 insertions(+), 136 deletions(-)
create mode 100755 tools/testing/selftests/bpf/gen_raw_tp_null.py
create mode 100755 tools/testing/selftests/bpf/gen_raw_tp_null.sh
create mode 100644 tools/testing/selftests/bpf/progs/raw_tp_scalar.c
base-commit: 7d0d673627e20cfa3b21a829a896ce03b58a4f1c
--
2.43.5
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf v1 1/4] bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL"
2024-12-11 2:01 [PATCH bpf v1 0/4] Explicit raw_tp NULL arguments Kumar Kartikeya Dwivedi
@ 2024-12-11 2:01 ` Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 2/4] selftests/bpf: Revert "selftests/bpf: Add tests for raw_tp null handling" Kumar Kartikeya Dwivedi
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2024-12-11 2:01 UTC (permalink / raw)
To: bpf
Cc: kkd, Manu Bretelle, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Martin KaFai Lau, Eduard Zingerman, Jiri Olsa,
Juri Lelli, kernel-team
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 was 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.
[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>
---
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, 9 insertions(+), 87 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 805040813f5d..6e63dd3443b9 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3514,10 +3514,4 @@ static inline bool bpf_is_subprog(const struct bpf_prog *prog)
return prog->aux->func_idx != 0;
}
-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 e7a59e6462a9..ed3219da7181 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6588,10 +6588,7 @@ 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;
- else if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
+ if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
info->reg_type |= PTR_MAYBE_NULL;
if (tgt_prog) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c2e5d0e6e3d0..aa110a01317f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -420,25 +420,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;
@@ -6801,7 +6782,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) {
@@ -6873,21 +6853,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; */
@@ -6948,13 +6914,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;
}
@@ -7329,7 +7290,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) {
@@ -9032,7 +8993,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;
@@ -9073,11 +9033,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;
@@ -9872,17 +9832,14 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
return ret;
} else if (base_type(arg->arg_type) == ARG_PTR_TO_BTF_ID) {
struct bpf_call_arg_meta meta;
- bool mask;
int err;
if (register_is_null(reg) && type_may_be_null(arg->arg_type))
continue;
memset(&meta, 0, sizeof(meta)); /* leave func_id as zero */
- mask = mask_raw_tp_reg(env, reg);
err = check_reg_type(env, regno, arg->arg_type, &arg->btf_id, &meta);
err = err ?: check_func_arg_reg_off(env, reg, regno, arg->arg_type);
- unmask_raw_tp_reg(reg, mask);
if (err)
return err;
} else {
@@ -12205,7 +12162,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);
@@ -12264,15 +12220,12 @@ 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);
return -EACCES;
}
- unmask_raw_tp_reg(reg, mask);
if (reg->ref_obj_id) {
if (is_kfunc_release(meta) && meta->ref_obj_id) {
@@ -12330,24 +12283,16 @@ 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:
case KF_ARG_PTR_TO_DYNPTR:
@@ -12370,9 +12315,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;
@@ -12549,7 +12492,6 @@ 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))) &&
@@ -12558,11 +12500,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;
@@ -13535,7 +13475,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;
@@ -13549,7 +13489,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 = ®s[dst];
@@ -13576,14 +13515,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_CTX:
@@ -20126,7 +20062,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 |
diff --git a/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c b/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
index 5aaf2b065f86..bba3e37f749b 100644
--- a/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
+++ b/tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c
@@ -7,11 +7,7 @@
#include "bpf_misc.h"
SEC("tp_btf/bpf_testmod_test_nullable_bare")
-/* 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
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
int BPF_PROG(handle_tp_btf_nullable_bare1, struct bpf_testmod_test_read_ctx *nullable_ctx)
{
return nullable_ctx->len;
--
2.43.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf v1 2/4] selftests/bpf: Revert "selftests/bpf: Add tests for raw_tp null handling"
2024-12-11 2:01 [PATCH bpf v1 0/4] Explicit raw_tp NULL arguments Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 1/4] bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL" Kumar Kartikeya Dwivedi
@ 2024-12-11 2:01 ` Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 4/4] selftests/bpf: Add autogenerated tests for raw_tp NULL args Kumar Kartikeya Dwivedi
3 siblings, 0 replies; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2024-12-11 2:01 UTC (permalink / raw)
To: bpf
Cc: kkd, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, Manu Bretelle, Jiri Olsa,
Juri Lelli, kernel-team
This patch reverts
commit d798ce3f4cab ("selftests/bpf: Add tests for raw_tp null handling")
since it's parent
commit cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
is also being reverted.
Fixes: d798ce3f4cab ("selftests/bpf: Add tests for raw_tp null handling")
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
.../bpf/bpf_testmod/bpf_testmod-events.h | 8 -----
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 2 --
.../selftests/bpf/prog_tests/raw_tp_null.c | 25 ---------------
.../testing/selftests/bpf/progs/raw_tp_null.c | 32 -------------------
4 files changed, 67 deletions(-)
delete mode 100644 tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
delete mode 100644 tools/testing/selftests/bpf/progs/raw_tp_null.c
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
index aeef86b3da74..6c3b4d4f173a 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
@@ -40,14 +40,6 @@ DECLARE_TRACE(bpf_testmod_test_nullable_bare,
TP_ARGS(ctx__nullable)
);
-struct sk_buff;
-
-DECLARE_TRACE(bpf_testmod_test_raw_tp_null,
- TP_PROTO(struct sk_buff *skb),
- TP_ARGS(skb)
-);
-
-
#undef BPF_TESTMOD_DECLARE_TRACE
#ifdef DECLARE_TRACE_WRITABLE
#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index cc9dde507aba..c64dd001a4ed 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -413,8 +413,6 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj,
(void)bpf_testmod_test_arg_ptr_to_struct(&struct_arg1_2);
- (void)trace_bpf_testmod_test_raw_tp_null(NULL);
-
bpf_testmod_test_struct_ops3();
struct_arg3 = kmalloc((sizeof(struct bpf_testmod_struct_arg_3) +
diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
deleted file mode 100644
index 6fa19449297e..000000000000
--- a/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
+++ /dev/null
@@ -1,25 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
-
-#include <test_progs.h>
-#include "raw_tp_null.skel.h"
-
-void test_raw_tp_null(void)
-{
- struct raw_tp_null *skel;
-
- skel = raw_tp_null__open_and_load();
- if (!ASSERT_OK_PTR(skel, "raw_tp_null__open_and_load"))
- return;
-
- skel->bss->tid = sys_gettid();
-
- if (!ASSERT_OK(raw_tp_null__attach(skel), "raw_tp_null__attach"))
- goto end;
-
- ASSERT_OK(trigger_module_test_read(2), "trigger testmod read");
- ASSERT_EQ(skel->bss->i, 3, "invocations");
-
-end:
- raw_tp_null__destroy(skel);
-}
diff --git a/tools/testing/selftests/bpf/progs/raw_tp_null.c b/tools/testing/selftests/bpf/progs/raw_tp_null.c
deleted file mode 100644
index 457f34c151e3..000000000000
--- a/tools/testing/selftests/bpf/progs/raw_tp_null.c
+++ /dev/null
@@ -1,32 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
-
-#include <vmlinux.h>
-#include <bpf/bpf_tracing.h>
-
-char _license[] SEC("license") = "GPL";
-
-int tid;
-int i;
-
-SEC("tp_btf/bpf_testmod_test_raw_tp_null")
-int BPF_PROG(test_raw_tp_null, struct sk_buff *skb)
-{
- struct task_struct *task = bpf_get_current_task_btf();
-
- if (task->pid != tid)
- return 0;
-
- i = i + skb->mark + 1;
- /* The compiler may move the NULL check before this deref, which causes
- * the load to fail as deref of scalar. Prevent that by using a barrier.
- */
- barrier();
- /* If dead code elimination kicks in, the increment below will
- * be removed. For raw_tp programs, we mark input arguments as
- * PTR_MAYBE_NULL, so branch prediction should never kick in.
- */
- if (!skb)
- i += 2;
- return 0;
-}
--
2.43.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL
2024-12-11 2:01 [PATCH bpf v1 0/4] Explicit raw_tp NULL arguments Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 1/4] bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL" Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 2/4] selftests/bpf: Revert "selftests/bpf: Add tests for raw_tp null handling" Kumar Kartikeya Dwivedi
@ 2024-12-11 2:01 ` Kumar Kartikeya Dwivedi
2024-12-11 12:24 ` Jiri Olsa
` (3 more replies)
2024-12-11 2:01 ` [PATCH bpf v1 4/4] selftests/bpf: Add autogenerated tests for raw_tp NULL args Kumar Kartikeya Dwivedi
3 siblings, 4 replies; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2024-12-11 2:01 UTC (permalink / raw)
To: bpf
Cc: kkd, Juri Lelli, Manu Bretelle, Jiri Olsa, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau,
Eduard Zingerman, kernel-team
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.
A previous attempt [1], i.e. the second fixed commit, was made to
simulate symbolic execution as if in most accesses, the argument is a
non-NULL raw_tp, except for conditional jumps. This tried to suppress
branch prediction while preserving compatibility, but surfaced issues
with production programs that were difficult to solve without increasing
verifier complexity. A more complete discussion of issues and fixes is
available at [2].
Fix this by maintaining an explicit, incomplete list of tracepoints
where the arguments are known to be NULL, and mark the positional
arguments as PTR_MAYBE_NULL. Additionally, capture the tracepoints where
arguments are known to be PTR_ERR, and mark these arguments as scalar
values to prevent potential dereference.
In the future, an automated pass will be used to produce such a list, or
insert __nullable annotations automatically for tracepoints. Anyhow,
this is an attempt to close the gap until the automation lands, and
reflets the current best known list according to Jiri's analysis in [3].
[0]: https://lore.kernel.org/bpf/ZrCZS6nisraEqehw@jlelli-thinkpadt14gen4.remote.csb
[1]: https://lore.kernel.org/all/20241104171959.2938862-1-memxor@gmail.com
[2]: https://lore.kernel.org/bpf/20241206161053.809580-1-memxor@gmail.com
[3]: https://lore.kernel.org/bpf/Z1d-qbCdtJqg6Er4@krava
Reported-by: Juri Lelli <juri.lelli@redhat.com> # original bug
Reported-by: Manu Bretelle <chantra@meta.com> # bugs in masking fix
Fixes: 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")
Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
Co-developed-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
kernel/bpf/btf.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index ed3219da7181..cb72cbf04d12 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6439,6 +6439,96 @@ int btf_ctx_arg_offset(const struct btf *btf, const struct btf_type *func_proto,
return off;
}
+struct bpf_raw_tp_null_args {
+ const char *func;
+ u64 mask;
+};
+
+#define RAW_TP_NULL_ARGS(str, arg) { .func = "btf_trace_" #str, .mask = (arg) }
+/* Use 1-based indexing for argno */
+#define NULL_ARG(argno) (1 << (argno))
+
+struct bpf_raw_tp_null_args raw_tp_null_args[] = {
+ /* sched */
+ RAW_TP_NULL_ARGS(sched_pi_setprio, NULL_ARG(2)),
+ /* ... from sched_numa_pair_template event class */
+ RAW_TP_NULL_ARGS(sched_stick_numa, NULL_ARG(3)),
+ RAW_TP_NULL_ARGS(sched_swap_numa, NULL_ARG(3)),
+ /* afs */
+ RAW_TP_NULL_ARGS(afs_make_fs_call, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(afs_make_fs_calli, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(afs_make_fs_call1, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(afs_make_fs_call2, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(afs_protocol_error, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(afs_flock_ev, NULL_ARG(2)),
+ /* cachefiles */
+ RAW_TP_NULL_ARGS(cachefiles_lookup, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_unlink, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_rename, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_prep_read, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_mark_active, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_mark_failed, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_mark_inactive, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_vfs_error, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_io_error, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_ondemand_open, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_ondemand_copen, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_ondemand_close, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_ondemand_read, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_ondemand_cread, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_ondemand_fd_write, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(cachefiles_ondemand_fd_release, NULL_ARG(1)),
+ /* ext4, from ext4__mballoc event class */
+ RAW_TP_NULL_ARGS(ext4_mballoc_discard, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(ext4_mballoc_free, NULL_ARG(2)),
+ /* fib */
+ RAW_TP_NULL_ARGS(fib_table_lookup, NULL_ARG(3)),
+ /* filelock */
+ /* ... from filelock_lock event class */
+ RAW_TP_NULL_ARGS(posix_lock_inode, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(fcntl_setlk, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(locks_remove_posix, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(flock_lock_inode, NULL_ARG(2)),
+ /* ... from filelock_lease event class */
+ RAW_TP_NULL_ARGS(break_lease_noblock, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(break_lease_block, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(break_lease_unblock, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(generic_delete_lease, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(time_out_leases, NULL_ARG(2)),
+ /* host1x */
+ RAW_TP_NULL_ARGS(host1x_cdma_push_gather, NULL_ARG(5)),
+ /* huge_memory */
+ RAW_TP_NULL_ARGS(mm_khugepaged_scan_pmd, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(mm_collapse_huge_page_isolate, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(mm_khugepaged_scan_file, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(mm_khugepaged_collapse_file, NULL_ARG(2)),
+ /* kmem */
+ RAW_TP_NULL_ARGS(mm_page_alloc, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(mm_page_pcpu_drain, NULL_ARG(1)),
+ /* .. from mm_page event class */
+ RAW_TP_NULL_ARGS(mm_page_alloc_zone_locked, NULL_ARG(1)),
+ /* netfs */
+ RAW_TP_NULL_ARGS(netfs_failure, NULL_ARG(2)),
+ /* power */
+ RAW_TP_NULL_ARGS(device_pm_callback_start, NULL_ARG(2)),
+ /* qdisc */
+ RAW_TP_NULL_ARGS(qdisc_dequeue, NULL_ARG(4)),
+ /* rxrpc */
+ RAW_TP_NULL_ARGS(rxrpc_recvdata, NULL_ARG(1)),
+ RAW_TP_NULL_ARGS(rxrpc_resend, NULL_ARG(2)),
+ /* sunrpc */
+ RAW_TP_NULL_ARGS(xs_stream_read_data, NULL_ARG(1)),
+ /* tcp */
+ RAW_TP_NULL_ARGS(tcp_send_reset, NULL_ARG(1) | NULL_ARG(2)),
+ /* tegra_apb_dma */
+ RAW_TP_NULL_ARGS(tegra_dma_tx_status, NULL_ARG(3)),
+ /* timer_migration */
+ RAW_TP_NULL_ARGS(tmigr_update_events, NULL_ARG(1)),
+ /* writeback, from writeback_folio_template event class */
+ RAW_TP_NULL_ARGS(writeback_dirty_folio, NULL_ARG(2)),
+ RAW_TP_NULL_ARGS(folio_wait_writeback, NULL_ARG(2)),
+};
+
bool btf_ctx_access(int off, int size, enum bpf_access_type type,
const struct bpf_prog *prog,
struct bpf_insn_access_aux *info)
@@ -6449,6 +6539,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
const char *tname = prog->aux->attach_func_name;
struct bpf_verifier_log *log = info->log;
const struct btf_param *args;
+ bool ptr_err_raw_tp = false;
const char *tag_value;
u32 nr_args, arg;
int i, ret;
@@ -6591,6 +6682,36 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
info->reg_type |= PTR_MAYBE_NULL;
+ if (prog->expected_attach_type == BPF_TRACE_RAW_TP) {
+ struct btf *btf = prog->aux->attach_btf;
+ const struct btf_type *t;
+ const char *tname;
+
+ t = btf_type_by_id(btf, prog->aux->attach_btf_id);
+ if (!t)
+ goto done;
+ tname = btf_name_by_offset(btf, t->name_off);
+ if (!tname)
+ goto done;
+ for (int i = 0; i < ARRAY_SIZE(raw_tp_null_args); i++) {
+ /* Is this a func with potential NULL args? */
+ if (strcmp(tname, raw_tp_null_args[i].func))
+ continue;
+ /* Is the current arg NULL? */
+ if (raw_tp_null_args[i].mask & NULL_ARG(arg + 1))
+ info->reg_type |= PTR_MAYBE_NULL;
+ break;
+ }
+ /* Hardcode the only cases which has a IS_ERR pointer, i.e.
+ * mr_integ_alloc's 4th argument (mr), and
+ * cachefiles_lookup's 3rd argument (de).
+ */
+ if (!strcmp(tname, "btf_trace_mr_integ_alloc") && (arg + 1) == 4)
+ ptr_err_raw_tp = true;
+ if (!strcmp(tname, "btf_trace_cachefiles_lookup") && (arg + 1) == 3)
+ ptr_err_raw_tp = true;
+ }
+done:
if (tgt_prog) {
enum bpf_prog_type tgt_type;
@@ -6635,6 +6756,14 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n",
tname, arg, info->btf_id, btf_type_str(t),
__btf_name_by_offset(btf, t->name_off));
+
+ /* Perform all checks on the validity of type for this argument, but if
+ * we know it can be IS_ERR at runtime, scrub pointer type and mark as
+ * scalar. We do not handle is_retval case as we hardcode ptr_err_raw_tp
+ * handling for known tps.
+ */
+ if (ptr_err_raw_tp)
+ info->reg_type = SCALAR_VALUE;
return true;
}
EXPORT_SYMBOL_GPL(btf_ctx_access);
--
2.43.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf v1 4/4] selftests/bpf: Add autogenerated tests for raw_tp NULL args
2024-12-11 2:01 [PATCH bpf v1 0/4] Explicit raw_tp NULL arguments Kumar Kartikeya Dwivedi
` (2 preceding siblings ...)
2024-12-11 2:01 ` [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL Kumar Kartikeya Dwivedi
@ 2024-12-11 2:01 ` Kumar Kartikeya Dwivedi
2024-12-11 16:02 ` Alexei Starovoitov
3 siblings, 1 reply; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2024-12-11 2:01 UTC (permalink / raw)
To: bpf
Cc: kkd, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, Manu Bretelle, Jiri Olsa,
Juri Lelli, kernel-team
Add bash and python scripts that process the RAW_TP_NULL_ARGS list in
kernel/bpf/btf.c and produce selftests automatically. This is not done
automatically on build as the file requires some human-guided post
processing (like disabling certain tests for which we don't enable
CONFIG options), and only needs to be run once when growing the list.
Once the list generation becomes automatic in kernel/bpf/btf.c, the
script can run on build, or be modified if we support __nullable BTF
type tags in the future to parse them and generate tests accordingly.
The tests basically ensure the pointer is marked or_null_, and likewise
for raw_tp_scalar.c case (where it needs to be marked scalar).
Enable enough config options to cover all but 4 without increasing build
time significantly. By enabling AFS, CACHEFILES, and INFINIBAND, we gain
enough coverage to cover distinct cases and positional arguments.
The config is modified to include some new options to test most options,
but driver tracepoints that include too much stuff are disabled manually
after the script produces it's output.
Whenever adding a new RAW_TP_NULL_ARGS specification or removing one,
the developer can run this script to update the selftest, reject hunks
removing the comments around disabled tracepoints, and accept other
hunks that are relevant for the newly added tracepoint.
There are some tracepoints manually hardcoded in btf_ctx_access that
have an IS_ERR argument type. For now, these are manually encoded in
raw_tp_scalar.c, but if the list grows, we can introduce a new mask
for IS_ERR args, an ERR_ARG() macro, and augment the script to generate
tests for these cases and ensure argument is marked scalar.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
tools/testing/selftests/bpf/config | 5 +
.../testing/selftests/bpf/gen_raw_tp_null.py | 58 +++
.../testing/selftests/bpf/gen_raw_tp_null.sh | 3 +
.../selftests/bpf/prog_tests/raw_tp_null.c | 12 +
.../testing/selftests/bpf/progs/raw_tp_null.c | 417 ++++++++++++++++++
.../selftests/bpf/progs/raw_tp_scalar.c | 24 +
6 files changed, 519 insertions(+)
create mode 100755 tools/testing/selftests/bpf/gen_raw_tp_null.py
create mode 100755 tools/testing/selftests/bpf/gen_raw_tp_null.sh
create mode 100644 tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
create mode 100644 tools/testing/selftests/bpf/progs/raw_tp_null.c
create mode 100644 tools/testing/selftests/bpf/progs/raw_tp_scalar.c
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 4ca84c8d9116..75d3416e2c11 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -112,3 +112,8 @@ CONFIG_XDP_SOCKETS=y
CONFIG_XFRM_INTERFACE=y
CONFIG_TCP_CONG_DCTCP=y
CONFIG_TCP_CONG_BBR=y
+CONFIG_AFS_FS=y
+CONFIG_FSCACHE=y
+CONFIG_CACHEFILES=y
+CONFIG_CACHEFILES_ONDEMAND=y
+CONFIG_INFINIBAND=y
diff --git a/tools/testing/selftests/bpf/gen_raw_tp_null.py b/tools/testing/selftests/bpf/gen_raw_tp_null.py
new file mode 100755
index 000000000000..4d15a5b92012
--- /dev/null
+++ b/tools/testing/selftests/bpf/gen_raw_tp_null.py
@@ -0,0 +1,58 @@
+#!/usr/bin/python3
+import re
+import sys
+
+def parse_null_args(arg_str):
+ pattern = r'NULL_ARG\((\d+)\)'
+ numbers = []
+ for part in arg_str.split('|'):
+ part = part.strip()
+ match = re.match(pattern, part)
+ if match:
+ numbers.append(int(match.group(1)))
+ return numbers
+
+def parse_tracepoint_line(line):
+ line = line.strip().rstrip(',')
+ match = re.match(r'RAW_TP_NULL_ARGS\(([^,]+),\s*(.*)\)', line)
+
+ if match:
+ tp_name = match.group(1).strip()
+ arg_part = match.group(2).strip()
+ arg_nums = parse_null_args(arg_part)
+ if arg_nums:
+ return tp_name, arg_nums
+ return None, None
+
+def generate_tests(entries):
+ tests = []
+
+ for tp_name, arg_nums in entries:
+ for arg_num in arg_nums:
+ test = ['', 'SEC("tp_btf/' + tp_name + '")',
+ '__failure __msg("R1 invalid mem access \'trusted_ptr_or_null_\'")',
+ f'int test_raw_tp_null_{tp_name}_arg_{arg_num}(void *ctx) {{']
+ n = (arg_num - 1) * 8
+ test.append(f' asm volatile("r1 = *(u64 *)(r1 +{n}); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);')
+ test.extend([' return 0;', '}'])
+ tests.extend(test)
+ return '\n'.join(tests)
+
+# Read directly from stdin
+entries = []
+for line in sys.stdin:
+ tp_name, arg_num = parse_tracepoint_line(line)
+ if tp_name and arg_num is not None:
+ entries.append((tp_name, arg_num))
+print(
+'''// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+/* WARNING: This file is automatically generated, run gen_raw_tp_null.sh to update! */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";''')
+print(generate_tests(entries))
diff --git a/tools/testing/selftests/bpf/gen_raw_tp_null.sh b/tools/testing/selftests/bpf/gen_raw_tp_null.sh
new file mode 100755
index 000000000000..1c99757f7baf
--- /dev/null
+++ b/tools/testing/selftests/bpf/gen_raw_tp_null.sh
@@ -0,0 +1,3 @@
+#!/bin/bash
+
+cat ../../../../kernel/bpf/btf.c | grep RAW_TP_NULL_ARGS | grep -v "define RAW_TP" | ./gen_raw_tp_null.py | tee progs/raw_tp_null.c
diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
new file mode 100644
index 000000000000..bb5524eabde9
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+#include <test_progs.h>
+#include "raw_tp_null.skel.h"
+#include "raw_tp_scalar.skel.h"
+
+void test_raw_tp_null(void)
+{
+ RUN_TESTS(raw_tp_null);
+ RUN_TESTS(raw_tp_scalar);
+}
diff --git a/tools/testing/selftests/bpf/progs/raw_tp_null.c b/tools/testing/selftests/bpf/progs/raw_tp_null.c
new file mode 100644
index 000000000000..fd4de11b587f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/raw_tp_null.c
@@ -0,0 +1,417 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+/* WARNING: This file is automatically generated, run gen_raw_tp_null.sh to update! */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("tp_btf/sched_pi_setprio")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_sched_pi_setprio_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/sched_stick_numa")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_sched_stick_numa_arg_3(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +16); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/sched_swap_numa")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_sched_swap_numa_arg_3(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +16); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/afs_make_fs_call")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_afs_make_fs_call_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/afs_make_fs_calli")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_afs_make_fs_calli_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/afs_make_fs_call1")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_afs_make_fs_call1_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/afs_make_fs_call2")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_afs_make_fs_call2_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/afs_protocol_error")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_afs_protocol_error_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/afs_flock_ev")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_afs_flock_ev_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_lookup")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_lookup_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_unlink")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_unlink_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_rename")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_rename_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_prep_read")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_prep_read_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_mark_active")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_mark_active_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_mark_failed")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_mark_failed_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_mark_inactive")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_mark_inactive_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_vfs_error")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_vfs_error_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_io_error")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_io_error_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_ondemand_open")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_ondemand_open_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_ondemand_copen")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_ondemand_copen_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_ondemand_close")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_ondemand_close_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_ondemand_read")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_ondemand_read_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_ondemand_cread")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_ondemand_cread_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_ondemand_fd_write")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_ondemand_fd_write_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_ondemand_fd_release")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_cachefiles_ondemand_fd_release_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/ext4_mballoc_discard")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_ext4_mballoc_discard_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/ext4_mballoc_free")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_ext4_mballoc_free_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/fib_table_lookup")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_fib_table_lookup_arg_3(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +16); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/posix_lock_inode")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_posix_lock_inode_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/fcntl_setlk")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_fcntl_setlk_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/locks_remove_posix")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_locks_remove_posix_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/flock_lock_inode")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_flock_lock_inode_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/break_lease_noblock")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_break_lease_noblock_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/break_lease_block")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_break_lease_block_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/break_lease_unblock")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_break_lease_unblock_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/generic_delete_lease")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_generic_delete_lease_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/time_out_leases")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_time_out_leases_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+/* Disabled due to missing CONFIG
+SEC("tp_btf/host1x_cdma_push_gather")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_host1x_cdma_push_gather_arg_5(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +32); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+*/
+
+SEC("tp_btf/mm_khugepaged_scan_pmd")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_mm_khugepaged_scan_pmd_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/mm_collapse_huge_page_isolate")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_mm_collapse_huge_page_isolate_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/mm_khugepaged_scan_file")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_mm_khugepaged_scan_file_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/mm_khugepaged_collapse_file")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_mm_khugepaged_collapse_file_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/mm_page_alloc")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_mm_page_alloc_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/mm_page_pcpu_drain")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_mm_page_pcpu_drain_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/mm_page_alloc_zone_locked")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_mm_page_alloc_zone_locked_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/netfs_failure")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_netfs_failure_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+/* Disabled due to missing CONFIG
+SEC("tp_btf/device_pm_callback_start")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_device_pm_callback_start_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+*/
+
+SEC("tp_btf/qdisc_dequeue")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_qdisc_dequeue_arg_4(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +24); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/rxrpc_recvdata")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_rxrpc_recvdata_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/rxrpc_resend")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_rxrpc_resend_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+/* Disabled due to missing CONFIG
+SEC("tp_btf/xs_stream_read_data")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_xs_stream_read_data_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+*/
+
+SEC("tp_btf/tcp_send_reset")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_tcp_send_reset_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/tcp_send_reset")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_tcp_send_reset_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+/* Disabled due to missing CONFIG
+SEC("tp_btf/tegra_dma_tx_status")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_tegra_dma_tx_status_arg_3(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +16); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+*/
+
+SEC("tp_btf/tmigr_update_events")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_tmigr_update_events_arg_1(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +0); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/writeback_dirty_folio")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_writeback_dirty_folio_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/folio_wait_writeback")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int test_raw_tp_null_folio_wait_writeback_arg_2(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/raw_tp_scalar.c b/tools/testing/selftests/bpf/progs/raw_tp_scalar.c
new file mode 100644
index 000000000000..b44bb9a94305
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/raw_tp_scalar.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+/* Since we have a couple of cases, we just write this file by hand. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("tp_btf/mr_integ_alloc")
+__failure __msg("R1 invalid mem access 'scalar'")
+int test_raw_tp_scalar_mr_integ_alloc_arg_4(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +24); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
+
+SEC("tp_btf/cachefiles_lookup")
+__failure __msg("R1 invalid mem access 'scalar'")
+int test_raw_tp_scalar_cachefiles_lookup_arg_3(void *ctx) {
+ asm volatile("r1 = *(u64 *)(r1 +16); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
+ return 0;
+}
--
2.43.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL
2024-12-11 2:01 ` [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL Kumar Kartikeya Dwivedi
@ 2024-12-11 12:24 ` Jiri Olsa
2024-12-11 14:56 ` Jiri Olsa
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2024-12-11 12:24 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, kkd, Juri Lelli, Manu Bretelle, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau,
Eduard Zingerman, kernel-team
On Tue, Dec 10, 2024 at 06:01:55PM -0800, Kumar Kartikeya Dwivedi wrote:
> 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.
>
> A previous attempt [1], i.e. the second fixed commit, was made to
> simulate symbolic execution as if in most accesses, the argument is a
> non-NULL raw_tp, except for conditional jumps. This tried to suppress
> branch prediction while preserving compatibility, but surfaced issues
> with production programs that were difficult to solve without increasing
> verifier complexity. A more complete discussion of issues and fixes is
> available at [2].
>
> Fix this by maintaining an explicit, incomplete list of tracepoints
> where the arguments are known to be NULL, and mark the positional
> arguments as PTR_MAYBE_NULL. Additionally, capture the tracepoints where
> arguments are known to be PTR_ERR, and mark these arguments as scalar
> values to prevent potential dereference.
>
> In the future, an automated pass will be used to produce such a list, or
> insert __nullable annotations automatically for tracepoints. Anyhow,
> this is an attempt to close the gap until the automation lands, and
so this won't cover modules with raw tracepoints, but I guess it's fine
as temporary solution until we have __nullable annotation support
SNIP
> bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> const struct bpf_prog *prog,
> struct bpf_insn_access_aux *info)
> @@ -6449,6 +6539,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> const char *tname = prog->aux->attach_func_name;
> struct bpf_verifier_log *log = info->log;
> const struct btf_param *args;
> + bool ptr_err_raw_tp = false;
> const char *tag_value;
> u32 nr_args, arg;
> int i, ret;
> @@ -6591,6 +6682,36 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
> info->reg_type |= PTR_MAYBE_NULL;
>
> + if (prog->expected_attach_type == BPF_TRACE_RAW_TP) {
> + struct btf *btf = prog->aux->attach_btf;
> + const struct btf_type *t;
> + const char *tname;
> +
> + t = btf_type_by_id(btf, prog->aux->attach_btf_id);
> + if (!t)
> + goto done;
> + tname = btf_name_by_offset(btf, t->name_off);
> + if (!tname)
> + goto done;
I think both btf_type_by_id and btf_name_by_offset should succeed for
BPF_TRACE_RAW_TP .. should be already checked in bpf_check_attach_target
> + for (int i = 0; i < ARRAY_SIZE(raw_tp_null_args); i++) {
> + /* Is this a func with potential NULL args? */
> + if (strcmp(tname, raw_tp_null_args[i].func))
> + continue;
> + /* Is the current arg NULL? */
> + if (raw_tp_null_args[i].mask & NULL_ARG(arg + 1))
> + info->reg_type |= PTR_MAYBE_NULL;
> + break;
> + }
> + /* Hardcode the only cases which has a IS_ERR pointer, i.e.
> + * mr_integ_alloc's 4th argument (mr), and
> + * cachefiles_lookup's 3rd argument (de).
> + */
> + if (!strcmp(tname, "btf_trace_mr_integ_alloc") && (arg + 1) == 4)
> + ptr_err_raw_tp = true;
> + if (!strcmp(tname, "btf_trace_cachefiles_lookup") && (arg + 1) == 3)
> + ptr_err_raw_tp = true;
could we have extra mask value (or split the current one in half) in
struct bpf_raw_tp_null_args and use it for scalar arguments? so we don't
have special checks and handle everything in the loop above
jirka
> + }
> +done:
> if (tgt_prog) {
> enum bpf_prog_type tgt_type;
>
> @@ -6635,6 +6756,14 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n",
> tname, arg, info->btf_id, btf_type_str(t),
> __btf_name_by_offset(btf, t->name_off));
> +
> + /* Perform all checks on the validity of type for this argument, but if
> + * we know it can be IS_ERR at runtime, scrub pointer type and mark as
> + * scalar. We do not handle is_retval case as we hardcode ptr_err_raw_tp
> + * handling for known tps.
> + */
> + if (ptr_err_raw_tp)
> + info->reg_type = SCALAR_VALUE;
> return true;
> }
> EXPORT_SYMBOL_GPL(btf_ctx_access);
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL
2024-12-11 2:01 ` [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL Kumar Kartikeya Dwivedi
2024-12-11 12:24 ` Jiri Olsa
@ 2024-12-11 14:56 ` Jiri Olsa
2024-12-11 15:56 ` Alexei Starovoitov
2024-12-12 11:39 ` kernel test robot
3 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2024-12-11 14:56 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, kkd, Juri Lelli, Manu Bretelle, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau,
Eduard Zingerman, kernel-team
On Tue, Dec 10, 2024 at 06:01:55PM -0800, Kumar Kartikeya Dwivedi wrote:
SNIP
> +struct bpf_raw_tp_null_args raw_tp_null_args[] = {
> + /* sched */
> + RAW_TP_NULL_ARGS(sched_pi_setprio, NULL_ARG(2)),
> + /* ... from sched_numa_pair_template event class */
> + RAW_TP_NULL_ARGS(sched_stick_numa, NULL_ARG(3)),
> + RAW_TP_NULL_ARGS(sched_swap_numa, NULL_ARG(3)),
> + /* afs */
> + RAW_TP_NULL_ARGS(afs_make_fs_call, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(afs_make_fs_calli, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(afs_make_fs_call1, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(afs_make_fs_call2, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(afs_protocol_error, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(afs_flock_ev, NULL_ARG(2)),
> + /* cachefiles */
> + RAW_TP_NULL_ARGS(cachefiles_lookup, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_unlink, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_rename, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_prep_read, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_mark_active, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_mark_failed, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_mark_inactive, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_vfs_error, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_io_error, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_ondemand_open, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_ondemand_copen, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_ondemand_close, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_ondemand_read, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_ondemand_cread, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_ondemand_fd_write, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(cachefiles_ondemand_fd_release, NULL_ARG(1)),
> + /* ext4, from ext4__mballoc event class */
> + RAW_TP_NULL_ARGS(ext4_mballoc_discard, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(ext4_mballoc_free, NULL_ARG(2)),
> + /* fib */
> + RAW_TP_NULL_ARGS(fib_table_lookup, NULL_ARG(3)),
> + /* filelock */
> + /* ... from filelock_lock event class */
> + RAW_TP_NULL_ARGS(posix_lock_inode, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(fcntl_setlk, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(locks_remove_posix, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(flock_lock_inode, NULL_ARG(2)),
> + /* ... from filelock_lease event class */
> + RAW_TP_NULL_ARGS(break_lease_noblock, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(break_lease_block, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(break_lease_unblock, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(generic_delete_lease, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(time_out_leases, NULL_ARG(2)),
> + /* host1x */
> + RAW_TP_NULL_ARGS(host1x_cdma_push_gather, NULL_ARG(5)),
> + /* huge_memory */
> + RAW_TP_NULL_ARGS(mm_khugepaged_scan_pmd, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(mm_collapse_huge_page_isolate, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(mm_khugepaged_scan_file, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(mm_khugepaged_collapse_file, NULL_ARG(2)),
> + /* kmem */
> + RAW_TP_NULL_ARGS(mm_page_alloc, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(mm_page_pcpu_drain, NULL_ARG(1)),
> + /* .. from mm_page event class */
> + RAW_TP_NULL_ARGS(mm_page_alloc_zone_locked, NULL_ARG(1)),
> + /* netfs */
> + RAW_TP_NULL_ARGS(netfs_failure, NULL_ARG(2)),
> + /* power */
> + RAW_TP_NULL_ARGS(device_pm_callback_start, NULL_ARG(2)),
> + /* qdisc */
> + RAW_TP_NULL_ARGS(qdisc_dequeue, NULL_ARG(4)),
> + /* rxrpc */
> + RAW_TP_NULL_ARGS(rxrpc_recvdata, NULL_ARG(1)),
> + RAW_TP_NULL_ARGS(rxrpc_resend, NULL_ARG(2)),
> + /* sunrpc */
> + RAW_TP_NULL_ARGS(xs_stream_read_data, NULL_ARG(1)),
I missed one more in sunrpc: xprt_cong_event class
jirka
> + /* tcp */
> + RAW_TP_NULL_ARGS(tcp_send_reset, NULL_ARG(1) | NULL_ARG(2)),
> + /* tegra_apb_dma */
> + RAW_TP_NULL_ARGS(tegra_dma_tx_status, NULL_ARG(3)),
> + /* timer_migration */
> + RAW_TP_NULL_ARGS(tmigr_update_events, NULL_ARG(1)),
> + /* writeback, from writeback_folio_template event class */
> + RAW_TP_NULL_ARGS(writeback_dirty_folio, NULL_ARG(2)),
> + RAW_TP_NULL_ARGS(folio_wait_writeback, NULL_ARG(2)),
> +};
> +
> bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> const struct bpf_prog *prog,
> struct bpf_insn_access_aux *info)
> @@ -6449,6 +6539,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> const char *tname = prog->aux->attach_func_name;
> struct bpf_verifier_log *log = info->log;
> const struct btf_param *args;
> + bool ptr_err_raw_tp = false;
> const char *tag_value;
> u32 nr_args, arg;
> int i, ret;
> @@ -6591,6 +6682,36 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> if (btf_param_match_suffix(btf, &args[arg], "__nullable"))
> info->reg_type |= PTR_MAYBE_NULL;
>
> + if (prog->expected_attach_type == BPF_TRACE_RAW_TP) {
> + struct btf *btf = prog->aux->attach_btf;
> + const struct btf_type *t;
> + const char *tname;
> +
> + t = btf_type_by_id(btf, prog->aux->attach_btf_id);
> + if (!t)
> + goto done;
> + tname = btf_name_by_offset(btf, t->name_off);
> + if (!tname)
> + goto done;
> + for (int i = 0; i < ARRAY_SIZE(raw_tp_null_args); i++) {
> + /* Is this a func with potential NULL args? */
> + if (strcmp(tname, raw_tp_null_args[i].func))
> + continue;
> + /* Is the current arg NULL? */
> + if (raw_tp_null_args[i].mask & NULL_ARG(arg + 1))
> + info->reg_type |= PTR_MAYBE_NULL;
> + break;
> + }
> + /* Hardcode the only cases which has a IS_ERR pointer, i.e.
> + * mr_integ_alloc's 4th argument (mr), and
> + * cachefiles_lookup's 3rd argument (de).
> + */
> + if (!strcmp(tname, "btf_trace_mr_integ_alloc") && (arg + 1) == 4)
> + ptr_err_raw_tp = true;
> + if (!strcmp(tname, "btf_trace_cachefiles_lookup") && (arg + 1) == 3)
> + ptr_err_raw_tp = true;
> + }
> +done:
> if (tgt_prog) {
> enum bpf_prog_type tgt_type;
>
> @@ -6635,6 +6756,14 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n",
> tname, arg, info->btf_id, btf_type_str(t),
> __btf_name_by_offset(btf, t->name_off));
> +
> + /* Perform all checks on the validity of type for this argument, but if
> + * we know it can be IS_ERR at runtime, scrub pointer type and mark as
> + * scalar. We do not handle is_retval case as we hardcode ptr_err_raw_tp
> + * handling for known tps.
> + */
> + if (ptr_err_raw_tp)
> + info->reg_type = SCALAR_VALUE;
> return true;
> }
> EXPORT_SYMBOL_GPL(btf_ctx_access);
> --
> 2.43.5
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL
2024-12-11 2:01 ` [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL Kumar Kartikeya Dwivedi
2024-12-11 12:24 ` Jiri Olsa
2024-12-11 14:56 ` Jiri Olsa
@ 2024-12-11 15:56 ` Alexei Starovoitov
2024-12-12 11:39 ` kernel test robot
3 siblings, 0 replies; 10+ messages in thread
From: Alexei Starovoitov @ 2024-12-11 15:56 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, kkd, Juri Lelli, Manu Bretelle, Jiri Olsa,
Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, Kernel Team
On Tue, Dec 10, 2024 at 6:02 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> 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.
>
> A previous attempt [1], i.e. the second fixed commit, was made to
> simulate symbolic execution as if in most accesses, the argument is a
> non-NULL raw_tp, except for conditional jumps. This tried to suppress
> branch prediction while preserving compatibility, but surfaced issues
> with production programs that were difficult to solve without increasing
> verifier complexity. A more complete discussion of issues and fixes is
> available at [2].
>
> Fix this by maintaining an explicit, incomplete list of tracepoints
> where the arguments are known to be NULL, and mark the positional
> arguments as PTR_MAYBE_NULL. Additionally, capture the tracepoints where
> arguments are known to be PTR_ERR, and mark these arguments as scalar
> values to prevent potential dereference.
>
> In the future, an automated pass will be used to produce such a list, or
> insert __nullable annotations automatically for tracepoints. Anyhow,
> this is an attempt to close the gap until the automation lands, and
> reflets the current best known list according to Jiri's analysis in [3].
>
> [0]: https://lore.kernel.org/bpf/ZrCZS6nisraEqehw@jlelli-thinkpadt14gen4.remote.csb
> [1]: https://lore.kernel.org/all/20241104171959.2938862-1-memxor@gmail.com
> [2]: https://lore.kernel.org/bpf/20241206161053.809580-1-memxor@gmail.com
> [3]: https://lore.kernel.org/bpf/Z1d-qbCdtJqg6Er4@krava
>
> Reported-by: Juri Lelli <juri.lelli@redhat.com> # original bug
> Reported-by: Manu Bretelle <chantra@meta.com> # bugs in masking fix
> Fixes: 3f00c5239344 ("bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs")
> Fixes: cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL")
> Co-developed-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
> kernel/bpf/btf.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 129 insertions(+)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index ed3219da7181..cb72cbf04d12 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6439,6 +6439,96 @@ int btf_ctx_arg_offset(const struct btf *btf, const struct btf_type *func_proto,
> return off;
> }
>
> +struct bpf_raw_tp_null_args {
> + const char *func;
> + u64 mask;
> +};
> +
> +#define RAW_TP_NULL_ARGS(str, arg) { .func = "btf_trace_" #str, .mask = (arg) }
> +/* Use 1-based indexing for argno */
> +#define NULL_ARG(argno) (1 << (argno))
> +
> +struct bpf_raw_tp_null_args raw_tp_null_args[] = {
> + /* sched */
> + RAW_TP_NULL_ARGS(sched_pi_setprio, NULL_ARG(2)),
> + /* ... from sched_numa_pair_template event class */
> + RAW_TP_NULL_ARGS(sched_stick_numa, NULL_ARG(3)),
Let's avoid LOUD macros.
"btf_trace_" prefix can also be dropped to save space.
How about the following encoding:
{"sched_pi_setprio", 0x10},
{"sched_stick_numa", 0x100},
> + RAW_TP_NULL_ARGS(cachefiles_lookup, NULL_ARG(1)),
{"cachefiles_lookup", 0x1},
There is no need for arg0, since the count starts from arg1.
> + if (!strcmp(tname, "btf_trace_mr_integ_alloc") && (arg + 1) == 4)
> + ptr_err_raw_tp = true;
> + if (!strcmp(tname, "btf_trace_cachefiles_lookup") && (arg + 1) == 3)
> + ptr_err_raw_tp = true;
+1 to Jiri's point.
Let's use 0x2 to encode the scalar ?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf v1 4/4] selftests/bpf: Add autogenerated tests for raw_tp NULL args
2024-12-11 2:01 ` [PATCH bpf v1 4/4] selftests/bpf: Add autogenerated tests for raw_tp NULL args Kumar Kartikeya Dwivedi
@ 2024-12-11 16:02 ` Alexei Starovoitov
0 siblings, 0 replies; 10+ messages in thread
From: Alexei Starovoitov @ 2024-12-11 16:02 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, kkd, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, Manu Bretelle, Jiri Olsa,
Juri Lelli, Kernel Team
On Tue, Dec 10, 2024 at 6:02 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
> +cat ../../../../kernel/bpf/btf.c | grep RAW_TP_NULL_ARGS | grep -v "define RAW_TP" | ./gen_raw_tp_null.py | tee progs/raw_tp_null.c
This is a serious overkill for something that will be removed soon
when automation to analyse TP_fast_assign() is ready.
> +++ b/tools/testing/selftests/bpf/progs/raw_tp_null.c
> @@ -0,0 +1,417 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
> +
> +/* WARNING: This file is automatically generated, run gen_raw_tp_null.sh to update! */
let's not.
> +
> +#include <vmlinux.h>
> +#include <bpf/bpf_tracing.h>
> +#include "bpf_misc.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("tp_btf/sched_pi_setprio")
> +__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
> +int test_raw_tp_null_sched_pi_setprio_arg_2(void *ctx) {
> + asm volatile("r1 = *(u64 *)(r1 +8); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
> + return 0;
> +}
This one is enough to test it.
Drop all below. They don't add value. Copy paste doesn't improve coverage.
> +
> +SEC("tp_btf/sched_stick_numa")
> +__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
> +int test_raw_tp_null_sched_stick_numa_arg_3(void *ctx) {
> + asm volatile("r1 = *(u64 *)(r1 +16); r1 = *(u64 *)(r1 +0);" ::: __clobber_all);
> + return 0;
> +}
...
pw-bot: cr
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL
2024-12-11 2:01 ` [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL Kumar Kartikeya Dwivedi
` (2 preceding siblings ...)
2024-12-11 15:56 ` Alexei Starovoitov
@ 2024-12-12 11:39 ` kernel test robot
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-12-12 11:39 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi, bpf
Cc: oe-kbuild-all, kkd, Juri Lelli, Manu Bretelle, Jiri Olsa,
Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Eduard Zingerman, kernel-team
Hi Kumar,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 7d0d673627e20cfa3b21a829a896ce03b58a4f1c]
url: https://github.com/intel-lab-lkp/linux/commits/Kumar-Kartikeya-Dwivedi/bpf-Revert-bpf-Mark-raw_tp-arguments-with-PTR_MAYBE_NULL/20241211-100358
base: 7d0d673627e20cfa3b21a829a896ce03b58a4f1c
patch link: https://lore.kernel.org/r/20241211020156.18966-4-memxor%40gmail.com
patch subject: [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL
config: i386-randconfig-062-20241212 (https://download.01.org/0day-ci/archive/20241212/202412121921.xPnFS8u5-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241212/202412121921.xPnFS8u5-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412121921.xPnFS8u5-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> kernel/bpf/btf.c:6451:29: sparse: sparse: symbol 'raw_tp_null_args' was not declared. Should it be static?
kernel/bpf/btf.c: note: in included file (through include/linux/bpf.h, include/linux/bpf_verifier.h):
include/linux/bpfptr.h:65:40: sparse: sparse: cast to non-scalar
include/linux/bpfptr.h:65:40: sparse: sparse: cast from non-scalar
vim +/raw_tp_null_args +6451 kernel/bpf/btf.c
6450
> 6451 struct bpf_raw_tp_null_args raw_tp_null_args[] = {
6452 /* sched */
6453 RAW_TP_NULL_ARGS(sched_pi_setprio, NULL_ARG(2)),
6454 /* ... from sched_numa_pair_template event class */
6455 RAW_TP_NULL_ARGS(sched_stick_numa, NULL_ARG(3)),
6456 RAW_TP_NULL_ARGS(sched_swap_numa, NULL_ARG(3)),
6457 /* afs */
6458 RAW_TP_NULL_ARGS(afs_make_fs_call, NULL_ARG(2)),
6459 RAW_TP_NULL_ARGS(afs_make_fs_calli, NULL_ARG(2)),
6460 RAW_TP_NULL_ARGS(afs_make_fs_call1, NULL_ARG(2)),
6461 RAW_TP_NULL_ARGS(afs_make_fs_call2, NULL_ARG(2)),
6462 RAW_TP_NULL_ARGS(afs_protocol_error, NULL_ARG(1)),
6463 RAW_TP_NULL_ARGS(afs_flock_ev, NULL_ARG(2)),
6464 /* cachefiles */
6465 RAW_TP_NULL_ARGS(cachefiles_lookup, NULL_ARG(1)),
6466 RAW_TP_NULL_ARGS(cachefiles_unlink, NULL_ARG(1)),
6467 RAW_TP_NULL_ARGS(cachefiles_rename, NULL_ARG(1)),
6468 RAW_TP_NULL_ARGS(cachefiles_prep_read, NULL_ARG(1)),
6469 RAW_TP_NULL_ARGS(cachefiles_mark_active, NULL_ARG(1)),
6470 RAW_TP_NULL_ARGS(cachefiles_mark_failed, NULL_ARG(1)),
6471 RAW_TP_NULL_ARGS(cachefiles_mark_inactive, NULL_ARG(1)),
6472 RAW_TP_NULL_ARGS(cachefiles_vfs_error, NULL_ARG(1)),
6473 RAW_TP_NULL_ARGS(cachefiles_io_error, NULL_ARG(1)),
6474 RAW_TP_NULL_ARGS(cachefiles_ondemand_open, NULL_ARG(1)),
6475 RAW_TP_NULL_ARGS(cachefiles_ondemand_copen, NULL_ARG(1)),
6476 RAW_TP_NULL_ARGS(cachefiles_ondemand_close, NULL_ARG(1)),
6477 RAW_TP_NULL_ARGS(cachefiles_ondemand_read, NULL_ARG(1)),
6478 RAW_TP_NULL_ARGS(cachefiles_ondemand_cread, NULL_ARG(1)),
6479 RAW_TP_NULL_ARGS(cachefiles_ondemand_fd_write, NULL_ARG(1)),
6480 RAW_TP_NULL_ARGS(cachefiles_ondemand_fd_release, NULL_ARG(1)),
6481 /* ext4, from ext4__mballoc event class */
6482 RAW_TP_NULL_ARGS(ext4_mballoc_discard, NULL_ARG(2)),
6483 RAW_TP_NULL_ARGS(ext4_mballoc_free, NULL_ARG(2)),
6484 /* fib */
6485 RAW_TP_NULL_ARGS(fib_table_lookup, NULL_ARG(3)),
6486 /* filelock */
6487 /* ... from filelock_lock event class */
6488 RAW_TP_NULL_ARGS(posix_lock_inode, NULL_ARG(2)),
6489 RAW_TP_NULL_ARGS(fcntl_setlk, NULL_ARG(2)),
6490 RAW_TP_NULL_ARGS(locks_remove_posix, NULL_ARG(2)),
6491 RAW_TP_NULL_ARGS(flock_lock_inode, NULL_ARG(2)),
6492 /* ... from filelock_lease event class */
6493 RAW_TP_NULL_ARGS(break_lease_noblock, NULL_ARG(2)),
6494 RAW_TP_NULL_ARGS(break_lease_block, NULL_ARG(2)),
6495 RAW_TP_NULL_ARGS(break_lease_unblock, NULL_ARG(2)),
6496 RAW_TP_NULL_ARGS(generic_delete_lease, NULL_ARG(2)),
6497 RAW_TP_NULL_ARGS(time_out_leases, NULL_ARG(2)),
6498 /* host1x */
6499 RAW_TP_NULL_ARGS(host1x_cdma_push_gather, NULL_ARG(5)),
6500 /* huge_memory */
6501 RAW_TP_NULL_ARGS(mm_khugepaged_scan_pmd, NULL_ARG(2)),
6502 RAW_TP_NULL_ARGS(mm_collapse_huge_page_isolate, NULL_ARG(1)),
6503 RAW_TP_NULL_ARGS(mm_khugepaged_scan_file, NULL_ARG(2)),
6504 RAW_TP_NULL_ARGS(mm_khugepaged_collapse_file, NULL_ARG(2)),
6505 /* kmem */
6506 RAW_TP_NULL_ARGS(mm_page_alloc, NULL_ARG(1)),
6507 RAW_TP_NULL_ARGS(mm_page_pcpu_drain, NULL_ARG(1)),
6508 /* .. from mm_page event class */
6509 RAW_TP_NULL_ARGS(mm_page_alloc_zone_locked, NULL_ARG(1)),
6510 /* netfs */
6511 RAW_TP_NULL_ARGS(netfs_failure, NULL_ARG(2)),
6512 /* power */
6513 RAW_TP_NULL_ARGS(device_pm_callback_start, NULL_ARG(2)),
6514 /* qdisc */
6515 RAW_TP_NULL_ARGS(qdisc_dequeue, NULL_ARG(4)),
6516 /* rxrpc */
6517 RAW_TP_NULL_ARGS(rxrpc_recvdata, NULL_ARG(1)),
6518 RAW_TP_NULL_ARGS(rxrpc_resend, NULL_ARG(2)),
6519 /* sunrpc */
6520 RAW_TP_NULL_ARGS(xs_stream_read_data, NULL_ARG(1)),
6521 /* tcp */
6522 RAW_TP_NULL_ARGS(tcp_send_reset, NULL_ARG(1) | NULL_ARG(2)),
6523 /* tegra_apb_dma */
6524 RAW_TP_NULL_ARGS(tegra_dma_tx_status, NULL_ARG(3)),
6525 /* timer_migration */
6526 RAW_TP_NULL_ARGS(tmigr_update_events, NULL_ARG(1)),
6527 /* writeback, from writeback_folio_template event class */
6528 RAW_TP_NULL_ARGS(writeback_dirty_folio, NULL_ARG(2)),
6529 RAW_TP_NULL_ARGS(folio_wait_writeback, NULL_ARG(2)),
6530 };
6531
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-12-12 11:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-11 2:01 [PATCH bpf v1 0/4] Explicit raw_tp NULL arguments Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 1/4] bpf: Revert "bpf: Mark raw_tp arguments with PTR_MAYBE_NULL" Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 2/4] selftests/bpf: Revert "selftests/bpf: Add tests for raw_tp null handling" Kumar Kartikeya Dwivedi
2024-12-11 2:01 ` [PATCH bpf v1 3/4] bpf: Augment raw_tp arguments with PTR_MAYBE_NULL Kumar Kartikeya Dwivedi
2024-12-11 12:24 ` Jiri Olsa
2024-12-11 14:56 ` Jiri Olsa
2024-12-11 15:56 ` Alexei Starovoitov
2024-12-12 11:39 ` kernel test robot
2024-12-11 2:01 ` [PATCH bpf v1 4/4] selftests/bpf: Add autogenerated tests for raw_tp NULL args Kumar Kartikeya Dwivedi
2024-12-11 16:02 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox