Skip to content

Commit

Permalink
bpf: Zero index arg error string for dynptr and iter
Browse files Browse the repository at this point in the history 
Andrii spotted that process_dynptr_func's rejection of incorrect
argument register type will print an error string where argument numbers
are not zero-indexed, unlike elsewhere in the verifier.  Fix this by
subtracting 1 from regno. The same scenario exists for iterator
messages. Fix selftest error strings that match on the exact argument
number while we're at it to ensure clean bisection.

Suggested-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
  • Loading branch information
@kkdwivedi
kkdwivedi authored and Kernel Patches Daemon committed Dec 3, 2024
1 parent 64752e5 commit 5239184
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 29 deletions.
12 changes: 6 additions & 6 deletions kernel/bpf/verifier.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8071,7 +8071,7 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn
if (reg->type != PTR_TO_STACK && reg->type != CONST_PTR_TO_DYNPTR) {
verbose(env,
"arg#%d expected pointer to stack or const struct bpf_dynptr\n",
regno);
regno - 1);
return -EINVAL;
}

Expand Down Expand Up @@ -8125,15 +8125,15 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn
if (!is_dynptr_reg_valid_init(env, reg)) {
verbose(env,
"Expected an initialized dynptr as arg #%d\n",
regno);
regno - 1);
return -EINVAL;
}

/* Fold modifiers (in this case, MEM_RDONLY) when checking expected type */
if (!is_dynptr_type_expected(env, reg, arg_type & ~MEM_RDONLY)) {
verbose(env,
"Expected a dynptr of type %s as arg #%d\n",
dynptr_type_str(arg_to_dynptr_type(arg_type)), regno);
dynptr_type_str(arg_to_dynptr_type(arg_type)), regno - 1);
return -EINVAL;
}

Expand Down Expand Up @@ -8197,7 +8197,7 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
*/
btf_id = btf_check_iter_arg(meta->btf, meta->func_proto, regno - 1);
if (btf_id < 0) {
verbose(env, "expected valid iter pointer as arg #%d\n", regno);
verbose(env, "expected valid iter pointer as arg #%d\n", regno - 1);
return -EINVAL;
}
t = btf_type_by_id(meta->btf, btf_id);
Expand All @@ -8207,7 +8207,7 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
/* bpf_iter_<type>_new() expects pointer to uninit iter state */
if (!is_iter_reg_valid_uninit(env, reg, nr_slots)) {
verbose(env, "expected uninitialized iter_%s as arg #%d\n",
iter_type_str(meta->btf, btf_id), regno);
iter_type_str(meta->btf, btf_id), regno - 1);
return -EINVAL;
}

Expand All @@ -8231,7 +8231,7 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
break;
case -EINVAL:
verbose(env, "expected an initialized iter_%s as arg #%d\n",
iter_type_str(meta->btf, btf_id), regno);
iter_type_str(meta->btf, btf_id), regno - 1);
return err;
case -EPROTO:
verbose(env, "expected an RCU CS when using %s\n", meta->func_name);
Expand Down
22 changes: 11 additions & 11 deletions tools/testing/selftests/bpf/progs/dynptr_fail.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ int ringbuf_release_uninit_dynptr(void *ctx)

/* A dynptr can't be used after it has been invalidated */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #3")
__failure __msg("Expected an initialized dynptr as arg #2")
int use_after_invalid(void *ctx)
{
struct bpf_dynptr ptr;
Expand Down Expand Up @@ -428,7 +428,7 @@ int invalid_helper2(void *ctx)

/* A bpf_dynptr is invalidated if it's been written into */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #1")
__failure __msg("Expected an initialized dynptr as arg #0")
int invalid_write1(void *ctx)
{
struct bpf_dynptr ptr;
Expand Down Expand Up @@ -1407,7 +1407,7 @@ int invalid_slice_rdwr_rdonly(struct __sk_buff *skb)

/* bpf_dynptr_adjust can only be called on initialized dynptrs */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #1")
__failure __msg("Expected an initialized dynptr as arg #0")
int dynptr_adjust_invalid(void *ctx)
{
struct bpf_dynptr ptr = {};
Expand All @@ -1420,7 +1420,7 @@ int dynptr_adjust_invalid(void *ctx)

/* bpf_dynptr_is_null can only be called on initialized dynptrs */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #1")
__failure __msg("Expected an initialized dynptr as arg #0")
int dynptr_is_null_invalid(void *ctx)
{
struct bpf_dynptr ptr = {};
Expand All @@ -1433,7 +1433,7 @@ int dynptr_is_null_invalid(void *ctx)

/* bpf_dynptr_is_rdonly can only be called on initialized dynptrs */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #1")
__failure __msg("Expected an initialized dynptr as arg #0")
int dynptr_is_rdonly_invalid(void *ctx)
{
struct bpf_dynptr ptr = {};
Expand All @@ -1446,7 +1446,7 @@ int dynptr_is_rdonly_invalid(void *ctx)

/* bpf_dynptr_size can only be called on initialized dynptrs */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #1")
__failure __msg("Expected an initialized dynptr as arg #0")
int dynptr_size_invalid(void *ctx)
{
struct bpf_dynptr ptr = {};
Expand All @@ -1459,7 +1459,7 @@ int dynptr_size_invalid(void *ctx)

/* Only initialized dynptrs can be cloned */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #1")
__failure __msg("Expected an initialized dynptr as arg #0")
int clone_invalid1(void *ctx)
{
struct bpf_dynptr ptr1 = {};
Expand Down Expand Up @@ -1493,7 +1493,7 @@ int clone_invalid2(struct xdp_md *xdp)

/* Invalidating a dynptr should invalidate its clones */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #3")
__failure __msg("Expected an initialized dynptr as arg #2")
int clone_invalidate1(void *ctx)
{
struct bpf_dynptr clone;
Expand All @@ -1514,7 +1514,7 @@ int clone_invalidate1(void *ctx)

/* Invalidating a dynptr should invalidate its parent */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #3")
__failure __msg("Expected an initialized dynptr as arg #2")
int clone_invalidate2(void *ctx)
{
struct bpf_dynptr ptr;
Expand All @@ -1535,7 +1535,7 @@ int clone_invalidate2(void *ctx)

/* Invalidating a dynptr should invalidate its siblings */
SEC("?raw_tp")
__failure __msg("Expected an initialized dynptr as arg #3")
__failure __msg("Expected an initialized dynptr as arg #2")
int clone_invalidate3(void *ctx)
{
struct bpf_dynptr ptr;
Expand Down Expand Up @@ -1723,7 +1723,7 @@ __noinline long global_call_bpf_dynptr(const struct bpf_dynptr *dynptr)
}

SEC("?raw_tp")
__failure __msg("arg#1 expected pointer to stack or const struct bpf_dynptr")
__failure __msg("arg#0 expected pointer to stack or const struct bpf_dynptr")
int test_dynptr_reg_type(void *ctx)
{
struct task_struct *current = NULL;
Expand Down
14 changes: 7 additions & 7 deletions tools/testing/selftests/bpf/progs/iters_state_safety.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ int create_and_forget_to_destroy_fail(void *ctx)
}

SEC("?raw_tp")
__failure __msg("expected an initialized iter_num as arg #1")
__failure __msg("expected an initialized iter_num as arg #0")
int destroy_without_creating_fail(void *ctx)
{
/* init with zeros to stop verifier complaining about uninit stack */
Expand All @@ -91,7 +91,7 @@ int destroy_without_creating_fail(void *ctx)
}

SEC("?raw_tp")
__failure __msg("expected an initialized iter_num as arg #1")
__failure __msg("expected an initialized iter_num as arg #0")
int compromise_iter_w_direct_write_fail(void *ctx)
{
struct bpf_iter_num iter;
Expand Down Expand Up @@ -143,7 +143,7 @@ int compromise_iter_w_direct_write_and_skip_destroy_fail(void *ctx)
}

SEC("?raw_tp")
__failure __msg("expected an initialized iter_num as arg #1")
__failure __msg("expected an initialized iter_num as arg #0")
int compromise_iter_w_helper_write_fail(void *ctx)
{
struct bpf_iter_num iter;
Expand Down Expand Up @@ -230,7 +230,7 @@ int valid_stack_reuse(void *ctx)
}

SEC("?raw_tp")
__failure __msg("expected uninitialized iter_num as arg #1")
__failure __msg("expected uninitialized iter_num as arg #0")
int double_create_fail(void *ctx)
{
struct bpf_iter_num iter;
Expand Down Expand Up @@ -258,7 +258,7 @@ int double_create_fail(void *ctx)
}

SEC("?raw_tp")
__failure __msg("expected an initialized iter_num as arg #1")
__failure __msg("expected an initialized iter_num as arg #0")
int double_destroy_fail(void *ctx)
{
struct bpf_iter_num iter;
Expand All @@ -284,7 +284,7 @@ int double_destroy_fail(void *ctx)
}

SEC("?raw_tp")
__failure __msg("expected an initialized iter_num as arg #1")
__failure __msg("expected an initialized iter_num as arg #0")
int next_without_new_fail(void *ctx)
{
struct bpf_iter_num iter;
Expand All @@ -305,7 +305,7 @@ int next_without_new_fail(void *ctx)
}

SEC("?raw_tp")
__failure __msg("expected an initialized iter_num as arg #1")
__failure __msg("expected an initialized iter_num as arg #0")
int next_after_destroy_fail(void *ctx)
{
struct bpf_iter_num iter;
Expand Down
4 changes: 2 additions & 2 deletions tools/testing/selftests/bpf/progs/iters_testmod_seq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ int testmod_seq_truncated(const void *ctx)

SEC("?raw_tp")
__failure
__msg("expected an initialized iter_testmod_seq as arg #2")
__msg("expected an initialized iter_testmod_seq as arg #1")
int testmod_seq_getter_before_bad(const void *ctx)
{
struct bpf_iter_testmod_seq it;
Expand All @@ -89,7 +89,7 @@ int testmod_seq_getter_before_bad(const void *ctx)

SEC("?raw_tp")
__failure
__msg("expected an initialized iter_testmod_seq as arg #2")
__msg("expected an initialized iter_testmod_seq as arg #1")
int testmod_seq_getter_after_bad(const void *ctx)
{
struct bpf_iter_testmod_seq it;
Expand Down
2 changes: 1 addition & 1 deletion tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size)
}

SEC("?lsm.s/bpf")
__failure __msg("arg#1 expected pointer to stack or const struct bpf_dynptr")
__failure __msg("arg#0 expected pointer to stack or const struct bpf_dynptr")
int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size)
{
unsigned long val = 0;
Expand Down
4 changes: 2 additions & 2 deletions tools/testing/selftests/bpf/progs/verifier_bits_iter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ int BPF_PROG(no_destroy, struct bpf_iter_meta *meta, struct cgroup *cgrp)

SEC("iter/cgroup")
__description("uninitialized iter in ->next()")
__failure __msg("expected an initialized iter_bits as arg #1")
__failure __msg("expected an initialized iter_bits as arg #0")
int BPF_PROG(next_uninit, struct bpf_iter_meta *meta, struct cgroup *cgrp)
{
struct bpf_iter_bits *it = NULL;
Expand All @@ -43,7 +43,7 @@ int BPF_PROG(next_uninit, struct bpf_iter_meta *meta, struct cgroup *cgrp)

SEC("iter/cgroup")
__description("uninitialized iter in ->destroy()")
__failure __msg("expected an initialized iter_bits as arg #1")
__failure __msg("expected an initialized iter_bits as arg #0")
int BPF_PROG(destroy_uninit, struct bpf_iter_meta *meta, struct cgroup *cgrp)
{
struct bpf_iter_bits it = {};
Expand Down

0 comments on commit 5239184

Please sign in to comment.