* [PATCH bpf-next v1 1/4] bpf: allow void* cast using bpf_rdonly_cast()
2025-06-24 19:10 [PATCH bpf-next v1 0/4] bpf: allow void* cast using bpf_rdonly_cast() Eduard Zingerman
@ 2025-06-24 19:10 ` Eduard Zingerman
2025-06-24 19:10 ` [PATCH bpf-next v1 2/4] bpf: add bpf_features enum Eduard Zingerman
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Eduard Zingerman @ 2025-06-24 19:10 UTC (permalink / raw)
To: bpf, ast, andrii
Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87,
Alexei Starovoitov, Andrii Nakryiko
Introduce support for `bpf_rdonly_cast(v, 0)`, which casts the value
`v` to an untyped, untrusted pointer, logically similar to a `void *`.
The memory pointed to by such a pointer is treated as read-only.
As with other untrusted pointers, memory access violations on loads
return zero instead of causing a fault.
Technically:
- The resulting pointer is represented as a register of type
`PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED` with size zero.
- Offsets within such pointers are not tracked.
- Same load instructions are allowed to have both
`PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED` and `PTR_TO_BTF_ID`
as the base pointer types.
In such cases, `bpf_insn_aux_data->ptr_type` is considered the
weaker of the two: `PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED`.
The following constraints apply to the new pointer type:
- can be used as a base for LDX instructions;
- can't be used as a base for ST/STX or atomic instructions;
- can't be used as parameter for kfuncs or helpers.
These constraints are enforced by existing handling of `MEM_RDONLY`
flag and `PTR_TO_MEM` of size zero.
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
kernel/bpf/verifier.c | 72 +++++++++++++++++++++++++++++++++++--------
1 file changed, 60 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 279a64933262..8fd65eb74051 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7535,6 +7535,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
}
} else if (base_type(reg->type) == PTR_TO_MEM) {
bool rdonly_mem = type_is_rdonly_mem(reg->type);
+ bool rdonly_untrusted = rdonly_mem && (reg->type & PTR_UNTRUSTED);
if (type_may_be_null(reg->type)) {
verbose(env, "R%d invalid mem access '%s'\n", regno,
@@ -7554,8 +7555,13 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
return -EACCES;
}
- err = check_mem_region_access(env, regno, off, size,
- reg->mem_size, false);
+ /*
+ * Accesses to untrusted PTR_TO_MEM are done through probe
+ * instructions, hence no need to check bounds in that case.
+ */
+ if (!rdonly_untrusted)
+ err = check_mem_region_access(env, regno, off, size,
+ reg->mem_size, false);
if (!err && value_regno >= 0 && (t == BPF_READ || rdonly_mem))
mark_reg_unknown(env, regs, value_regno);
} else if (reg->type == PTR_TO_CTX) {
@@ -13602,16 +13608,24 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_ca
regs[BPF_REG_0].btf_id = meta->ret_btf_id;
} else if (meta->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) {
ret_t = btf_type_by_id(desc_btf, meta->arg_constant.value);
- if (!ret_t || !btf_type_is_struct(ret_t)) {
+ if (!ret_t) {
+ verbose(env, "Unknown type ID %lld passed to kfunc bpf_rdonly_cast\n",
+ meta->arg_constant.value);
+ return -EINVAL;
+ } else if (btf_type_is_struct(ret_t)) {
+ mark_reg_known_zero(env, regs, BPF_REG_0);
+ regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_UNTRUSTED;
+ regs[BPF_REG_0].btf = desc_btf;
+ regs[BPF_REG_0].btf_id = meta->arg_constant.value;
+ } else if (btf_type_is_void(ret_t)) {
+ mark_reg_known_zero(env, regs, BPF_REG_0);
+ regs[BPF_REG_0].type = PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED;
+ regs[BPF_REG_0].mem_size = 0;
+ } else {
verbose(env,
- "kfunc bpf_rdonly_cast type ID argument must be of a struct\n");
+ "kfunc bpf_rdonly_cast type ID argument must be of a struct or void\n");
return -EINVAL;
}
-
- mark_reg_known_zero(env, regs, BPF_REG_0);
- regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_UNTRUSTED;
- regs[BPF_REG_0].btf = desc_btf;
- regs[BPF_REG_0].btf_id = meta->arg_constant.value;
} else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_slice] ||
meta->func_id == special_kfunc_list[KF_bpf_dynptr_slice_rdwr]) {
enum bpf_type_flag type_flag = get_dynptr_type_flag(meta->initialized_dynptr.type);
@@ -14410,6 +14424,13 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
return -EACCES;
}
+ /*
+ * Accesses to untrusted PTR_TO_MEM are done through probe
+ * instructions, hence no need to track offsets.
+ */
+ if (base_type(ptr_reg->type) == PTR_TO_MEM && (ptr_reg->type & PTR_UNTRUSTED))
+ return 0;
+
switch (base_type(ptr_reg->type)) {
case PTR_TO_CTX:
case PTR_TO_MAP_VALUE:
@@ -19567,10 +19588,27 @@ static bool reg_type_mismatch(enum bpf_reg_type src, enum bpf_reg_type prev)
!reg_type_mismatch_ok(prev));
}
+static bool is_ptr_to_mem_or_btf_id(enum bpf_reg_type type)
+{
+ switch (base_type(type)) {
+ case PTR_TO_MEM:
+ case PTR_TO_BTF_ID:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool is_ptr_to_mem(enum bpf_reg_type type)
+{
+ return base_type(type) == PTR_TO_MEM;
+}
+
static int save_aux_ptr_type(struct bpf_verifier_env *env, enum bpf_reg_type type,
bool allow_trust_mismatch)
{
enum bpf_reg_type *prev_type = &env->insn_aux_data[env->insn_idx].ptr_type;
+ enum bpf_reg_type merged_type;
if (*prev_type == NOT_INIT) {
/* Saw a valid insn
@@ -19587,15 +19625,24 @@ static int save_aux_ptr_type(struct bpf_verifier_env *env, enum bpf_reg_type typ
* Reject it.
*/
if (allow_trust_mismatch &&
- base_type(type) == PTR_TO_BTF_ID &&
- base_type(*prev_type) == PTR_TO_BTF_ID) {
+ is_ptr_to_mem_or_btf_id(type) &&
+ is_ptr_to_mem_or_btf_id(*prev_type)) {
/*
* Have to support a use case when one path through
* the program yields TRUSTED pointer while another
* is UNTRUSTED. Fallback to UNTRUSTED to generate
* BPF_PROBE_MEM/BPF_PROBE_MEMSX.
+ * Same behavior of MEM_RDONLY flag.
*/
- *prev_type = PTR_TO_BTF_ID | PTR_UNTRUSTED;
+ if (is_ptr_to_mem(type) || is_ptr_to_mem(*prev_type))
+ merged_type = PTR_TO_MEM;
+ else
+ merged_type = PTR_TO_BTF_ID;
+ if ((type & PTR_UNTRUSTED) || (*prev_type & PTR_UNTRUSTED))
+ merged_type |= PTR_UNTRUSTED;
+ if ((type & MEM_RDONLY) || (*prev_type & MEM_RDONLY))
+ merged_type |= MEM_RDONLY;
+ *prev_type = merged_type;
} else {
verbose(env, "same insn cannot be used with different pointers\n");
return -EINVAL;
@@ -21203,6 +21250,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
* for this case.
*/
case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
+ case PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED:
if (type == BPF_READ) {
if (BPF_MODE(insn->code) == BPF_MEM)
insn->code = BPF_LDX | BPF_PROBE_MEM |
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH bpf-next v1 2/4] bpf: add bpf_features enum
2025-06-24 19:10 [PATCH bpf-next v1 0/4] bpf: allow void* cast using bpf_rdonly_cast() Eduard Zingerman
2025-06-24 19:10 ` [PATCH bpf-next v1 1/4] " Eduard Zingerman
@ 2025-06-24 19:10 ` Eduard Zingerman
2025-06-24 21:59 ` Alexei Starovoitov
2025-06-24 19:10 ` [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN Eduard Zingerman
2025-06-24 19:10 ` [PATCH bpf-next v1 4/4] selftests/bpf: check operations on untrusted ro pointers to mem Eduard Zingerman
3 siblings, 1 reply; 11+ messages in thread
From: Eduard Zingerman @ 2025-06-24 19:10 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
This commit adds a kernel side enum for use in conjucntion with BTF
CO-RE bpf_core_enum_value_exists. The goal of the enum is to assist
with available BPF features detection.
Support for bpf_rdonly_cast to void* is the first feature listed in
the enum. Here is an example usage:
if (bpf_core_enum_value_exists(enum bpf_features,
BPF_FEAT_RDONLY_CAST_TO_VOID))
... bpf_rdonly_cast(..., 0) ...
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
kernel/bpf/verifier.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8fd65eb74051..01050d1f7389 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -44,6 +44,11 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
#undef BPF_LINK_TYPE
};
+enum bpf_features {
+ BPF_FEAT_RDONLY_CAST_TO_VOID = 0,
+ BPF_FEAT_TOTAL,
+};
+
struct bpf_mem_alloc bpf_global_percpu_ma;
static bool bpf_global_percpu_ma_set;
@@ -24436,6 +24441,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
u32 log_true_size;
bool is_priv;
+ BTF_TYPE_EMIT(enum bpf_features);
+
/* no program is valid */
if (ARRAY_SIZE(bpf_verifier_ops) == 0)
return -EINVAL;
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH bpf-next v1 2/4] bpf: add bpf_features enum
2025-06-24 19:10 ` [PATCH bpf-next v1 2/4] bpf: add bpf_features enum Eduard Zingerman
@ 2025-06-24 21:59 ` Alexei Starovoitov
2025-06-24 22:07 ` Eduard Zingerman
0 siblings, 1 reply; 11+ messages in thread
From: Alexei Starovoitov @ 2025-06-24 21:59 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Tue, Jun 24, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> This commit adds a kernel side enum for use in conjucntion with BTF
> CO-RE bpf_core_enum_value_exists. The goal of the enum is to assist
> with available BPF features detection.
>
> Support for bpf_rdonly_cast to void* is the first feature listed in
> the enum. Here is an example usage:
>
> if (bpf_core_enum_value_exists(enum bpf_features,
> BPF_FEAT_RDONLY_CAST_TO_VOID))
> ... bpf_rdonly_cast(..., 0) ...
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
> kernel/bpf/verifier.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8fd65eb74051..01050d1f7389 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -44,6 +44,11 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
> #undef BPF_LINK_TYPE
> };
>
> +enum bpf_features {
> + BPF_FEAT_RDONLY_CAST_TO_VOID = 0,
> + BPF_FEAT_TOTAL,
I don't see the value of 'total', but not strongly against it.
But pls be consistent with __MAX_BPF_CMD, __MAX_BPF_MAP_TYPE, ...
Say, __MAX_BPF_FEAT ?
Also it's better to introduce this enum in some earlier patch,
and then always add BTF_FEAT_... to this enum
in the same patch that adds the feature to make
sure backports won't screw it up.
Another rule should be to always assign a number to it.
At the end with random backports the __MAX_BPF_FEAT
won't be accurate, but whatever.
> +};
> +
> struct bpf_mem_alloc bpf_global_percpu_ma;
> static bool bpf_global_percpu_ma_set;
>
> @@ -24436,6 +24441,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
> u32 log_true_size;
> bool is_priv;
>
> + BTF_TYPE_EMIT(enum bpf_features);
> +
> /* no program is valid */
> if (ARRAY_SIZE(bpf_verifier_ops) == 0)
> return -EINVAL;
> --
> 2.47.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH bpf-next v1 2/4] bpf: add bpf_features enum
2025-06-24 21:59 ` Alexei Starovoitov
@ 2025-06-24 22:07 ` Eduard Zingerman
0 siblings, 0 replies; 11+ messages in thread
From: Eduard Zingerman @ 2025-06-24 22:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Tue, 2025-06-24 at 14:59 -0700, Alexei Starovoitov wrote:
[...]
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 8fd65eb74051..01050d1f7389 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -44,6 +44,11 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
> > #undef BPF_LINK_TYPE
> > };
> >
> > +enum bpf_features {
> > + BPF_FEAT_RDONLY_CAST_TO_VOID = 0,
> > + BPF_FEAT_TOTAL,
>
> I don't see the value of 'total', but not strongly against it.
> But pls be consistent with __MAX_BPF_CMD, __MAX_BPF_MAP_TYPE, ...
> Say, __MAX_BPF_FEAT ?
>
>
> Also it's better to introduce this enum in some earlier patch,
> and then always add BTF_FEAT_... to this enum
> in the same patch that adds the feature to make
> sure backports won't screw it up.
> Another rule should be to always assign a number to it.
>
> At the end with random backports the __MAX_BPF_FEAT
> won't be accurate, but whatever.
Ack. Andrii asked to add MAX for people willing to do broken kind of
feature detection and just in case.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN
2025-06-24 19:10 [PATCH bpf-next v1 0/4] bpf: allow void* cast using bpf_rdonly_cast() Eduard Zingerman
2025-06-24 19:10 ` [PATCH bpf-next v1 1/4] " Eduard Zingerman
2025-06-24 19:10 ` [PATCH bpf-next v1 2/4] bpf: add bpf_features enum Eduard Zingerman
@ 2025-06-24 19:10 ` Eduard Zingerman
2025-06-24 21:55 ` Alexei Starovoitov
2025-06-24 19:10 ` [PATCH bpf-next v1 4/4] selftests/bpf: check operations on untrusted ro pointers to mem Eduard Zingerman
3 siblings, 1 reply; 11+ messages in thread
From: Eduard Zingerman @ 2025-06-24 19:10 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
Originally prog_tests/verifier.c was developed to run tests ported
from test_verifier binary. test_verifier runs tests with CAP_SYS_ADMIN
dropped, hence this behaviour was copied in prog_tests/verifier.c.
BPF_OBJ_GET_NEXT_ID BPF syscall command fails w/o CAP_SYS_ADMIN and
this prevents libbpf from loading module BTFs.
This commit adds an optout from capability drop.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../testing/selftests/bpf/prog_tests/verifier.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index c9da06741104..cedb86d8f717 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -115,14 +115,16 @@ struct test_val {
__maybe_unused
static void run_tests_aux(const char *skel_name,
skel_elf_bytes_fn elf_bytes_factory,
- pre_execution_cb pre_execution_cb)
+ pre_execution_cb pre_execution_cb,
+ bool drop_sysadmin)
{
struct test_loader tester = {};
- __u64 old_caps;
+ __u64 caps_to_drop, old_caps;
int err;
/* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
- err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
+ caps_to_drop = drop_sysadmin ? 1ULL << CAP_SYS_ADMIN : 0;
+ err = cap_disable_effective(caps_to_drop, &old_caps);
if (err) {
PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
return;
@@ -137,7 +139,8 @@ static void run_tests_aux(const char *skel_name,
PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
}
-#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL)
+#define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL, true)
+#define RUN_FULL_CAPS(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL, false)
void test_verifier_and(void) { RUN(verifier_and); }
void test_verifier_arena(void) { RUN(verifier_arena); }
@@ -272,7 +275,8 @@ void test_verifier_array_access(void)
{
run_tests_aux("verifier_array_access",
verifier_array_access__elf_bytes,
- init_array_access_maps);
+ init_array_access_maps,
+ true);
}
static int init_value_ptr_arith_maps(struct bpf_object *obj)
@@ -284,5 +288,6 @@ void test_verifier_value_ptr_arith(void)
{
run_tests_aux("verifier_value_ptr_arith",
verifier_value_ptr_arith__elf_bytes,
- init_value_ptr_arith_maps);
+ init_value_ptr_arith_maps,
+ true);
}
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN
2025-06-24 19:10 ` [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN Eduard Zingerman
@ 2025-06-24 21:55 ` Alexei Starovoitov
2025-06-24 22:05 ` Eduard Zingerman
2025-06-24 23:31 ` Eduard Zingerman
0 siblings, 2 replies; 11+ messages in thread
From: Alexei Starovoitov @ 2025-06-24 21:55 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Tue, Jun 24, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> Originally prog_tests/verifier.c was developed to run tests ported
> from test_verifier binary. test_verifier runs tests with CAP_SYS_ADMIN
> dropped, hence this behaviour was copied in prog_tests/verifier.c.
> BPF_OBJ_GET_NEXT_ID BPF syscall command fails w/o CAP_SYS_ADMIN and
> this prevents libbpf from loading module BTFs.
You need this only because of 'bpf_kfunc_trusted_num_test' access
in patch 4?
Can you use kernel kfunc instead?
This needs more thought.
s/RUN/RUN_FULL_CAPS/ just because of kfunc in the bpf_testmod
doesn't look like a good long term approach.
I thought we agreed to relax BPF_OBJ_GET_NEXT_ID to allow for CAP_BPF.
Probably even unpriv can do it.
Just knowing a set of prog, map, bpf IDs is not a security threat.
BPF_BTF_GET_FD_BY_ID can also be allowed for unpriv,
since one can do it already from /sys/kernel/btf/
> This commit adds an optout from capability drop.
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
> .../testing/selftests/bpf/prog_tests/verifier.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index c9da06741104..cedb86d8f717 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -115,14 +115,16 @@ struct test_val {
> __maybe_unused
> static void run_tests_aux(const char *skel_name,
> skel_elf_bytes_fn elf_bytes_factory,
> - pre_execution_cb pre_execution_cb)
> + pre_execution_cb pre_execution_cb,
> + bool drop_sysadmin)
I have an allergic reaction to bool arguments.
> run_tests_aux("verifier_array_access",
> verifier_array_access__elf_bytes,
> - init_array_access_maps);
> + init_array_access_maps,
> + true);
This is not readable without looking at the argument name.
--
pw-bot: cr
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN
2025-06-24 21:55 ` Alexei Starovoitov
@ 2025-06-24 22:05 ` Eduard Zingerman
2025-06-24 23:31 ` Eduard Zingerman
1 sibling, 0 replies; 11+ messages in thread
From: Eduard Zingerman @ 2025-06-24 22:05 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Tue, 2025-06-24 at 14:55 -0700, Alexei Starovoitov wrote:
> On Tue, Jun 24, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > Originally prog_tests/verifier.c was developed to run tests ported
> > from test_verifier binary. test_verifier runs tests with CAP_SYS_ADMIN
> > dropped, hence this behaviour was copied in prog_tests/verifier.c.
> > BPF_OBJ_GET_NEXT_ID BPF syscall command fails w/o CAP_SYS_ADMIN and
> > this prevents libbpf from loading module BTFs.
>
> You need this only because of 'bpf_kfunc_trusted_num_test' access
> in patch 4?
Yes.
> Can you use kernel kfunc instead?
Should be able to.
> This needs more thought.
> s/RUN/RUN_FULL_CAPS/ just because of kfunc in the bpf_testmod
> doesn't look like a good long term approach.
>
> I thought we agreed to relax BPF_OBJ_GET_NEXT_ID to allow for CAP_BPF.
> Probably even unpriv can do it.
> Just knowing a set of prog, map, bpf IDs is not a security threat.
>
> BPF_BTF_GET_FD_BY_ID can also be allowed for unpriv,
> since one can do it already from /sys/kernel/btf/
Makes sense to me.
> > This commit adds an optout from capability drop.
> >
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> > ---
> > .../testing/selftests/bpf/prog_tests/verifier.c | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > index c9da06741104..cedb86d8f717 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> > @@ -115,14 +115,16 @@ struct test_val {
> > __maybe_unused
> > static void run_tests_aux(const char *skel_name,
> > skel_elf_bytes_fn elf_bytes_factory,
> > - pre_execution_cb pre_execution_cb)
> > + pre_execution_cb pre_execution_cb,
> > + bool drop_sysadmin)
>
> I have an allergic reaction to bool arguments.
>
> > run_tests_aux("verifier_array_access",
> > verifier_array_access__elf_bytes,
> > - init_array_access_maps);
> > + init_array_access_maps,
> > + true);
>
> This is not readable without looking at the argument name.
I'll drop this change in v2.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN
2025-06-24 21:55 ` Alexei Starovoitov
2025-06-24 22:05 ` Eduard Zingerman
@ 2025-06-24 23:31 ` Eduard Zingerman
2025-06-25 0:04 ` Alexei Starovoitov
1 sibling, 1 reply; 11+ messages in thread
From: Eduard Zingerman @ 2025-06-24 23:31 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Tue, 2025-06-24 at 14:55 -0700, Alexei Starovoitov wrote:
> On Tue, Jun 24, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > Originally prog_tests/verifier.c was developed to run tests ported
> > from test_verifier binary. test_verifier runs tests with CAP_SYS_ADMIN
> > dropped, hence this behaviour was copied in prog_tests/verifier.c.
> > BPF_OBJ_GET_NEXT_ID BPF syscall command fails w/o CAP_SYS_ADMIN and
> > this prevents libbpf from loading module BTFs.
>
> You need this only because of 'bpf_kfunc_trusted_num_test' access
> in patch 4?
> Can you use kernel kfunc instead?
This turned out non-trivial, not many kernel kfuncs take pointers to
primitive types, and those that do are either STRUCT_OPS or need
device bound program or have special checks requiring stack pointers.
I declared a separate prog_tests/mem_rdonly_untrusted.c runner.
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN
2025-06-24 23:31 ` Eduard Zingerman
@ 2025-06-25 0:04 ` Alexei Starovoitov
0 siblings, 0 replies; 11+ messages in thread
From: Alexei Starovoitov @ 2025-06-25 0:04 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Martin KaFai Lau, Kernel Team, Yonghong Song
On Tue, Jun 24, 2025 at 4:31 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Tue, 2025-06-24 at 14:55 -0700, Alexei Starovoitov wrote:
> > On Tue, Jun 24, 2025 at 12:10 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > Originally prog_tests/verifier.c was developed to run tests ported
> > > from test_verifier binary. test_verifier runs tests with CAP_SYS_ADMIN
> > > dropped, hence this behaviour was copied in prog_tests/verifier.c.
> > > BPF_OBJ_GET_NEXT_ID BPF syscall command fails w/o CAP_SYS_ADMIN and
> > > this prevents libbpf from loading module BTFs.
> >
> > You need this only because of 'bpf_kfunc_trusted_num_test' access
> > in patch 4?
> > Can you use kernel kfunc instead?
>
> This turned out non-trivial, not many kernel kfuncs take pointers to
> primitive types, and those that do are either STRUCT_OPS or need
> device bound program or have special checks requiring stack pointers.
>
> I declared a separate prog_tests/mem_rdonly_untrusted.c runner.
Just skip it for now.
Adding unpriv btf access can be a follow up.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next v1 4/4] selftests/bpf: check operations on untrusted ro pointers to mem
2025-06-24 19:10 [PATCH bpf-next v1 0/4] bpf: allow void* cast using bpf_rdonly_cast() Eduard Zingerman
` (2 preceding siblings ...)
2025-06-24 19:10 ` [PATCH bpf-next v1 3/4] selftests/bpf: allow tests from verifier.c not to drop CAP_SYS_ADMIN Eduard Zingerman
@ 2025-06-24 19:10 ` Eduard Zingerman
3 siblings, 0 replies; 11+ messages in thread
From: Eduard Zingerman @ 2025-06-24 19:10 UTC (permalink / raw)
To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87
The following cases are tested:
- it is ok to load memory at any offset from rdonly_untrusted_mem;
- rdonly_untrusted_mem offset/bounds are not tracked;
- writes into rdonly_untrusted_mem are forbidden;
- atomic operations on rdonly_untrusted_mem are forbidden;
- rdonly_untrusted_mem can't be passed as a memory argument of a
helper of kfunc;
- it is ok to use PTR_TO_MEM and PTR_TO_BTF_ID in a same load
instruction.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../selftests/bpf/prog_tests/verifier.c | 2 +
.../bpf/progs/verifier_mem_rdonly_untrusted.c | 136 ++++++++++++++++++
2 files changed, 138 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/verifier_mem_rdonly_untrusted.c
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index cedb86d8f717..5cb49ba089d2 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -57,6 +57,7 @@
#include "verifier_may_goto_1.skel.h"
#include "verifier_may_goto_2.skel.h"
#include "verifier_meta_access.skel.h"
+#include "verifier_mem_rdonly_untrusted.skel.h"
#include "verifier_movsx.skel.h"
#include "verifier_mtu.skel.h"
#include "verifier_netfilter_ctx.skel.h"
@@ -205,6 +206,7 @@ void test_verifier_prevent_map_lookup(void) { RUN(verifier_prevent_map_lookup)
void test_verifier_private_stack(void) { RUN(verifier_private_stack); }
void test_verifier_raw_stack(void) { RUN(verifier_raw_stack); }
void test_verifier_raw_tp_writable(void) { RUN(verifier_raw_tp_writable); }
+void test_verifier_mem_rdonly_untrusted(void) { RUN_FULL_CAPS(verifier_mem_rdonly_untrusted); }
void test_verifier_reg_equal(void) { RUN(verifier_reg_equal); }
void test_verifier_ref_tracking(void) { RUN(verifier_ref_tracking); }
void test_verifier_regalloc(void) { RUN(verifier_regalloc); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_mem_rdonly_untrusted.c b/tools/testing/selftests/bpf/progs/verifier_mem_rdonly_untrusted.c
new file mode 100644
index 000000000000..00604755e698
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_mem_rdonly_untrusted.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_core_read.h>
+#include "bpf_misc.h"
+#include "../test_kmods/bpf_testmod_kfunc.h"
+
+SEC("socket")
+__success
+__retval(0)
+int ldx_is_ok_bad_addr(void *ctx)
+{
+ char *p;
+
+ if (!bpf_core_enum_value_exists(enum bpf_features, BPF_FEAT_RDONLY_CAST_TO_VOID))
+ return 42;
+
+ p = bpf_rdonly_cast(0, 0);
+ return p[0x7fff];
+}
+
+SEC("socket")
+__success
+__retval(1)
+int ldx_is_ok_good_addr(void *ctx)
+{
+ int v, *p;
+
+ v = 1;
+ p = bpf_rdonly_cast(&v, 0);
+ return *p;
+}
+
+SEC("socket")
+__success
+int offset_not_tracked(void *ctx)
+{
+ int *p, i, s;
+
+ p = bpf_rdonly_cast(0, 0);
+ s = 0;
+ bpf_for(i, 0, 1000 * 1000 * 1000) {
+ p++;
+ s += *p;
+ }
+ return s;
+}
+
+SEC("socket")
+__failure
+__msg("cannot write into rdonly_untrusted_mem")
+int stx_not_ok(void *ctx)
+{
+ int v, *p;
+
+ v = 1;
+ p = bpf_rdonly_cast(&v, 0);
+ *p = 1;
+ return 0;
+}
+
+SEC("socket")
+__failure
+__msg("cannot write into rdonly_untrusted_mem")
+int atomic_not_ok(void *ctx)
+{
+ int v, *p;
+
+ v = 1;
+ p = bpf_rdonly_cast(&v, 0);
+ __sync_fetch_and_add(p, 1);
+ return 0;
+}
+
+SEC("socket")
+__failure
+__msg("cannot write into rdonly_untrusted_mem")
+int atomic_rmw_not_ok(void *ctx)
+{
+ long v, *p;
+
+ v = 1;
+ p = bpf_rdonly_cast(&v, 0);
+ return __sync_val_compare_and_swap(p, 0, 42);
+}
+
+SEC("socket")
+__failure
+__msg("invalid access to memory, mem_size=0 off=0 size=4")
+__msg("R1 min value is outside of the allowed memory range")
+int kfunc_param_not_ok(void *ctx)
+{
+ int *p;
+
+ p = bpf_rdonly_cast(0, 0);
+ bpf_kfunc_trusted_num_test(p);
+ return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure
+__msg("R1 type=rdonly_untrusted_mem expected=")
+int helper_param_not_ok(void *ctx)
+{
+ char *p;
+
+ p = bpf_rdonly_cast(0, 0);
+ /*
+ * Any helper with ARG_CONST_SIZE_OR_ZERO constraint will do,
+ * the most permissive constraint
+ */
+ bpf_copy_from_user(p, 0, (void *)42);
+ return 0;
+}
+
+static __noinline u64 *get_some_addr(void)
+{
+ if (bpf_get_prandom_u32())
+ return bpf_rdonly_cast(0, bpf_core_type_id_kernel(struct sock));
+ else
+ return bpf_rdonly_cast(0, 0);
+}
+
+SEC("socket")
+__success
+__retval(0)
+int mixed_mem_type(void *ctx)
+{
+ u64 *p;
+
+ /* Try to avoid compiler hoisting load to if branches by using __noinline func. */
+ p = get_some_addr();
+ return *p;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.47.1
^ permalink raw reply related [flat|nested] 11+ messages in thread