Skip to content

Commit

Permalink
bpf: Fix ref_obj_id for dynptr data slices in verifier
Browse files Browse the repository at this point in the history
When a data slice is obtained from a dynptr (through the bpf_dynptr_data API),
the ref obj id of the dynptr must be found and then associated with the data
slice.

The ref obj id of the dynptr must be found *before* the caller saved regs are
reset. Without this fix, the ref obj id tracking is not correct for
dynptrs that are at an offset from the frame pointer.

Please also note that the data slice's ref obj id must be assigned after the
ret types are parsed, since RET_PTR_TO_ALLOC_MEM-type return regs get
zero-marked.

Fixes: 34d4ef5 ("bpf: Add dynptr data slices")
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
Acked-by: David Vernet <void@manifault.com>
Link: https://lore.kernel.org/r/20220809214055.4050604-1-joannelkoong@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
joannekoong authored and Alexei Starovoitov committed Aug 10, 2022
1 parent a7be0ab commit 8837434
Showing 1 changed file with 20 additions and 18 deletions.
38 changes: 20 additions & 18 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ static bool is_ptr_cast_function(enum bpf_func_id func_id)
func_id == BPF_FUNC_skc_to_tcp_request_sock;
}

static bool is_dynptr_acquire_function(enum bpf_func_id func_id)
static bool is_dynptr_ref_function(enum bpf_func_id func_id)
{
return func_id == BPF_FUNC_dynptr_data;
}
Expand All @@ -518,7 +518,7 @@ static bool helper_multiple_ref_obj_use(enum bpf_func_id func_id,
ref_obj_uses++;
if (is_acquire_function(func_id, map))
ref_obj_uses++;
if (is_dynptr_acquire_function(func_id))
if (is_dynptr_ref_function(func_id))
ref_obj_uses++;

return ref_obj_uses > 1;
Expand Down Expand Up @@ -7320,6 +7320,23 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
}
}
break;
case BPF_FUNC_dynptr_data:
for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) {
if (arg_type_is_dynptr(fn->arg_type[i])) {
if (meta.ref_obj_id) {
verbose(env, "verifier internal error: meta.ref_obj_id already set\n");
return -EFAULT;
}
/* Find the id of the dynptr we're tracking the reference of */
meta.ref_obj_id = stack_slot_get_id(env, &regs[BPF_REG_1 + i]);
break;
}
}
if (i == MAX_BPF_FUNC_REG_ARGS) {
verbose(env, "verifier internal error: no dynptr in bpf_dynptr_data()\n");
return -EFAULT;
}
break;
}

if (err)
Expand Down Expand Up @@ -7457,7 +7474,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
return -EFAULT;
}

if (is_ptr_cast_function(func_id)) {
if (is_ptr_cast_function(func_id) || is_dynptr_ref_function(func_id)) {
/* For release_reference() */
regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
} else if (is_acquire_function(func_id, meta.map_ptr)) {
Expand All @@ -7469,21 +7486,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
regs[BPF_REG_0].id = id;
/* For release_reference() */
regs[BPF_REG_0].ref_obj_id = id;
} else if (is_dynptr_acquire_function(func_id)) {
int dynptr_id = 0, i;

/* Find the id of the dynptr we're tracking the reference of */
for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) {
if (arg_type_is_dynptr(fn->arg_type[i])) {
if (dynptr_id) {
verbose(env, "verifier internal error: multiple dynptr args in func\n");
return -EFAULT;
}
dynptr_id = stack_slot_get_id(env, &regs[BPF_REG_1 + i]);
}
}
/* For release_reference() */
regs[BPF_REG_0].ref_obj_id = dynptr_id;
}

do_refine_retval_range(regs, fn->ret_type, func_id, &meta);
Expand Down

0 comments on commit 8837434

Please sign in to comment.