* [PATCH bpf-next v1 1/8] bpf: make makr_btf_ld_reg return error for unexpected reg types
2025-07-02 22:42 [PATCH bpf-next v1 0/8] bpf: additional use-cases for untrusted PTR_TO_MEM Eduard Zingerman
@ 2025-07-02 22:42 ` Eduard Zingerman
2025-07-04 17:20 ` Kumar Kartikeya Dwivedi
2025-07-02 22:42 ` [PATCH bpf-next v1 2/8] bpf: rdonly_untrusted_mem for btf id walk pointer leafs Eduard Zingerman
` (6 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-02 22:42 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
Non-functional change:
mark_btf_ld_reg() expects 'reg_type' parameter to be either
SCALAR_VALUE or PTR_TO_BTF_ID. Next commit expands this set, so update
this function to fail if unexpected type is passed. Also update
callers to propagate the error.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
kernel/bpf/verifier.c | 59 ++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 20 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 52e36fd23f40..b6d26e8bd767 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2795,22 +2795,28 @@ static void mark_reg_not_init(struct bpf_verifier_env *env,
__mark_reg_not_init(env, regs + regno);
}
-static void mark_btf_ld_reg(struct bpf_verifier_env *env,
- struct bpf_reg_state *regs, u32 regno,
- enum bpf_reg_type reg_type,
- struct btf *btf, u32 btf_id,
- enum bpf_type_flag flag)
+static int mark_btf_ld_reg(struct bpf_verifier_env *env,
+ struct bpf_reg_state *regs, u32 regno,
+ enum bpf_reg_type reg_type,
+ struct btf *btf, u32 btf_id,
+ enum bpf_type_flag flag)
{
- if (reg_type == SCALAR_VALUE) {
+ switch (reg_type) {
+ case SCALAR_VALUE:
mark_reg_unknown(env, regs, regno);
- return;
+ return 0;
+ case PTR_TO_BTF_ID:
+ mark_reg_known_zero(env, regs, regno);
+ regs[regno].type = PTR_TO_BTF_ID | flag;
+ regs[regno].btf = btf;
+ regs[regno].btf_id = btf_id;
+ if (type_may_be_null(flag))
+ regs[regno].id = ++env->id_gen;
+ return 0;
+ default:
+ verifier_bug(env, "unexpected reg_type %d in %s\n", reg_type, __func__);
+ return -EFAULT;
}
- mark_reg_known_zero(env, regs, regno);
- regs[regno].type = PTR_TO_BTF_ID | flag;
- regs[regno].btf = btf;
- regs[regno].btf_id = btf_id;
- if (type_may_be_null(flag))
- regs[regno].id = ++env->id_gen;
}
#define DEF_NOT_SUBREG (0)
@@ -5964,6 +5970,7 @@ static int check_map_kptr_access(struct bpf_verifier_env *env, u32 regno,
struct bpf_insn *insn = &env->prog->insnsi[insn_idx];
int class = BPF_CLASS(insn->code);
struct bpf_reg_state *val_reg;
+ int ret;
/* Things we already checked for in check_map_access and caller:
* - Reject cases where variable offset may touch kptr
@@ -5997,8 +6004,11 @@ static int check_map_kptr_access(struct bpf_verifier_env *env, u32 regno,
/* We can simply mark the value_regno receiving the pointer
* value from map as PTR_TO_BTF_ID, with the correct type.
*/
- mark_btf_ld_reg(env, cur_regs(env), value_regno, PTR_TO_BTF_ID, kptr_field->kptr.btf,
- kptr_field->kptr.btf_id, btf_ld_kptr_type(env, kptr_field));
+ ret = mark_btf_ld_reg(env, cur_regs(env), value_regno, PTR_TO_BTF_ID,
+ kptr_field->kptr.btf, kptr_field->kptr.btf_id,
+ btf_ld_kptr_type(env, kptr_field));
+ if (ret < 0)
+ return ret;
} else if (class == BPF_STX) {
val_reg = reg_state(env, value_regno);
if (!register_is_null(val_reg) &&
@@ -7297,8 +7307,11 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
clear_trusted_flags(&flag);
}
- if (atype == BPF_READ && value_regno >= 0)
- mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, flag);
+ if (atype == BPF_READ && value_regno >= 0) {
+ ret = mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, flag);
+ if (ret < 0)
+ return ret;
+ }
return 0;
}
@@ -7352,13 +7365,19 @@ static int check_ptr_to_map_access(struct bpf_verifier_env *env,
/* Simulate access to a PTR_TO_BTF_ID */
memset(&map_reg, 0, sizeof(map_reg));
- mark_btf_ld_reg(env, &map_reg, 0, PTR_TO_BTF_ID, btf_vmlinux, *map->ops->map_btf_id, 0);
+ ret = mark_btf_ld_reg(env, &map_reg, 0, PTR_TO_BTF_ID,
+ btf_vmlinux, *map->ops->map_btf_id, 0);
+ if (ret < 0)
+ return ret;
ret = btf_struct_access(&env->log, &map_reg, off, size, atype, &btf_id, &flag, NULL);
if (ret < 0)
return ret;
- if (value_regno >= 0)
- mark_btf_ld_reg(env, regs, value_regno, ret, btf_vmlinux, btf_id, flag);
+ if (value_regno >= 0) {
+ ret = mark_btf_ld_reg(env, regs, value_regno, ret, btf_vmlinux, btf_id, flag);
+ if (ret < 0)
+ return ret;
+ }
return 0;
}
--
2.47.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 1/8] bpf: make makr_btf_ld_reg return error for unexpected reg types
2025-07-02 22:42 ` [PATCH bpf-next v1 1/8] bpf: make makr_btf_ld_reg return error for unexpected reg types Eduard Zingerman
@ 2025-07-04 17:20 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 17:20 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song
On Thu, 3 Jul 2025 at 00:47, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Non-functional change:
> mark_btf_ld_reg() expects 'reg_type' parameter to be either
> SCALAR_VALUE or PTR_TO_BTF_ID. Next commit expands this set, so update
> this function to fail if unexpected type is passed. Also update
> callers to propagate the error.
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> [...]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH bpf-next v1 2/8] bpf: rdonly_untrusted_mem for btf id walk pointer leafs
2025-07-02 22:42 [PATCH bpf-next v1 0/8] bpf: additional use-cases for untrusted PTR_TO_MEM Eduard Zingerman
2025-07-02 22:42 ` [PATCH bpf-next v1 1/8] bpf: make makr_btf_ld_reg return error for unexpected reg types Eduard Zingerman
@ 2025-07-02 22:42 ` Eduard Zingerman
2025-07-04 17:27 ` Kumar Kartikeya Dwivedi
2025-07-02 22:42 ` [PATCH bpf-next v1 3/8] selftests/bpf: ptr_to_btf_id struct walk ending with primitive pointer Eduard Zingerman
` (5 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-02 22:42 UTC (permalink / raw)
To: bpf, ast, andrii
Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87,
Alexei Starovoitov
When processing a load from a PTR_TO_BTF_ID, the verifier calculates
the type of the loaded structure field based on the load offset.
For example, given the following types:
struct foo {
struct foo *a;
int *b;
} *p;
The verifier would calculate the type of `p->a` as a pointer to
`struct foo`. However, the type of `p->b` is currently calculated as a
SCALAR_VALUE.
This commit updates the logic for processing PTR_TO_BTF_ID to instead
calculate the type of p->b as PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED.
This change allows further dereferencing of such pointers (using probe
memory instructions).
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
kernel/bpf/btf.c | 6 ++++++
kernel/bpf/verifier.c | 5 +++++
tools/testing/selftests/bpf/prog_tests/linked_list.c | 2 +-
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 05fd64a371af..b3c8a95d38fb 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6915,6 +6915,7 @@ enum bpf_struct_walk_result {
/* < 0 error */
WALK_SCALAR = 0,
WALK_PTR,
+ WALK_PTR_UNTRUSTED,
WALK_STRUCT,
};
@@ -7156,6 +7157,8 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
*field_name = mname;
return WALK_PTR;
}
+
+ return WALK_PTR_UNTRUSTED;
}
/* Allow more flexible access within an int as long as
@@ -7228,6 +7231,9 @@ int btf_struct_access(struct bpf_verifier_log *log,
*next_btf_id = id;
*flag = tmp_flag;
return PTR_TO_BTF_ID;
+ case WALK_PTR_UNTRUSTED:
+ *flag = MEM_RDONLY | PTR_UNTRUSTED;
+ return PTR_TO_MEM;
case WALK_SCALAR:
return SCALAR_VALUE;
case WALK_STRUCT:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b6d26e8bd767..cd2344e50db8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2813,6 +2813,11 @@ static int mark_btf_ld_reg(struct bpf_verifier_env *env,
if (type_may_be_null(flag))
regs[regno].id = ++env->id_gen;
return 0;
+ case PTR_TO_MEM:
+ mark_reg_known_zero(env, regs, regno);
+ regs[regno].type = PTR_TO_MEM | flag;
+ regs[regno].mem_size = 0;
+ return 0;
default:
verifier_bug(env, "unexpected reg_type %d in %s\n", reg_type, __func__);
return -EFAULT;
diff --git a/tools/testing/selftests/bpf/prog_tests/linked_list.c b/tools/testing/selftests/bpf/prog_tests/linked_list.c
index 5266c7022863..14c5a7ef0e87 100644
--- a/tools/testing/selftests/bpf/prog_tests/linked_list.c
+++ b/tools/testing/selftests/bpf/prog_tests/linked_list.c
@@ -72,7 +72,7 @@ static struct {
{ "new_null_ret", "R0 invalid mem access 'ptr_or_null_'" },
{ "obj_new_acq", "Unreleased reference id=" },
{ "use_after_drop", "invalid mem access 'scalar'" },
- { "ptr_walk_scalar", "type=scalar expected=percpu_ptr_" },
+ { "ptr_walk_scalar", "type=rdonly_untrusted_mem expected=percpu_ptr_" },
{ "direct_read_lock", "direct access to bpf_spin_lock is disallowed" },
{ "direct_write_lock", "direct access to bpf_spin_lock is disallowed" },
{ "direct_read_head", "direct access to bpf_list_head is disallowed" },
--
2.47.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 2/8] bpf: rdonly_untrusted_mem for btf id walk pointer leafs
2025-07-02 22:42 ` [PATCH bpf-next v1 2/8] bpf: rdonly_untrusted_mem for btf id walk pointer leafs Eduard Zingerman
@ 2025-07-04 17:27 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 17:27 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On Thu, 3 Jul 2025 at 00:42, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> When processing a load from a PTR_TO_BTF_ID, the verifier calculates
> the type of the loaded structure field based on the load offset.
> For example, given the following types:
>
> struct foo {
> struct foo *a;
> int *b;
> } *p;
>
> The verifier would calculate the type of `p->a` as a pointer to
> `struct foo`. However, the type of `p->b` is currently calculated as a
> SCALAR_VALUE.
>
> This commit updates the logic for processing PTR_TO_BTF_ID to instead
> calculate the type of p->b as PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED.
> This change allows further dereferencing of such pointers (using probe
> memory instructions).
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> [...]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH bpf-next v1 3/8] selftests/bpf: ptr_to_btf_id struct walk ending with primitive pointer
2025-07-02 22:42 [PATCH bpf-next v1 0/8] bpf: additional use-cases for untrusted PTR_TO_MEM Eduard Zingerman
2025-07-02 22:42 ` [PATCH bpf-next v1 1/8] bpf: make makr_btf_ld_reg return error for unexpected reg types Eduard Zingerman
2025-07-02 22:42 ` [PATCH bpf-next v1 2/8] bpf: rdonly_untrusted_mem for btf id walk pointer leafs Eduard Zingerman
@ 2025-07-02 22:42 ` Eduard Zingerman
2025-07-04 17:34 ` Kumar Kartikeya Dwivedi
2025-07-02 22:42 ` [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters Eduard Zingerman
` (4 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-02 22:42 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
Validate that reading a PTR_TO_BTF_ID field produces a value of type
PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED, if field is a pointer to a
primitive type.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../bpf/progs/mem_rdonly_untrusted.c | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
index 8185130ede95..4f94c971ae86 100644
--- a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
+++ b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
@@ -5,6 +5,37 @@
#include "bpf_misc.h"
#include "../test_kmods/bpf_testmod_kfunc.h"
+SEC("tp_btf/sys_enter")
+__success
+__log_level(2)
+__msg("r8 = *(u64 *)(r7 +0) ; R7_w=ptr_nameidata(off={{[0-9]+}}) R8_w=rdonly_untrusted_mem(sz=0)")
+__msg("r9 = *(u8 *)(r8 +0) ; R8_w=rdonly_untrusted_mem(sz=0) R9_w=scalar")
+int btf_id_to_ptr_mem(void *ctx)
+{
+ struct task_struct *task;
+ struct nameidata *idata;
+ u64 ret, off;
+
+ task = bpf_get_current_task_btf();
+ idata = task->nameidata;
+ off = bpf_core_field_offset(struct nameidata, pathname);
+ /*
+ * asm block to have reliable match target for __msg, equivalent of:
+ * ret = task->nameidata->pathname[0];
+ */
+ asm volatile (
+ "r7 = %[idata];"
+ "r7 += %[off];"
+ "r8 = *(u64 *)(r7 + 0);"
+ "r9 = *(u8 *)(r8 + 0);"
+ "%[ret] = r9;"
+ : [ret]"=r"(ret)
+ : [idata]"r"(idata),
+ [off]"r"(off)
+ : "r7", "r8", "r9");
+ return ret;
+}
+
SEC("socket")
__success
__retval(0)
--
2.47.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 3/8] selftests/bpf: ptr_to_btf_id struct walk ending with primitive pointer
2025-07-02 22:42 ` [PATCH bpf-next v1 3/8] selftests/bpf: ptr_to_btf_id struct walk ending with primitive pointer Eduard Zingerman
@ 2025-07-04 17:34 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 17:34 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song
On Thu, 3 Jul 2025 at 00:42, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Validate that reading a PTR_TO_BTF_ID field produces a value of type
> PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED, if field is a pointer to a
> primitive type.
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> [...]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-02 22:42 [PATCH bpf-next v1 0/8] bpf: additional use-cases for untrusted PTR_TO_MEM Eduard Zingerman
` (2 preceding siblings ...)
2025-07-02 22:42 ` [PATCH bpf-next v1 3/8] selftests/bpf: ptr_to_btf_id struct walk ending with primitive pointer Eduard Zingerman
@ 2025-07-02 22:42 ` Eduard Zingerman
2025-07-03 3:18 ` Alexei Starovoitov
2025-07-04 18:03 ` Kumar Kartikeya Dwivedi
2025-07-02 22:42 ` [PATCH bpf-next v1 5/8] libbpf: __arg_untrusted in bpf_helpers.h Eduard Zingerman
` (3 subsequent siblings)
7 siblings, 2 replies; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-02 22:42 UTC (permalink / raw)
To: bpf, ast, andrii
Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87,
Alexei Starovoitov
Add support for PTR_TO_BTF_ID | PTR_UNTRUSTED global function
parameters. Anything is allowed to pass to such parameters, as these
are read-only and probe read instructions would protect against
invalid memory access.
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
kernel/bpf/btf.c | 29 ++++++++++++++++++++++++-----
kernel/bpf/verifier.c | 7 +++++++
2 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index b3c8a95d38fb..28cb0a2a5402 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7646,11 +7646,12 @@ static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx,
}
enum btf_arg_tag {
- ARG_TAG_CTX = BIT_ULL(0),
- ARG_TAG_NONNULL = BIT_ULL(1),
- ARG_TAG_TRUSTED = BIT_ULL(2),
- ARG_TAG_NULLABLE = BIT_ULL(3),
- ARG_TAG_ARENA = BIT_ULL(4),
+ ARG_TAG_CTX = BIT_ULL(0),
+ ARG_TAG_NONNULL = BIT_ULL(1),
+ ARG_TAG_TRUSTED = BIT_ULL(2),
+ ARG_TAG_UNTRUSTED = BIT_ULL(3),
+ ARG_TAG_NULLABLE = BIT_ULL(4),
+ ARG_TAG_ARENA = BIT_ULL(5),
};
/* Process BTF of a function to produce high-level expectation of function
@@ -7758,6 +7759,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
tags |= ARG_TAG_CTX;
} else if (strcmp(tag, "trusted") == 0) {
tags |= ARG_TAG_TRUSTED;
+ } else if (strcmp(tag, "untrusted") == 0) {
+ tags |= ARG_TAG_UNTRUSTED;
} else if (strcmp(tag, "nonnull") == 0) {
tags |= ARG_TAG_NONNULL;
} else if (strcmp(tag, "nullable") == 0) {
@@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
sub->args[i].btf_id = kern_type_id;
continue;
}
+ if (tags & ARG_TAG_UNTRUSTED) {
+ int kern_type_id;
+
+ if (tags & ~ARG_TAG_UNTRUSTED) {
+ bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
+ return -EINVAL;
+ }
+
+ kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
+ if (kern_type_id < 0)
+ return kern_type_id;
+
+ sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
+ sub->args[i].btf_id = kern_type_id;
+ continue;
+ }
if (tags & ARG_TAG_ARENA) {
if (tags & ~ARG_TAG_ARENA) {
bpf_log(log, "arg#%d arena cannot be combined with any other tags\n", i);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index cd2344e50db8..dfb5a2f8e58f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10436,6 +10436,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
bpf_log(log, "R%d is not a scalar\n", regno);
return -EINVAL;
}
+ } else if (arg->arg_type & PTR_UNTRUSTED) {
+ /*
+ * Anything is allowed for untrusted arguments, as these are
+ * read-only and probe read instructions would protect against
+ * invalid memory access.
+ */
+ continue;
} else if (arg->arg_type == ARG_PTR_TO_CTX) {
ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
if (ret < 0)
--
2.47.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-02 22:42 ` [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters Eduard Zingerman
@ 2025-07-03 3:18 ` Alexei Starovoitov
2025-07-03 21:25 ` Eduard Zingerman
2025-07-04 18:03 ` Kumar Kartikeya Dwivedi
1 sibling, 1 reply; 34+ messages in thread
From: Alexei Starovoitov @ 2025-07-03 3:18 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Wed, Jul 2, 2025 at 3:42 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Add support for PTR_TO_BTF_ID | PTR_UNTRUSTED global function
> parameters. Anything is allowed to pass to such parameters, as these
> are read-only and probe read instructions would protect against
> invalid memory access.
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
> kernel/bpf/btf.c | 29 ++++++++++++++++++++++++-----
> kernel/bpf/verifier.c | 7 +++++++
> 2 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index b3c8a95d38fb..28cb0a2a5402 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7646,11 +7646,12 @@ static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx,
> }
>
> enum btf_arg_tag {
> - ARG_TAG_CTX = BIT_ULL(0),
> - ARG_TAG_NONNULL = BIT_ULL(1),
> - ARG_TAG_TRUSTED = BIT_ULL(2),
> - ARG_TAG_NULLABLE = BIT_ULL(3),
> - ARG_TAG_ARENA = BIT_ULL(4),
> + ARG_TAG_CTX = BIT_ULL(0),
> + ARG_TAG_NONNULL = BIT_ULL(1),
> + ARG_TAG_TRUSTED = BIT_ULL(2),
> + ARG_TAG_UNTRUSTED = BIT_ULL(3),
> + ARG_TAG_NULLABLE = BIT_ULL(4),
> + ARG_TAG_ARENA = BIT_ULL(5),
> };
>
> /* Process BTF of a function to produce high-level expectation of function
> @@ -7758,6 +7759,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> tags |= ARG_TAG_CTX;
> } else if (strcmp(tag, "trusted") == 0) {
> tags |= ARG_TAG_TRUSTED;
> + } else if (strcmp(tag, "untrusted") == 0) {
> + tags |= ARG_TAG_UNTRUSTED;
> } else if (strcmp(tag, "nonnull") == 0) {
> tags |= ARG_TAG_NONNULL;
> } else if (strcmp(tag, "nullable") == 0) {
> @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> sub->args[i].btf_id = kern_type_id;
> continue;
> }
> + if (tags & ARG_TAG_UNTRUSTED) {
> + int kern_type_id;
> +
> + if (tags & ~ARG_TAG_UNTRUSTED) {
> + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> + return -EINVAL;
> + }
> +
> + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
> + if (kern_type_id < 0)
> + return kern_type_id;
> +
> + sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
> + sub->args[i].btf_id = kern_type_id;
> + continue;
> + }
Looking at this hunk standalone (without patch 7) one might get
an impression that odd ptr_to_btf_id is allowed that points
to non-struct type,
but patch 7 sort-of fixes it by handling primitive types first.
Still, I think it would be good to add a check here that kern_type_id
is a struct kind.
> if (tags & ARG_TAG_ARENA) {
> if (tags & ~ARG_TAG_ARENA) {
> bpf_log(log, "arg#%d arena cannot be combined with any other tags\n", i);
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index cd2344e50db8..dfb5a2f8e58f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10436,6 +10436,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
> bpf_log(log, "R%d is not a scalar\n", regno);
> return -EINVAL;
> }
> + } else if (arg->arg_type & PTR_UNTRUSTED) {
> + /*
> + * Anything is allowed for untrusted arguments, as these are
> + * read-only and probe read instructions would protect against
> + * invalid memory access.
> + */
> + continue;
nit: All except one 'else if' in this loop don't do explicit 'continue'.
I think the above comment would be enough.
> } else if (arg->arg_type == ARG_PTR_TO_CTX) {
> ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
> if (ret < 0)
> --
> 2.47.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-03 3:18 ` Alexei Starovoitov
@ 2025-07-03 21:25 ` Eduard Zingerman
2025-07-04 18:17 ` Alexei Starovoitov
0 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-03 21:25 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Wed, 2025-07-02 at 20:18 -0700, Alexei Starovoitov wrote:
> On Wed, Jul 2, 2025 at 3:42 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
[...]
> > @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> > sub->args[i].btf_id = kern_type_id;
> > continue;
> > }
> > + if (tags & ARG_TAG_UNTRUSTED) {
> > + int kern_type_id;
> > +
> > + if (tags & ~ARG_TAG_UNTRUSTED) {
> > + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> > + return -EINVAL;
> > + }
> > +
> > + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
> > + if (kern_type_id < 0)
> > + return kern_type_id;
> > +
> > + sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
> > + sub->args[i].btf_id = kern_type_id;
> > + continue;
> > + }
>
> Looking at this hunk standalone (without patch 7) one might get
> an impression that odd ptr_to_btf_id is allowed that points
> to non-struct type,
> but patch 7 sort-of fixes it by handling primitive types first.
>
> Still, I think it would be good to add a check here that kern_type_id
> is a struct kind.
I'm adding this check, but it will go w/o a test:
- unions are allowed by btf_struct_walk, so need to be accepted
- function types are anonymous and candidates search wants types with names
- float -- no candidate in kernel btf
- func/var/datasec -- need a corrupt BTF to sneak these in.
[...]
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-03 21:25 ` Eduard Zingerman
@ 2025-07-04 18:17 ` Alexei Starovoitov
0 siblings, 0 replies; 34+ messages in thread
From: Alexei Starovoitov @ 2025-07-04 18:17 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Thu, Jul 3, 2025 at 2:25 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2025-07-02 at 20:18 -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 2, 2025 at 3:42 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> [...]
>
> > > @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> > > sub->args[i].btf_id = kern_type_id;
> > > continue;
> > > }
> > > + if (tags & ARG_TAG_UNTRUSTED) {
> > > + int kern_type_id;
> > > +
> > > + if (tags & ~ARG_TAG_UNTRUSTED) {
> > > + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
> > > + if (kern_type_id < 0)
> > > + return kern_type_id;
> > > +
> > > + sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
> > > + sub->args[i].btf_id = kern_type_id;
> > > + continue;
> > > + }
> >
> > Looking at this hunk standalone (without patch 7) one might get
> > an impression that odd ptr_to_btf_id is allowed that points
> > to non-struct type,
> > but patch 7 sort-of fixes it by handling primitive types first.
> >
> > Still, I think it would be good to add a check here that kern_type_id
> > is a struct kind.
>
> I'm adding this check, but it will go w/o a test:
> - unions are allowed by btf_struct_walk, so need to be accepted
Of course, by "checking a struct kind" I meant btf_type_is_struct()
which does kind == BTF_KIND_STRUCT || kind == BTF_KIND_UNION.
> - function types are anonymous and candidates search wants types with names
> - float -- no candidate in kernel btf
> - func/var/datasec -- need a corrupt BTF to sneak these in.
You're probably right, but extra "if (btf_type_is_struct(..."
just to be safe is imo worth it. syzbot-s and such.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-02 22:42 ` [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters Eduard Zingerman
2025-07-03 3:18 ` Alexei Starovoitov
@ 2025-07-04 18:03 ` Kumar Kartikeya Dwivedi
2025-07-04 18:28 ` Eduard Zingerman
1 sibling, 1 reply; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 18:03 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On Thu, 3 Jul 2025 at 00:42, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Add support for PTR_TO_BTF_ID | PTR_UNTRUSTED global function
> parameters. Anything is allowed to pass to such parameters, as these
> are read-only and probe read instructions would protect against
> invalid memory access.
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Some comments below, but logic looks correct.
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> kernel/bpf/btf.c | 29 ++++++++++++++++++++++++-----
> kernel/bpf/verifier.c | 7 +++++++
> 2 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index b3c8a95d38fb..28cb0a2a5402 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7646,11 +7646,12 @@ static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx,
> }
>
> enum btf_arg_tag {
> - ARG_TAG_CTX = BIT_ULL(0),
> - ARG_TAG_NONNULL = BIT_ULL(1),
> - ARG_TAG_TRUSTED = BIT_ULL(2),
> - ARG_TAG_NULLABLE = BIT_ULL(3),
> - ARG_TAG_ARENA = BIT_ULL(4),
> + ARG_TAG_CTX = BIT_ULL(0),
> + ARG_TAG_NONNULL = BIT_ULL(1),
> + ARG_TAG_TRUSTED = BIT_ULL(2),
> + ARG_TAG_UNTRUSTED = BIT_ULL(3),
> + ARG_TAG_NULLABLE = BIT_ULL(4),
> + ARG_TAG_ARENA = BIT_ULL(5),
> };
>
> /* Process BTF of a function to produce high-level expectation of function
> @@ -7758,6 +7759,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> tags |= ARG_TAG_CTX;
> } else if (strcmp(tag, "trusted") == 0) {
> tags |= ARG_TAG_TRUSTED;
> + } else if (strcmp(tag, "untrusted") == 0) {
> + tags |= ARG_TAG_UNTRUSTED;
> } else if (strcmp(tag, "nonnull") == 0) {
> tags |= ARG_TAG_NONNULL;
> } else if (strcmp(tag, "nullable") == 0) {
> @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> sub->args[i].btf_id = kern_type_id;
> continue;
> }
> + if (tags & ARG_TAG_UNTRUSTED) {
> + int kern_type_id;
> +
> + if (tags & ~ARG_TAG_UNTRUSTED) {
> + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> + return -EINVAL;
> + }
> +
> + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
So while this makes sense for trusted, I think for untrusted, we
should allow types in program BTF as well.
This is one of the things I think lacks in bpf_rdonly_cast as well, to
be able to cast to types in program BTF.
Say you want to reinterpret some kernel memory into your own type and
access it using a struct in the program which is a different type.
I think it makes sense to make this work.
When I needed it in the past I just added a local new
bpf_rdonly_cast_local variant that uses prog->btf for btf_id and moved
on.
Supporting bpf_core_cast for both prog BTF and kernel BTF types is not
trivial because we cannot disambiguate local vs kernel types.
IIRC module BTF types probably don't work either but that's a different story.
> + if (kern_type_id < 0)
> + return kern_type_id;
> +
> + sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
> + sub->args[i].btf_id = kern_type_id;
> + continue;
> + }
> if (tags & ARG_TAG_ARENA) {
> if (tags & ~ARG_TAG_ARENA) {
> bpf_log(log, "arg#%d arena cannot be combined with any other tags\n", i);
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index cd2344e50db8..dfb5a2f8e58f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10436,6 +10436,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
> bpf_log(log, "R%d is not a scalar\n", regno);
> return -EINVAL;
> }
> + } else if (arg->arg_type & PTR_UNTRUSTED) {
> + /*
> + * Anything is allowed for untrusted arguments, as these are
> + * read-only and probe read instructions would protect against
> + * invalid memory access.
> + */
> + continue;
> } else if (arg->arg_type == ARG_PTR_TO_CTX) {
> ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
> if (ret < 0)
> --
> 2.47.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 18:03 ` Kumar Kartikeya Dwivedi
@ 2025-07-04 18:28 ` Eduard Zingerman
2025-07-04 18:33 ` Eduard Zingerman
0 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-04 18:28 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On Fri, 2025-07-04 at 20:03 +0200, Kumar Kartikeya Dwivedi wrote:
[...]
> > @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> > sub->args[i].btf_id = kern_type_id;
> > continue;
> > }
> > + if (tags & ARG_TAG_UNTRUSTED) {
> > + int kern_type_id;
> > +
> > + if (tags & ~ARG_TAG_UNTRUSTED) {
> > + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> > + return -EINVAL;
> > + }
> > +
> > + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
>
> So while this makes sense for trusted, I think for untrusted, we
> should allow types in program BTF as well.
> This is one of the things I think lacks in bpf_rdonly_cast as well, to
> be able to cast to types in program BTF.
> Say you want to reinterpret some kernel memory into your own type and
> access it using a struct in the program which is a different type.
> I think it makes sense to make this work.
Hi Kumar,
Thank you for the review.
Allowing local program BTF makes sense to me.
I assume we should first search in kernel BTF and fallback to program
BTF if nothing found. This way verifier might catch a program
accessing kernel data structure but having wrong assumptions about
field offsets (not using CO-RE). On the other hand, this might get
confusing if there is an accidental conflict between kernel data
structure name and program local data structure name.
Mechanically this would be a slightly bigger change,
as btf_prepare_func_args() does not record which BTF was used for a
parameter and always assumes vmlinux BTF.
> When I needed it in the past I just added a local new
> bpf_rdonly_cast_local variant that uses prog->btf for btf_id and moved
> on.
>
> Supporting bpf_core_cast for both prog BTF and kernel BTF types is not
> trivial because we cannot disambiguate local vs kernel types.
> IIRC module BTF types probably don't work either but that's a different story.
I can add bpf_rdonly_cast_local() as a followup, do you remember
context in which you needed this?
[...]
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 18:28 ` Eduard Zingerman
@ 2025-07-04 18:33 ` Eduard Zingerman
2025-07-04 18:50 ` Kumar Kartikeya Dwivedi
0 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-04 18:33 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On Fri, 2025-07-04 at 11:28 -0700, Eduard Zingerman wrote:
> On Fri, 2025-07-04 at 20:03 +0200, Kumar Kartikeya Dwivedi wrote:
>
> [...]
>
> > > @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> > > sub->args[i].btf_id = kern_type_id;
> > > continue;
> > > }
> > > + if (tags & ARG_TAG_UNTRUSTED) {
> > > + int kern_type_id;
> > > +
> > > + if (tags & ~ARG_TAG_UNTRUSTED) {
> > > + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
> >
> > So while this makes sense for trusted, I think for untrusted, we
> > should allow types in program BTF as well.
> > This is one of the things I think lacks in bpf_rdonly_cast as well, to
> > be able to cast to types in program BTF.
> > Say you want to reinterpret some kernel memory into your own type and
> > access it using a struct in the program which is a different type.
> > I think it makes sense to make this work.
>
> Hi Kumar,
>
> Thank you for the review.
> Allowing local program BTF makes sense to me.
> I assume we should first search in kernel BTF and fallback to program
> BTF if nothing found. This way verifier might catch a program
> accessing kernel data structure but having wrong assumptions about
> field offsets (not using CO-RE). On the other hand, this might get
> confusing if there is an accidental conflict between kernel data
> structure name and program local data structure name.
Maybe just add __arg_untrusted_local and avoid ambiguity?
[...]
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 18:33 ` Eduard Zingerman
@ 2025-07-04 18:50 ` Kumar Kartikeya Dwivedi
2025-07-04 19:07 ` Eduard Zingerman
0 siblings, 1 reply; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 18:50 UTC (permalink / raw)
To: Eduard Zingerman, Matt Bobrowski
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On Fri, 4 Jul 2025 at 20:33, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2025-07-04 at 11:28 -0700, Eduard Zingerman wrote:
> > On Fri, 2025-07-04 at 20:03 +0200, Kumar Kartikeya Dwivedi wrote:
> >
> > [...]
> >
> > > > @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> > > > sub->args[i].btf_id = kern_type_id;
> > > > continue;
> > > > }
> > > > + if (tags & ARG_TAG_UNTRUSTED) {
> > > > + int kern_type_id;
> > > > +
> > > > + if (tags & ~ARG_TAG_UNTRUSTED) {
> > > > + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
> > >
> > > So while this makes sense for trusted, I think for untrusted, we
> > > should allow types in program BTF as well.
> > > This is one of the things I think lacks in bpf_rdonly_cast as well, to
> > > be able to cast to types in program BTF.
> > > Say you want to reinterpret some kernel memory into your own type and
> > > access it using a struct in the program which is a different type.
> > > I think it makes sense to make this work.
> >
> > Hi Kumar,
> >
> > Thank you for the review.
> > Allowing local program BTF makes sense to me.
> > I assume we should first search in kernel BTF and fallback to program
> > BTF if nothing found. This way verifier might catch a program
> > accessing kernel data structure but having wrong assumptions about
> > field offsets (not using CO-RE). On the other hand, this might get
> > confusing if there is an accidental conflict between kernel data
> > structure name and program local data structure name.
>
> Maybe just add __arg_untrusted_local and avoid ambiguity?
That might be less ambiguous, sure. But I don't see why the fallback
would be confusing.
It might be nice if we can support it without asking users to learn
about the difference between the two tags, but if it's too ugly we can
go with explicit local tag.
A user can have a struct without preserve_access_index now and having
the same name as the kernel struct, and the program will load things
at potentially wrong offsets.
If the same type exists, the program would fail compilation in C due
to duplicate types.
Are there any other cases where it might be a footgun that you anticipate?
>
> > Supporting bpf_core_cast for both prog BTF and kernel BTF types is not
> > trivial because we cannot disambiguate local vs kernel types.
> > IIRC module BTF types probably don't work either but that's a different story.
> I can add bpf_rdonly_cast_local() as a followup, do you remember
> context in which you needed this?
Adding Matt.
Not long ago we were discussing iterating over the bpf linked list
since support doesn't exist in the kernel and it was safe in the
specific context to iterate over the list.
Ofcourse, we could add iteration support in the kernel, but another
approach would be the ability to subtract offset from node to arrive
at an untrusted pointer to type in prog BTF (that was allocated using
bpf_obj_new).
But bpf_core_cast didn't work there, so we ended up discarding that approach.
Not to get hung up on this specific example, but I think it would be
useful in general.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 18:50 ` Kumar Kartikeya Dwivedi
@ 2025-07-04 19:07 ` Eduard Zingerman
2025-07-04 19:15 ` Kumar Kartikeya Dwivedi
0 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-04 19:07 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi, Matt Bobrowski
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On Fri, 2025-07-04 at 20:50 +0200, Kumar Kartikeya Dwivedi wrote:
> On Fri, 4 Jul 2025 at 20:33, Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Fri, 2025-07-04 at 11:28 -0700, Eduard Zingerman wrote:
> > > On Fri, 2025-07-04 at 20:03 +0200, Kumar Kartikeya Dwivedi wrote:
> > >
> > > [...]
> > >
> > > > > @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> > > > > sub->args[i].btf_id = kern_type_id;
> > > > > continue;
> > > > > }
> > > > > + if (tags & ARG_TAG_UNTRUSTED) {
> > > > > + int kern_type_id;
> > > > > +
> > > > > + if (tags & ~ARG_TAG_UNTRUSTED) {
> > > > > + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
> > > >
> > > > So while this makes sense for trusted, I think for untrusted, we
> > > > should allow types in program BTF as well.
> > > > This is one of the things I think lacks in bpf_rdonly_cast as well, to
> > > > be able to cast to types in program BTF.
> > > > Say you want to reinterpret some kernel memory into your own type and
> > > > access it using a struct in the program which is a different type.
> > > > I think it makes sense to make this work.
> > >
> > > Hi Kumar,
> > >
> > > Thank you for the review.
> > > Allowing local program BTF makes sense to me.
> > > I assume we should first search in kernel BTF and fallback to program
> > > BTF if nothing found. This way verifier might catch a program
> > > accessing kernel data structure but having wrong assumptions about
> > > field offsets (not using CO-RE). On the other hand, this might get
> > > confusing if there is an accidental conflict between kernel data
> > > structure name and program local data structure name.
> >
> > Maybe just add __arg_untrusted_local and avoid ambiguity?
>
> That might be less ambiguous, sure. But I don't see why the fallback
> would be confusing.
> It might be nice if we can support it without asking users to learn
> about the difference between the two tags, but if it's too ugly we can
> go with explicit local tag.
> A user can have a struct without preserve_access_index now and having
> the same name as the kernel struct, and the program will load things
> at potentially wrong offsets.
> If the same type exists, the program would fail compilation in C due
> to duplicate types.
> Are there any other cases where it might be a footgun that you anticipate?
Well, basically two cases assuming that program does not use vmlinux.h:
struct kernel_type { // assume kernel type has 'i' at another offset
int *i;
};
__weak int global(struct kernel_type *p __arg_untrusted) {
return p->i[7]; // assume no CO-RE relocation for &p->i
}
In this case, if kernel BTF is searched first verifier can catch that
access to 'i' is bogus. However, that is not necessary if one assumes
that verifier should only check for errors that can bring down the
kernel.
Another case:
// assume there is kernel type with the same name, but program
// author does not know about that and does not use vmlinux.h,
// thus avoiding compilation error.
struct accidental_kernel_type {
int *i;
};
__weak int global(struct accidental_kernel_type *p __arg_untrusted) {
return p->i[7];
}
In this case user would not expect any errors at load time.
Hm.
Maybe just always use program BTF?
> > > Supporting bpf_core_cast for both prog BTF and kernel BTF types is not
> > > trivial because we cannot disambiguate local vs kernel types.
> > > IIRC module BTF types probably don't work either but that's a different story.
>
> > I can add bpf_rdonly_cast_local() as a followup, do you remember
> > context in which you needed this?
>
> Adding Matt.
>
> Not long ago we were discussing iterating over the bpf linked list
> since support doesn't exist in the kernel and it was safe in the
> specific context to iterate over the list.
> Ofcourse, we could add iteration support in the kernel, but another
> approach would be the ability to subtract offset from node to arrive
> at an untrusted pointer to type in prog BTF (that was allocated using
> bpf_obj_new).
> But bpf_core_cast didn't work there, so we ended up discarding that approach.
> Not to get hung up on this specific example, but I think it would be
> useful in general.
I see, makes sense.
That should be possible now using bpf_rdonly_cast(..., 0) but one
would need to explicitly cast each pointer in this way.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 19:07 ` Eduard Zingerman
@ 2025-07-04 19:15 ` Kumar Kartikeya Dwivedi
2025-07-04 19:23 ` Eduard Zingerman
0 siblings, 1 reply; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 19:15 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Matt Bobrowski, bpf, ast, andrii, daniel, martin.lau, kernel-team,
yonghong.song, Alexei Starovoitov
On Fri, 4 Jul 2025 at 21:07, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2025-07-04 at 20:50 +0200, Kumar Kartikeya Dwivedi wrote:
> > On Fri, 4 Jul 2025 at 20:33, Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > On Fri, 2025-07-04 at 11:28 -0700, Eduard Zingerman wrote:
> > > > On Fri, 2025-07-04 at 20:03 +0200, Kumar Kartikeya Dwivedi wrote:
> > > >
> > > > [...]
> > > >
> > > > > > @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> > > > > > sub->args[i].btf_id = kern_type_id;
> > > > > > continue;
> > > > > > }
> > > > > > + if (tags & ARG_TAG_UNTRUSTED) {
> > > > > > + int kern_type_id;
> > > > > > +
> > > > > > + if (tags & ~ARG_TAG_UNTRUSTED) {
> > > > > > + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> > > > > > + return -EINVAL;
> > > > > > + }
> > > > > > +
> > > > > > + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
> > > > >
> > > > > So while this makes sense for trusted, I think for untrusted, we
> > > > > should allow types in program BTF as well.
> > > > > This is one of the things I think lacks in bpf_rdonly_cast as well, to
> > > > > be able to cast to types in program BTF.
> > > > > Say you want to reinterpret some kernel memory into your own type and
> > > > > access it using a struct in the program which is a different type.
> > > > > I think it makes sense to make this work.
> > > >
> > > > Hi Kumar,
> > > >
> > > > Thank you for the review.
> > > > Allowing local program BTF makes sense to me.
> > > > I assume we should first search in kernel BTF and fallback to program
> > > > BTF if nothing found. This way verifier might catch a program
> > > > accessing kernel data structure but having wrong assumptions about
> > > > field offsets (not using CO-RE). On the other hand, this might get
> > > > confusing if there is an accidental conflict between kernel data
> > > > structure name and program local data structure name.
> > >
> > > Maybe just add __arg_untrusted_local and avoid ambiguity?
> >
> > That might be less ambiguous, sure. But I don't see why the fallback
> > would be confusing.
> > It might be nice if we can support it without asking users to learn
> > about the difference between the two tags, but if it's too ugly we can
> > go with explicit local tag.
> > A user can have a struct without preserve_access_index now and having
> > the same name as the kernel struct, and the program will load things
> > at potentially wrong offsets.
> > If the same type exists, the program would fail compilation in C due
> > to duplicate types.
> > Are there any other cases where it might be a footgun that you anticipate?
>
> Well, basically two cases assuming that program does not use vmlinux.h:
>
> struct kernel_type { // assume kernel type has 'i' at another offset
> int *i;
> };
>
> __weak int global(struct kernel_type *p __arg_untrusted) {
> return p->i[7]; // assume no CO-RE relocation for &p->i
> }
>
> In this case, if kernel BTF is searched first verifier can catch that
> access to 'i' is bogus. However, that is not necessary if one assumes
> that verifier should only check for errors that can bring down the
> kernel.
Right, but it's also possible that loading p->i[7] is ok in the actual
kernel type, so the kernel will just load the program but mark the
register as potentially unexpected type.
>
> Another case:
>
> // assume there is kernel type with the same name, but program
> // author does not know about that and does not use vmlinux.h,
> // thus avoiding compilation error.
> struct accidental_kernel_type {
> int *i;
> };
>
> __weak int global(struct accidental_kernel_type *p __arg_untrusted) {
> return p->i[7];
> }
>
> In this case user would not expect any errors at load time.
>
> Hm.
> Maybe just always use program BTF?
Yeah, so if the user specifies a type and has co-re enabled, they're
accessing a kernel struct.
If they're doing it without co-re, it's broken today already, or they
know the struct is fixed in layout somehow so it's ok.
If not, they want to access things at fixed offsets. So we can just
use the type they're using to model untrusted derefs.
So always using prog BTF makes sense to me.
>
> > > > Supporting bpf_core_cast for both prog BTF and kernel BTF types is not
> > > > trivial because we cannot disambiguate local vs kernel types.
> > > > IIRC module BTF types probably don't work either but that's a different story.
> >
> > > I can add bpf_rdonly_cast_local() as a followup, do you remember
> > > context in which you needed this?
> >
> > Adding Matt.
> >
> > Not long ago we were discussing iterating over the bpf linked list
> > since support doesn't exist in the kernel and it was safe in the
> > specific context to iterate over the list.
> > Ofcourse, we could add iteration support in the kernel, but another
> > approach would be the ability to subtract offset from node to arrive
> > at an untrusted pointer to type in prog BTF (that was allocated using
> > bpf_obj_new).
> > But bpf_core_cast didn't work there, so we ended up discarding that approach.
> > Not to get hung up on this specific example, but I think it would be
> > useful in general.
>
> I see, makes sense.
> That should be possible now using bpf_rdonly_cast(..., 0) but one
> would need to explicitly cast each pointer in this way.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 19:15 ` Kumar Kartikeya Dwivedi
@ 2025-07-04 19:23 ` Eduard Zingerman
2025-07-04 20:05 ` Alexei Starovoitov
0 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-04 19:23 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: Matt Bobrowski, bpf, ast, andrii, daniel, martin.lau, kernel-team,
yonghong.song, Alexei Starovoitov
On Fri, 2025-07-04 at 21:15 +0200, Kumar Kartikeya Dwivedi wrote:
[...]
> Yeah, so if the user specifies a type and has co-re enabled, they're
> accessing a kernel struct.
> If they're doing it without co-re, it's broken today already, or they
> know the struct is fixed in layout somehow so it's ok.
> If not, they want to access things at fixed offsets. So we can just
> use the type they're using to model untrusted derefs.
>
> So always using prog BTF makes sense to me.
Ok, I'm switching to always using prog BTF.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 19:23 ` Eduard Zingerman
@ 2025-07-04 20:05 ` Alexei Starovoitov
2025-07-04 20:20 ` Kumar Kartikeya Dwivedi
0 siblings, 1 reply; 34+ messages in thread
From: Alexei Starovoitov @ 2025-07-04 20:05 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Kumar Kartikeya Dwivedi, Matt Bobrowski, bpf, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, Kernel Team,
Yonghong Song
On Fri, Jul 4, 2025 at 12:23 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2025-07-04 at 21:15 +0200, Kumar Kartikeya Dwivedi wrote:
>
> [...]
>
> > Yeah, so if the user specifies a type and has co-re enabled, they're
> > accessing a kernel struct.
> > If they're doing it without co-re, it's broken today already, or they
> > know the struct is fixed in layout somehow so it's ok.
> > If not, they want to access things at fixed offsets. So we can just
> > use the type they're using to model untrusted derefs.
> >
> > So always using prog BTF makes sense to me.
>
> Ok, I'm switching to always using prog BTF.
Hold on. The concept of ptr_to_btf_id|untrusted that points to
prog type doesn't exist today. We should be careful when introducing
such things.
I prefer to keep btf_get_ptr_to_btf_id() in this patch
and think through untrusted|ptr_to prog type later,
since the use case of untrusted local type doesn't quite resonate with me.
Currently we only have mem_alloc|ptr_to prog type which is
read/write and came from obj_new, rbtree, link lists.
Untrusted == readonly for prog type is quite odd.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 20:05 ` Alexei Starovoitov
@ 2025-07-04 20:20 ` Kumar Kartikeya Dwivedi
2025-07-04 20:34 ` Eduard Zingerman
0 siblings, 1 reply; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 20:20 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Eduard Zingerman, Matt Bobrowski, bpf, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, Kernel Team,
Yonghong Song
On Fri, 4 Jul 2025 at 22:06, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 4, 2025 at 12:23 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Fri, 2025-07-04 at 21:15 +0200, Kumar Kartikeya Dwivedi wrote:
> >
> > [...]
> >
> > > Yeah, so if the user specifies a type and has co-re enabled, they're
> > > accessing a kernel struct.
> > > If they're doing it without co-re, it's broken today already, or they
> > > know the struct is fixed in layout somehow so it's ok.
> > > If not, they want to access things at fixed offsets. So we can just
> > > use the type they're using to model untrusted derefs.
> > >
> > > So always using prog BTF makes sense to me.
> >
> > Ok, I'm switching to always using prog BTF.
>
> Hold on. The concept of ptr_to_btf_id|untrusted that points to
> prog type doesn't exist today. We should be careful when introducing
> such things.
> I prefer to keep btf_get_ptr_to_btf_id() in this patch
> and think through untrusted|ptr_to prog type later,
> since the use case of untrusted local type doesn't quite resonate with me.
Yeah, we can add it separately from this set, but otherwise I don't
see the problem with the idea.
There is no reason to restrict ourselves to kernel types.
All accesses will be untrusted, it's like probe_read so it should be
well-formed for any type.
It's the same reason why pointers to non-struct makes sense. Ideally
any type should be allowed.
Otherwise to reconstruct a walk of untrusted pointer chains the user
will do it by hand.
Showing the structure types to the verifier allows it to be inserted
automatically.
> Currently we only have mem_alloc|ptr_to prog type which is
> read/write and came from obj_new, rbtree, link lists.
> Untrusted == readonly for prog type is quite odd.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 20:20 ` Kumar Kartikeya Dwivedi
@ 2025-07-04 20:34 ` Eduard Zingerman
2025-07-04 20:47 ` Alexei Starovoitov
0 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-04 20:34 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi, Alexei Starovoitov
Cc: Matt Bobrowski, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Martin KaFai Lau, Kernel Team, Yonghong Song
On Fri, 2025-07-04 at 22:20 +0200, Kumar Kartikeya Dwivedi wrote:
> On Fri, 4 Jul 2025 at 22:06, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Jul 4, 2025 at 12:23 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > On Fri, 2025-07-04 at 21:15 +0200, Kumar Kartikeya Dwivedi wrote:
> > >
> > > [...]
> > >
> > > > Yeah, so if the user specifies a type and has co-re enabled, they're
> > > > accessing a kernel struct.
> > > > If they're doing it without co-re, it's broken today already, or they
> > > > know the struct is fixed in layout somehow so it's ok.
> > > > If not, they want to access things at fixed offsets. So we can just
> > > > use the type they're using to model untrusted derefs.
> > > >
> > > > So always using prog BTF makes sense to me.
> > >
> > > Ok, I'm switching to always using prog BTF.
> >
> > Hold on. The concept of ptr_to_btf_id|untrusted that points to
> > prog type doesn't exist today. We should be careful when introducing
> > such things.
> > I prefer to keep btf_get_ptr_to_btf_id() in this patch
> > and think through untrusted|ptr_to prog type later,
> > since the use case of untrusted local type doesn't quite resonate with me.
>
> Yeah, we can add it separately from this set, but otherwise I don't
> see the problem with the idea.
> There is no reason to restrict ourselves to kernel types.
> All accesses will be untrusted, it's like probe_read so it should be
> well-formed for any type.
> It's the same reason why pointers to non-struct makes sense. Ideally
> any type should be allowed.
>
> Otherwise to reconstruct a walk of untrusted pointer chains the user
> will do it by hand.
> Showing the structure types to the verifier allows it to be inserted
> automatically.
I agree with Kumar, it's an expansion of the idea behind
bpf_rdonly_cast(...,0). Having some untrusted pointer source
(e.g. value from a program stack trace) one can already write:
struct foo {
struct bar *bar;
};
struct foo *foo = bpf_rdonly_cast(magic_value, 0);
struct bar *bar = bpf_rdonly_cast(foo->bar, 0);
We can make it more convenient by introducing an additional kfunc:
struct foo {
struct bar *bar;
};
struct foo *foo = bpf_rdonly_cast_local(magic_value, bpf_core_type_id_local(struct foo));
... just use foo->bar ...
As for global function parameters. Discussed this with Kumar off-list
a bit. There are types with generic names in vmlinux.h, like "region",
"regex", "hash", "key". So, imo, the above bpf_rdonly_cast_local()
should be accompanied by its own tag: __arg_untrusted_local.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters
2025-07-04 20:34 ` Eduard Zingerman
@ 2025-07-04 20:47 ` Alexei Starovoitov
0 siblings, 0 replies; 34+ messages in thread
From: Alexei Starovoitov @ 2025-07-04 20:47 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Kumar Kartikeya Dwivedi, Matt Bobrowski, bpf, Alexei Starovoitov,
Andrii Nakryiko, Daniel Borkmann, Martin KaFai Lau, Kernel Team,
Yonghong Song
On Fri, Jul 4, 2025 at 1:34 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2025-07-04 at 22:20 +0200, Kumar Kartikeya Dwivedi wrote:
> > On Fri, 4 Jul 2025 at 22:06, Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Jul 4, 2025 at 12:23 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > > >
> > > > On Fri, 2025-07-04 at 21:15 +0200, Kumar Kartikeya Dwivedi wrote:
> > > >
> > > > [...]
> > > >
> > > > > Yeah, so if the user specifies a type and has co-re enabled, they're
> > > > > accessing a kernel struct.
> > > > > If they're doing it without co-re, it's broken today already, or they
> > > > > know the struct is fixed in layout somehow so it's ok.
> > > > > If not, they want to access things at fixed offsets. So we can just
> > > > > use the type they're using to model untrusted derefs.
> > > > >
> > > > > So always using prog BTF makes sense to me.
> > > >
> > > > Ok, I'm switching to always using prog BTF.
> > >
> > > Hold on. The concept of ptr_to_btf_id|untrusted that points to
> > > prog type doesn't exist today. We should be careful when introducing
> > > such things.
> > > I prefer to keep btf_get_ptr_to_btf_id() in this patch
> > > and think through untrusted|ptr_to prog type later,
> > > since the use case of untrusted local type doesn't quite resonate with me.
> >
> > Yeah, we can add it separately from this set, but otherwise I don't
> > see the problem with the idea.
> > There is no reason to restrict ourselves to kernel types.
The reason not to do it is to avoid cornering ourselves.
Like, we defined byte access to 'char *' in a global func too soon.
I didn't like the idea, but didn't have better alternative, and
now we cannot extend it to an array of chars or any other semantics.
> > All accesses will be untrusted, it's like probe_read so it should be
> > well-formed for any type.
> > It's the same reason why pointers to non-struct makes sense. Ideally
> > any type should be allowed.
> >
> > Otherwise to reconstruct a walk of untrusted pointer chains the user
> > will do it by hand.
> > Showing the structure types to the verifier allows it to be inserted
> > automatically.
>
> I agree with Kumar, it's an expansion of the idea behind
> bpf_rdonly_cast(...,0). Having some untrusted pointer source
> (e.g. value from a program stack trace) one can already write:
>
> struct foo {
> struct bar *bar;
> };
>
> struct foo *foo = bpf_rdonly_cast(magic_value, 0);
> struct bar *bar = bpf_rdonly_cast(foo->bar, 0);
>
> We can make it more convenient by introducing an additional kfunc:
>
> struct foo {
> struct bar *bar;
> };
>
> struct foo *foo = bpf_rdonly_cast_local(magic_value, bpf_core_type_id_local(struct foo));
> ... just use foo->bar ...
Such usage only makes sense for kernel types.
If 'struct foo' is prog type then passing it to a global
function to do probe_mem style access doesn't make sense to me.
If it's prog type it's either used in arena or with bpf_obj_new.
I don't see a case whether the prog needs to probe_read
its own objects.
> As for global function parameters. Discussed this with Kumar off-list
> a bit. There are types with generic names in vmlinux.h, like "region",
> "regex", "hash", "key". So, imo, the above bpf_rdonly_cast_local()
> should be accompanied by its own tag: __arg_untrusted_local.
I still don't see a value in bpf_rdonly_cast_local.
struct foo example looks artificial.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH bpf-next v1 5/8] libbpf: __arg_untrusted in bpf_helpers.h
2025-07-02 22:42 [PATCH bpf-next v1 0/8] bpf: additional use-cases for untrusted PTR_TO_MEM Eduard Zingerman
` (3 preceding siblings ...)
2025-07-02 22:42 ` [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters Eduard Zingerman
@ 2025-07-02 22:42 ` Eduard Zingerman
2025-07-04 18:04 ` Kumar Kartikeya Dwivedi
2025-07-02 22:42 ` [PATCH bpf-next v1 6/8] selftests/bpf: test cases for __arg_untrusted Eduard Zingerman
` (2 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-02 22:42 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
Make btf_decl_tag("arg:untrusted") available for libbpf users via
macro. Makes the following usage possible:
void foo(struct bar *p __arg_untrusted) { ... }
void bar(struct foo *p __arg_trusted) {
...
foo(p->buz->bar); // buz derefrence looses __trusted
...
}
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/lib/bpf/bpf_helpers.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index a50773d4616e..cd7771951a71 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -215,6 +215,7 @@ enum libbpf_tristate {
#define __arg_nonnull __attribute((btf_decl_tag("arg:nonnull")))
#define __arg_nullable __attribute((btf_decl_tag("arg:nullable")))
#define __arg_trusted __attribute((btf_decl_tag("arg:trusted")))
+#define __arg_untrusted __attribute((btf_decl_tag("arg:untrusted")))
#define __arg_arena __attribute((btf_decl_tag("arg:arena")))
#ifndef ___bpf_concat
--
2.47.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 5/8] libbpf: __arg_untrusted in bpf_helpers.h
2025-07-02 22:42 ` [PATCH bpf-next v1 5/8] libbpf: __arg_untrusted in bpf_helpers.h Eduard Zingerman
@ 2025-07-04 18:04 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 18:04 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song
On Thu, 3 Jul 2025 at 00:47, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Make btf_decl_tag("arg:untrusted") available for libbpf users via
> macro. Makes the following usage possible:
>
> void foo(struct bar *p __arg_untrusted) { ... }
> void bar(struct foo *p __arg_trusted) {
> ...
> foo(p->buz->bar); // buz derefrence looses __trusted
> ...
> }
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> tools/lib/bpf/bpf_helpers.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index a50773d4616e..cd7771951a71 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -215,6 +215,7 @@ enum libbpf_tristate {
> #define __arg_nonnull __attribute((btf_decl_tag("arg:nonnull")))
> #define __arg_nullable __attribute((btf_decl_tag("arg:nullable")))
> #define __arg_trusted __attribute((btf_decl_tag("arg:trusted")))
> +#define __arg_untrusted __attribute((btf_decl_tag("arg:untrusted")))
> #define __arg_arena __attribute((btf_decl_tag("arg:arena")))
>
> #ifndef ___bpf_concat
> --
> 2.47.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH bpf-next v1 6/8] selftests/bpf: test cases for __arg_untrusted
2025-07-02 22:42 [PATCH bpf-next v1 0/8] bpf: additional use-cases for untrusted PTR_TO_MEM Eduard Zingerman
` (4 preceding siblings ...)
2025-07-02 22:42 ` [PATCH bpf-next v1 5/8] libbpf: __arg_untrusted in bpf_helpers.h Eduard Zingerman
@ 2025-07-02 22:42 ` Eduard Zingerman
2025-07-04 18:05 ` Kumar Kartikeya Dwivedi
2025-07-02 22:42 ` [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params Eduard Zingerman
2025-07-02 22:42 ` [PATCH bpf-next v1 8/8] selftests/bpf: tests for __arg_untrusted void * " Eduard Zingerman
7 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-02 22:42 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
Check usage of __arg_untrusted parameters with PTR_TO_BTF_ID:
- combining __arg_untrusted with other tags is forbidden;
- passing of {trusted, untrusted, map value, scalar value, values with
variable offset} to untrusted is ok;
- passing of PTR_TO_BTF_ID with a different type to untrusted is ok;
- passing of untrusted to trusted is forbidden.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../bpf/progs/verifier_global_ptr_args.c | 66 +++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
index 4ab0ef18d7eb..772e8dd3e001 100644
--- a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
+++ b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
@@ -179,4 +179,70 @@ int BPF_PROG(trusted_acq_rel, struct task_struct *task, u64 clone_flags)
return subprog_trusted_acq_rel(task);
}
+__weak int subprog_untrusted_bad_tags(struct task_struct *task __arg_untrusted __arg_nullable)
+{
+ return task->pid;
+}
+
+SEC("tp_btf/sys_enter")
+__failure
+__msg("arg#0 untrusted cannot be combined with any other tags")
+int untrusted_bad_tags(void *ctx)
+{
+ return subprog_untrusted_bad_tags(0);
+}
+
+__weak int subprog_untrusted(struct task_struct *task __arg_untrusted)
+{
+ return task->pid;
+}
+
+SEC("tp_btf/sys_enter")
+__success
+__log_level(2)
+__msg("r1 = {{.*}}; {{.*}}R1_w=trusted_ptr_task_struct()")
+__msg("Func#1 ('subprog_untrusted') is global and assumed valid.")
+__msg("Validating subprog_untrusted() func#1...")
+__msg(": R1=untrusted_ptr_task_struct")
+int trusted_to_untrusted(void *ctx)
+{
+ return subprog_untrusted(bpf_get_current_task_btf());
+}
+
+char mem[16];
+u32 off;
+
+SEC("tp_btf/sys_enter")
+__success
+int anything_to_untrusted(void *ctx)
+{
+ /* untrusted to untrusted */
+ subprog_untrusted(bpf_core_cast(0, struct task_struct));
+ /* wrong type to untrusted */
+ subprog_untrusted((void *)bpf_core_cast(0, struct bpf_verifier_env));
+ /* map value to untrusted */
+ subprog_untrusted((void *)mem);
+ /* scalar to untrusted */
+ subprog_untrusted(0);
+ /* variable offset to untrusted (map) */
+ subprog_untrusted((void *)mem + off);
+ /* variable offset to untrusted (trusted) */
+ subprog_untrusted((void *)bpf_get_current_task_btf() + off);
+ return 0;
+}
+
+__weak int subprog_untrusted2(struct task_struct *task __arg_untrusted)
+{
+ return subprog_trusted_task_nullable(task);
+}
+
+SEC("tp_btf/sys_enter")
+__failure
+__msg("R1 type=untrusted_ptr_ expected=ptr_, trusted_ptr_, rcu_ptr_")
+__msg("Caller passes invalid args into func#{{.*}} ('subprog_trusted_task_nullable')")
+int untrusted_to_trusted(void *ctx)
+{
+ return subprog_untrusted2(bpf_get_current_task_btf());
+}
+
char _license[] SEC("license") = "GPL";
--
2.47.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 6/8] selftests/bpf: test cases for __arg_untrusted
2025-07-02 22:42 ` [PATCH bpf-next v1 6/8] selftests/bpf: test cases for __arg_untrusted Eduard Zingerman
@ 2025-07-04 18:05 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 18:05 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song
On Thu, 3 Jul 2025 at 00:42, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Check usage of __arg_untrusted parameters with PTR_TO_BTF_ID:
> - combining __arg_untrusted with other tags is forbidden;
> - passing of {trusted, untrusted, map value, scalar value, values with
> variable offset} to untrusted is ok;
> - passing of PTR_TO_BTF_ID with a different type to untrusted is ok;
> - passing of untrusted to trusted is forbidden.
If you decide or do not decide to support program local types, one
extra test could exercise support/lack of support as well.
It should fail to find the candidate if unsupported, succeed if supported.
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> .../bpf/progs/verifier_global_ptr_args.c | 66 +++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
> index 4ab0ef18d7eb..772e8dd3e001 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
> @@ -179,4 +179,70 @@ int BPF_PROG(trusted_acq_rel, struct task_struct *task, u64 clone_flags)
> return subprog_trusted_acq_rel(task);
> }
>
> +__weak int subprog_untrusted_bad_tags(struct task_struct *task __arg_untrusted __arg_nullable)
> +{
> + return task->pid;
> +}
> +
> +SEC("tp_btf/sys_enter")
> +__failure
> +__msg("arg#0 untrusted cannot be combined with any other tags")
> +int untrusted_bad_tags(void *ctx)
> +{
> + return subprog_untrusted_bad_tags(0);
> +}
> +
> +__weak int subprog_untrusted(struct task_struct *task __arg_untrusted)
> +{
> + return task->pid;
> +}
> +
> +SEC("tp_btf/sys_enter")
> +__success
> +__log_level(2)
> +__msg("r1 = {{.*}}; {{.*}}R1_w=trusted_ptr_task_struct()")
> +__msg("Func#1 ('subprog_untrusted') is global and assumed valid.")
> +__msg("Validating subprog_untrusted() func#1...")
> +__msg(": R1=untrusted_ptr_task_struct")
> +int trusted_to_untrusted(void *ctx)
> +{
> + return subprog_untrusted(bpf_get_current_task_btf());
> +}
> +
> +char mem[16];
> +u32 off;
> +
> +SEC("tp_btf/sys_enter")
> +__success
> +int anything_to_untrusted(void *ctx)
> +{
> + /* untrusted to untrusted */
> + subprog_untrusted(bpf_core_cast(0, struct task_struct));
> + /* wrong type to untrusted */
> + subprog_untrusted((void *)bpf_core_cast(0, struct bpf_verifier_env));
> + /* map value to untrusted */
> + subprog_untrusted((void *)mem);
> + /* scalar to untrusted */
> + subprog_untrusted(0);
> + /* variable offset to untrusted (map) */
> + subprog_untrusted((void *)mem + off);
> + /* variable offset to untrusted (trusted) */
> + subprog_untrusted((void *)bpf_get_current_task_btf() + off);
> + return 0;
> +}
> +
> +__weak int subprog_untrusted2(struct task_struct *task __arg_untrusted)
> +{
> + return subprog_trusted_task_nullable(task);
> +}
> +
> +SEC("tp_btf/sys_enter")
> +__failure
> +__msg("R1 type=untrusted_ptr_ expected=ptr_, trusted_ptr_, rcu_ptr_")
> +__msg("Caller passes invalid args into func#{{.*}} ('subprog_trusted_task_nullable')")
> +int untrusted_to_trusted(void *ctx)
> +{
> + return subprog_untrusted2(bpf_get_current_task_btf());
> +}
> +
> char _license[] SEC("license") = "GPL";
> --
> 2.47.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params
2025-07-02 22:42 [PATCH bpf-next v1 0/8] bpf: additional use-cases for untrusted PTR_TO_MEM Eduard Zingerman
` (5 preceding siblings ...)
2025-07-02 22:42 ` [PATCH bpf-next v1 6/8] selftests/bpf: test cases for __arg_untrusted Eduard Zingerman
@ 2025-07-02 22:42 ` Eduard Zingerman
2025-07-03 3:20 ` Alexei Starovoitov
2025-07-04 18:09 ` Kumar Kartikeya Dwivedi
2025-07-02 22:42 ` [PATCH bpf-next v1 8/8] selftests/bpf: tests for __arg_untrusted void * " Eduard Zingerman
7 siblings, 2 replies; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-02 22:42 UTC (permalink / raw)
To: bpf, ast, andrii
Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87,
Alexei Starovoitov
Allow specifying __arg_untrusted for void */char */int */long *
parameters. Treat such parameters as
PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED of size zero.
Intended usage is as follows:
int memcmp(char *a __arg_untrusted, char *b __arg_untrusted, size_t n) {
bpf_for(i, 0, n) {
if (a[i] - b[i]) // load at any offset is allowed
return a[i] - b[i];
}
return 0;
}
Allocate register id for ARG_PTR_TO_MEM parameters only when
PTR_MAYBE_NULL is set. Register id for PTR_TO_MEM is used only to
propagate non-null status after conditionals.
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
include/linux/btf.h | 1 +
kernel/bpf/btf.c | 13 +++++++++++++
kernel/bpf/verifier.c | 7 ++++---
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index a40beb9cf160..9eda6b113f9b 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -223,6 +223,7 @@ u32 btf_nr_types(const struct btf *btf);
struct btf *btf_base_btf(const struct btf *btf);
bool btf_type_is_i32(const struct btf_type *t);
bool btf_type_is_i64(const struct btf_type *t);
+bool btf_type_is_primitive(const struct btf_type *t);
bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
const struct btf_member *m,
u32 expected_offset, u32 expected_size);
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 28cb0a2a5402..ffe560c0ec65 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -891,6 +891,12 @@ bool btf_type_is_i64(const struct btf_type *t)
return btf_type_is_int(t) && __btf_type_int_is_regular(t, 8);
}
+bool btf_type_is_primitive(const struct btf_type *t)
+{
+ return (btf_type_is_int(t) && btf_type_int_is_regular(t)) ||
+ btf_is_any_enum(t);
+}
+
/*
* Check that given struct member is a regular int with expected
* offset and size.
@@ -7829,6 +7835,13 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
return -EINVAL;
}
+ ref_t = btf_type_skip_modifiers(btf, t->type, NULL);
+ if (btf_type_is_void(ref_t) || btf_type_is_primitive(ref_t)) {
+ sub->args[i].arg_type = ARG_PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED;
+ sub->args[i].mem_size = 0;
+ continue;
+ }
+
kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
if (kern_type_id < 0)
return kern_type_id;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index dfb5a2f8e58f..53f70ef9adc0 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -23158,11 +23158,12 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
__mark_dynptr_reg(reg, BPF_DYNPTR_TYPE_LOCAL, true, ++env->id_gen);
} else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) {
reg->type = PTR_TO_MEM;
- if (arg->arg_type & PTR_MAYBE_NULL)
- reg->type |= PTR_MAYBE_NULL;
+ reg->type |= arg->arg_type &
+ (PTR_MAYBE_NULL | PTR_UNTRUSTED | MEM_RDONLY);
mark_reg_known_zero(env, regs, i);
reg->mem_size = arg->mem_size;
- reg->id = ++env->id_gen;
+ if (arg->arg_type & PTR_MAYBE_NULL)
+ reg->id = ++env->id_gen;
} else if (base_type(arg->arg_type) == ARG_PTR_TO_BTF_ID) {
reg->type = PTR_TO_BTF_ID;
if (arg->arg_type & PTR_MAYBE_NULL)
--
2.47.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params
2025-07-02 22:42 ` [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params Eduard Zingerman
@ 2025-07-03 3:20 ` Alexei Starovoitov
2025-07-03 21:49 ` Eduard Zingerman
2025-07-04 18:09 ` Kumar Kartikeya Dwivedi
1 sibling, 1 reply; 34+ messages in thread
From: Alexei Starovoitov @ 2025-07-03 3:20 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Wed, Jul 2, 2025 at 3:42 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Allow specifying __arg_untrusted for void */char */int */long *
> parameters. Treat such parameters as
> PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED of size zero.
> Intended usage is as follows:
>
> int memcmp(char *a __arg_untrusted, char *b __arg_untrusted, size_t n) {
> bpf_for(i, 0, n) {
> if (a[i] - b[i]) // load at any offset is allowed
> return a[i] - b[i];
> }
> return 0;
> }
...
> +bool btf_type_is_primitive(const struct btf_type *t)
> +{
> + return (btf_type_is_int(t) && btf_type_int_is_regular(t)) ||
> + btf_is_any_enum(t);
> +}
Should array of primitive types be allowed as well ?
Since in C
int memcmp(char a[] __arg_untrusted, char b[] __arg_untrusted, size_t n) {
bpf_for(i, 0, n) {
if (a[i] - b[i]) // load at any offset is allowed
return a[i] - b[i];
will work just like 'char *'.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params
2025-07-03 3:20 ` Alexei Starovoitov
@ 2025-07-03 21:49 ` Eduard Zingerman
2025-07-04 18:11 ` Alexei Starovoitov
0 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-03 21:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Wed, 2025-07-02 at 20:20 -0700, Alexei Starovoitov wrote:
> On Wed, Jul 2, 2025 at 3:42 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > Allow specifying __arg_untrusted for void */char */int */long *
> > parameters. Treat such parameters as
> > PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED of size zero.
> > Intended usage is as follows:
> >
> > int memcmp(char *a __arg_untrusted, char *b __arg_untrusted, size_t n) {
> > bpf_for(i, 0, n) {
> > if (a[i] - b[i]) // load at any offset is allowed
> > return a[i] - b[i];
> > }
> > return 0;
> > }
>
> ...
>
> > +bool btf_type_is_primitive(const struct btf_type *t)
> > +{
> > + return (btf_type_is_int(t) && btf_type_int_is_regular(t)) ||
> > + btf_is_any_enum(t);
> > +}
>
> Should array of primitive types be allowed as well ?
> Since in C
> int memcmp(char a[] __arg_untrusted, char b[] __arg_untrusted, size_t n) {
> bpf_for(i, 0, n) {
> if (a[i] - b[i]) // load at any offset is allowed
> return a[i] - b[i];
>
> will work just like 'char *'.
I agree in general, but compiler converts arrays to pointers for
function parameters, e.g.:
[~/tmp]
$ cat test-array-btf.c
int foo(int a[], char b[3]) {
return 0;
}
[~/tmp]
$ clang --target=bpf -c -g -O2 test-array-btf.c -o test-array-btf.o
[~/tmp]
$ bpftool btf dump file test-array-btf.o
[1] PTR '(anon)' type_id=2
[2] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
[3] PTR '(anon)' type_id=4
[4] INT 'char' size=1 bits_offset=0 nr_bits=8 encoding=SIGNED
[5] FUNC_PROTO '(anon)' ret_type_id=2 vlen=2
'a' type_id=1
'b' type_id=3
[6] FUNC 'foo' type_id=5 linkage=global
So, I'm inclined to skip this for now.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params
2025-07-03 21:49 ` Eduard Zingerman
@ 2025-07-04 18:11 ` Alexei Starovoitov
0 siblings, 0 replies; 34+ messages in thread
From: Alexei Starovoitov @ 2025-07-04 18:11 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Thu, Jul 3, 2025 at 2:49 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2025-07-02 at 20:20 -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 2, 2025 at 3:42 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > Allow specifying __arg_untrusted for void */char */int */long *
> > > parameters. Treat such parameters as
> > > PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED of size zero.
> > > Intended usage is as follows:
> > >
> > > int memcmp(char *a __arg_untrusted, char *b __arg_untrusted, size_t n) {
> > > bpf_for(i, 0, n) {
> > > if (a[i] - b[i]) // load at any offset is allowed
> > > return a[i] - b[i];
> > > }
> > > return 0;
> > > }
> >
> > ...
> >
> > > +bool btf_type_is_primitive(const struct btf_type *t)
> > > +{
> > > + return (btf_type_is_int(t) && btf_type_int_is_regular(t)) ||
> > > + btf_is_any_enum(t);
> > > +}
> >
> > Should array of primitive types be allowed as well ?
> > Since in C
> > int memcmp(char a[] __arg_untrusted, char b[] __arg_untrusted, size_t n) {
> > bpf_for(i, 0, n) {
> > if (a[i] - b[i]) // load at any offset is allowed
> > return a[i] - b[i];
> >
> > will work just like 'char *'.
>
> I agree in general, but compiler converts arrays to pointers for
> function parameters, e.g.:
>
> [~/tmp]
> $ cat test-array-btf.c
> int foo(int a[], char b[3]) {
> return 0;
> }
> [~/tmp]
> $ clang --target=bpf -c -g -O2 test-array-btf.c -o test-array-btf.o
> [~/tmp]
> $ bpftool btf dump file test-array-btf.o
> [1] PTR '(anon)' type_id=2
> [2] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
> [3] PTR '(anon)' type_id=4
> [4] INT 'char' size=1 bits_offset=0 nr_bits=8 encoding=SIGNED
> [5] FUNC_PROTO '(anon)' ret_type_id=2 vlen=2
> 'a' type_id=1
> 'b' type_id=3
> [6] FUNC 'foo' type_id=5 linkage=global
>
> So, I'm inclined to skip this for now.
I see. Makes sense then.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params
2025-07-02 22:42 ` [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params Eduard Zingerman
2025-07-03 3:20 ` Alexei Starovoitov
@ 2025-07-04 18:09 ` Kumar Kartikeya Dwivedi
1 sibling, 0 replies; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 18:09 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
Alexei Starovoitov
On Thu, 3 Jul 2025 at 00:43, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Allow specifying __arg_untrusted for void */char */int */long *
> parameters. Treat such parameters as
> PTR_TO_MEM|MEM_RDONLY|PTR_UNTRUSTED of size zero.
> Intended usage is as follows:
>
> int memcmp(char *a __arg_untrusted, char *b __arg_untrusted, size_t n) {
> bpf_for(i, 0, n) {
> if (a[i] - b[i]) // load at any offset is allowed
> return a[i] - b[i];
> }
> return 0;
> }
>
> Allocate register id for ARG_PTR_TO_MEM parameters only when
> PTR_MAYBE_NULL is set. Register id for PTR_TO_MEM is used only to
> propagate non-null status after conditionals.
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> [...]
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH bpf-next v1 8/8] selftests/bpf: tests for __arg_untrusted void * global func params
2025-07-02 22:42 [PATCH bpf-next v1 0/8] bpf: additional use-cases for untrusted PTR_TO_MEM Eduard Zingerman
` (6 preceding siblings ...)
2025-07-02 22:42 ` [PATCH bpf-next v1 7/8] bpf: support for void/primitive __arg_untrusted global func params Eduard Zingerman
@ 2025-07-02 22:42 ` Eduard Zingerman
2025-07-04 18:12 ` Kumar Kartikeya Dwivedi
7 siblings, 1 reply; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-02 22:42 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
Check usage of __arg_untrusted parameters of primitive type:
- passing of {trusted, untrusted, map value, scalar value, values with
variable offset} to untrusted `void *` or `char *` is ok;
- varifier represents such parameters as rdonly_untrusted_mem(sz=0).
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../bpf/progs/verifier_global_ptr_args.c | 41 +++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
index 772e8dd3e001..f91d9c2906aa 100644
--- a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
+++ b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
@@ -245,4 +245,45 @@ int untrusted_to_trusted(void *ctx)
return subprog_untrusted2(bpf_get_current_task_btf());
}
+__weak int subprog_void_untrusted(void *p __arg_untrusted)
+{
+ return *(int *)p;
+}
+
+__weak int subprog_char_untrusted(char *p __arg_untrusted)
+{
+ return *(int *)p;
+}
+
+SEC("tp_btf/sys_enter")
+__success
+__log_level(2)
+__msg("r1 = {{.*}}; {{.*}}R1_w=trusted_ptr_task_struct()")
+__msg("Func#1 ('subprog_void_untrusted') is global and assumed valid.")
+__msg("Validating subprog_void_untrusted() func#1...")
+__msg(": R1=rdonly_untrusted_mem(sz=0)")
+int trusted_to_untrusted_mem(void *ctx)
+{
+ return subprog_void_untrusted(bpf_get_current_task_btf());
+}
+
+SEC("tp_btf/sys_enter")
+__success
+int anything_to_untrusted_mem(void *ctx)
+{
+ /* untrusted to untrusted mem */
+ subprog_void_untrusted(bpf_core_cast(0, struct task_struct));
+ /* map value to untrusted mem */
+ subprog_void_untrusted(mem);
+ /* scalar to untrusted mem */
+ subprog_void_untrusted(0);
+ /* variable offset to untrusted mem (map) */
+ subprog_void_untrusted((void *)mem + off);
+ /* variable offset to untrusted mem (trusted) */
+ subprog_void_untrusted(bpf_get_current_task_btf() + off);
+ /* variable offset to untrusted char (map) */
+ subprog_char_untrusted(mem + off);
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
--
2.47.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 8/8] selftests/bpf: tests for __arg_untrusted void * global func params
2025-07-02 22:42 ` [PATCH bpf-next v1 8/8] selftests/bpf: tests for __arg_untrusted void * " Eduard Zingerman
@ 2025-07-04 18:12 ` Kumar Kartikeya Dwivedi
2025-07-04 18:35 ` Eduard Zingerman
0 siblings, 1 reply; 34+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2025-07-04 18:12 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song
On Thu, 3 Jul 2025 at 00:44, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Check usage of __arg_untrusted parameters of primitive type:
> - passing of {trusted, untrusted, map value, scalar value, values with
> variable offset} to untrusted `void *` or `char *` is ok;
> - varifier represents such parameters as rdonly_untrusted_mem(sz=0).
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
LGTM, but can we also exercise BTF_KIND_ENUM{,64}? Since you
explicitly handle both of them. I guess char * covers BTF_KIND_INT, so
we don't need more cases (but up to you).
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> .../bpf/progs/verifier_global_ptr_args.c | 41 +++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
> index 772e8dd3e001..f91d9c2906aa 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
> @@ -245,4 +245,45 @@ int untrusted_to_trusted(void *ctx)
> return subprog_untrusted2(bpf_get_current_task_btf());
> }
>
> +__weak int subprog_void_untrusted(void *p __arg_untrusted)
> +{
> + return *(int *)p;
> +}
> +
> +__weak int subprog_char_untrusted(char *p __arg_untrusted)
> +{
> + return *(int *)p;
> +}
> +
> +SEC("tp_btf/sys_enter")
> +__success
> +__log_level(2)
> +__msg("r1 = {{.*}}; {{.*}}R1_w=trusted_ptr_task_struct()")
> +__msg("Func#1 ('subprog_void_untrusted') is global and assumed valid.")
> +__msg("Validating subprog_void_untrusted() func#1...")
> +__msg(": R1=rdonly_untrusted_mem(sz=0)")
> +int trusted_to_untrusted_mem(void *ctx)
> +{
> + return subprog_void_untrusted(bpf_get_current_task_btf());
> +}
> +
> +SEC("tp_btf/sys_enter")
> +__success
> +int anything_to_untrusted_mem(void *ctx)
> +{
> + /* untrusted to untrusted mem */
> + subprog_void_untrusted(bpf_core_cast(0, struct task_struct));
> + /* map value to untrusted mem */
> + subprog_void_untrusted(mem);
> + /* scalar to untrusted mem */
> + subprog_void_untrusted(0);
> + /* variable offset to untrusted mem (map) */
> + subprog_void_untrusted((void *)mem + off);
> + /* variable offset to untrusted mem (trusted) */
> + subprog_void_untrusted(bpf_get_current_task_btf() + off);
> + /* variable offset to untrusted char (map) */
> + subprog_char_untrusted(mem + off);
> + return 0;
> +}
> +
> char _license[] SEC("license") = "GPL";
> --
> 2.47.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH bpf-next v1 8/8] selftests/bpf: tests for __arg_untrusted void * global func params
2025-07-04 18:12 ` Kumar Kartikeya Dwivedi
@ 2025-07-04 18:35 ` Eduard Zingerman
0 siblings, 0 replies; 34+ messages in thread
From: Eduard Zingerman @ 2025-07-04 18:35 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song
On Fri, 2025-07-04 at 20:12 +0200, Kumar Kartikeya Dwivedi wrote:
> On Thu, 3 Jul 2025 at 00:44, Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > Check usage of __arg_untrusted parameters of primitive type:
> > - passing of {trusted, untrusted, map value, scalar value, values with
> > variable offset} to untrusted `void *` or `char *` is ok;
> > - varifier represents such parameters as rdonly_untrusted_mem(sz=0).
> >
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> > ---
>
> LGTM, but can we also exercise BTF_KIND_ENUM{,64}? Since you
> explicitly handle both of them. I guess char * covers BTF_KIND_INT, so
> we don't need more cases (but up to you).
Sure, will add.
[...]
^ permalink raw reply [flat|nested] 34+ messages in thread