mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-07-16 17:57:38 -04:00
bpf: Refactor object relationship tracking and fix dynptr UAF bug
Refactor object relationship tracking in the verifier and fix a dynptr
use-after-free bug where file/skb dynptrs are not invalidated when the
parent referenced object is freed.
Add parent_id to bpf_reg_state to precisely track child-parent
relationships. A child object's parent_id points to the parent object's
id. This replaces the PTR_TO_MEM-specific dynptr_id.
Remove ref_obj_id from bpf_reg_state by folding its role into the
existing id field. Previously, id tracked pointer identity for null
checking while ref_obj_id tracked the owning reference for lifetime
management. These are now unified: acquire helpers and kfuncs set id
to the acquired reference id, and release paths use id directly.
Add reg_is_referenced() which checks if a register is referenced by
looking up its id in the reference array. This replaces all former
ref_obj_id checks.
For release_reference(), invalidating an object now also invalidates
all descendants by traversing the object tree. This is done using
stack-based DFS to avoid recursive call chains of release_reference() ->
unmark_stack_slots_dynptr() -> release_reference(). Referenced objects
encountered during tree traversal are reported as leaked references.
Add parent_id to bpf_reference_state to enable hierarchical reference
tracking. When acquiring a reference, a parent_id can be specified to
link the new reference to an existing one (e.g., referenced dynptrs
acquire a reference with parent_id linking to the parent object's
reference).
Pointer casting:
For pointer casting helpers (bpf_sk_fullsock, bpf_tcp_sock), instead of
propagating ref_obj_id, the cast result reuses the same reference id as
the source pointer. Since the cast may return NULL for a non-NULL input,
the NULL case is explored as a separate verifier branch. This allows
releasing any of the original or cast pointers to invalidate all others.
Referenced dynptrs:
When constructing a referenced dynptr, acquire a intermediate reference
with parent_id linking to the parent referenced object. The dynptr and
all clones share the same parent_id (pointing to the intermediate ref)
but get unique ids for independent slice tracking. Releasing a
referenced dynptr releases the parent reference, which in turn
invalidates all clones and their derived slices.
Owning to non-owning reference conversion:
After converting owning to non-owning by clearing id (e.g.,
object(id=1) -> object(id=0)), the verifier releases the reference
state via release_reference_nomark().
Note that the error message "reference has not been acquired before" in
the helper and kfunc release paths is removed. This message was already
unreachable. The verifier only calls release_reference() after
confirming the reference is valid, so the condition could never trigger
in practice.
Fixes: 870c28588a ("bpf: net_sched: Add basic bpf qdisc kfuncs")
Signed-off-by: Amery Hung <ameryhung@gmail.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20260529014936.2811085-6-ameryhung@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
committed by
Alexei Starovoitov
parent
06d518a558
commit
308c7a0ae8
@@ -1062,7 +1062,7 @@ struct bpf_insn_access_aux {
|
||||
struct {
|
||||
struct btf *btf;
|
||||
u32 btf_id;
|
||||
u32 ref_obj_id;
|
||||
u32 ref_id;
|
||||
};
|
||||
};
|
||||
struct bpf_verifier_log *log; /* for verbose logs */
|
||||
@@ -1631,7 +1631,7 @@ struct bpf_ctx_arg_aux {
|
||||
enum bpf_reg_type reg_type;
|
||||
struct btf *btf;
|
||||
u32 btf_id;
|
||||
u32 ref_obj_id;
|
||||
u32 ref_id;
|
||||
bool refcounted;
|
||||
};
|
||||
|
||||
|
||||
@@ -66,7 +66,6 @@ struct bpf_reg_state {
|
||||
|
||||
struct { /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
|
||||
u32 mem_size;
|
||||
u32 dynptr_id; /* for dynptr slices */
|
||||
};
|
||||
|
||||
/* For dynptr stack slots */
|
||||
@@ -148,46 +147,14 @@ struct bpf_reg_state {
|
||||
#define BPF_ADD_CONST32 (1U << 30)
|
||||
#define BPF_ADD_CONST (BPF_ADD_CONST64 | BPF_ADD_CONST32)
|
||||
u32 id;
|
||||
/* PTR_TO_SOCKET and PTR_TO_TCP_SOCK could be a ptr returned
|
||||
* from a pointer-cast helper, bpf_sk_fullsock() and
|
||||
* bpf_tcp_sock().
|
||||
*
|
||||
* Consider the following where "sk" is a reference counted
|
||||
* pointer returned from "sk = bpf_sk_lookup_tcp();":
|
||||
*
|
||||
* 1: sk = bpf_sk_lookup_tcp();
|
||||
* 2: if (!sk) { return 0; }
|
||||
* 3: fullsock = bpf_sk_fullsock(sk);
|
||||
* 4: if (!fullsock) { bpf_sk_release(sk); return 0; }
|
||||
* 5: tp = bpf_tcp_sock(fullsock);
|
||||
* 6: if (!tp) { bpf_sk_release(sk); return 0; }
|
||||
* 7: bpf_sk_release(sk);
|
||||
* 8: snd_cwnd = tp->snd_cwnd; // verifier will complain
|
||||
*
|
||||
* After bpf_sk_release(sk) at line 7, both "fullsock" ptr and
|
||||
* "tp" ptr should be invalidated also. In order to do that,
|
||||
* the reg holding "fullsock" and "sk" need to remember
|
||||
* the original refcounted ptr id (i.e. sk_reg->id) in ref_obj_id
|
||||
* such that the verifier can reset all regs which have
|
||||
* ref_obj_id matching the sk_reg->id.
|
||||
*
|
||||
* sk_reg->ref_obj_id is set to sk_reg->id at line 1.
|
||||
* sk_reg->id will stay as NULL-marking purpose only.
|
||||
* After NULL-marking is done, sk_reg->id can be reset to 0.
|
||||
*
|
||||
* After "fullsock = bpf_sk_fullsock(sk);" at line 3,
|
||||
* fullsock_reg->ref_obj_id is set to sk_reg->ref_obj_id.
|
||||
*
|
||||
* After "tp = bpf_tcp_sock(fullsock);" at line 5,
|
||||
* tp_reg->ref_obj_id is set to fullsock_reg->ref_obj_id
|
||||
* which is the same as sk_reg->ref_obj_id.
|
||||
*
|
||||
* From the verifier perspective, if sk, fullsock and tp
|
||||
* are not NULL, they are the same ptr with different
|
||||
* reg->type. In particular, bpf_sk_release(tp) is also
|
||||
* allowed and has the same effect as bpf_sk_release(sk).
|
||||
/*
|
||||
* Tracks the parent object this register was derived from.
|
||||
* Used for cascading invalidation: when the parent object is
|
||||
* released or invalidated, all registers with matching parent_id
|
||||
* are also invalidated. For example, a slice from bpf_dynptr_data()
|
||||
* gets parent_id set to the dynptr's id.
|
||||
*/
|
||||
u32 ref_obj_id;
|
||||
u32 parent_id;
|
||||
/* Inside the callee two registers can be both PTR_TO_STACK like
|
||||
* R1=fp-8 and R2=fp-8, but one of them points to this function stack
|
||||
* while another to the caller's stack. To differentiate them 'frameno'
|
||||
@@ -364,10 +331,14 @@ struct bpf_reference_state {
|
||||
* is used purely to inform the user of a reference leak.
|
||||
*/
|
||||
int insn_idx;
|
||||
/* Use to keep track of the source object of a lock, to ensure
|
||||
* it matches on unlock.
|
||||
*/
|
||||
void *ptr;
|
||||
union {
|
||||
/* For REF_TYPE_PTR */
|
||||
int parent_id;
|
||||
/* Use to keep track of the source object of a lock, to ensure
|
||||
* it matches on unlock.
|
||||
*/
|
||||
void *ptr;
|
||||
};
|
||||
};
|
||||
|
||||
struct bpf_retval_range {
|
||||
@@ -585,7 +556,7 @@ bpf_get_spilled_stack_arg(int slot, struct bpf_func_state *frame)
|
||||
iter < frame->out_stack_arg_cnt; \
|
||||
iter++, reg = bpf_get_spilled_stack_arg(iter, frame))
|
||||
|
||||
#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr) \
|
||||
#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __stack, __mask, __expr) \
|
||||
({ \
|
||||
struct bpf_verifier_state *___vstate = __vst; \
|
||||
int ___i, ___j; \
|
||||
@@ -593,6 +564,7 @@ bpf_get_spilled_stack_arg(int slot, struct bpf_func_state *frame)
|
||||
struct bpf_reg_state *___regs; \
|
||||
__state = ___vstate->frame[___i]; \
|
||||
___regs = __state->regs; \
|
||||
__stack = NULL; \
|
||||
for (___j = 0; ___j < MAX_BPF_REG; ___j++) { \
|
||||
__reg = &___regs[___j]; \
|
||||
(void)(__expr); \
|
||||
@@ -600,8 +572,10 @@ bpf_get_spilled_stack_arg(int slot, struct bpf_func_state *frame)
|
||||
bpf_for_each_spilled_reg(___j, __state, __reg, __mask) { \
|
||||
if (!__reg) \
|
||||
continue; \
|
||||
__stack = &__state->stack[___j]; \
|
||||
(void)(__expr); \
|
||||
} \
|
||||
__stack = NULL; \
|
||||
bpf_for_each_spilled_stack_arg(___j, __state, __reg) { \
|
||||
if (!__reg) \
|
||||
continue; \
|
||||
@@ -611,8 +585,13 @@ bpf_get_spilled_stack_arg(int slot, struct bpf_func_state *frame)
|
||||
})
|
||||
|
||||
/* Invoke __expr over regsiters in __vst, setting __state and __reg */
|
||||
#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
|
||||
bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, 1 << STACK_SPILL, __expr)
|
||||
#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
|
||||
({ \
|
||||
struct bpf_stack_state * ___stack; \
|
||||
(void)___stack; \
|
||||
bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, ___stack,\
|
||||
1 << STACK_SPILL, __expr); \
|
||||
})
|
||||
|
||||
/* linked list of verifier states used to prune search */
|
||||
struct bpf_verifier_state_list {
|
||||
@@ -1442,7 +1421,7 @@ struct bpf_map_desc {
|
||||
struct bpf_dynptr_desc {
|
||||
enum bpf_dynptr_type type;
|
||||
u32 id;
|
||||
u32 ref_obj_id;
|
||||
u32 parent_id;
|
||||
};
|
||||
|
||||
struct bpf_kfunc_call_arg_meta {
|
||||
@@ -1453,7 +1432,7 @@ struct bpf_kfunc_call_arg_meta {
|
||||
const struct btf_type *func_proto;
|
||||
const char *func_name;
|
||||
/* Out parameters */
|
||||
u32 ref_obj_id;
|
||||
u32 id;
|
||||
u8 release_regno;
|
||||
bool r0_rdonly;
|
||||
u32 ret_btf_id;
|
||||
|
||||
@@ -6957,7 +6957,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
|
||||
info->reg_type = ctx_arg_info->reg_type;
|
||||
info->btf = ctx_arg_info->btf ? : btf_vmlinux;
|
||||
info->btf_id = ctx_arg_info->btf_id;
|
||||
info->ref_obj_id = ctx_arg_info->ref_obj_id;
|
||||
info->ref_id = ctx_arg_info->ref_id;
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -870,7 +870,7 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
|
||||
case PTR_TO_BTF_ID:
|
||||
case PTR_TO_BTF_ID | PTR_UNTRUSTED:
|
||||
/* PTR_TO_BTF_ID | MEM_ALLOC always has a valid lifetime, unlike
|
||||
* PTR_TO_BTF_ID, and an active ref_obj_id, but the same cannot
|
||||
* PTR_TO_BTF_ID, and an active referenced id, but the same cannot
|
||||
* be said once it is marked PTR_UNTRUSTED, hence we must handle
|
||||
* any faults for loads into such types. BPF_WRITE is disallowed
|
||||
* for this case.
|
||||
|
||||
@@ -665,8 +665,8 @@ static void print_reg_state(struct bpf_verifier_env *env,
|
||||
verbose_a("id=%d", reg->id & ~BPF_ADD_CONST);
|
||||
if (reg->id & BPF_ADD_CONST)
|
||||
verbose(env, "%+d", reg->delta);
|
||||
if (reg->ref_obj_id)
|
||||
verbose_a("ref_obj_id=%d", reg->ref_obj_id);
|
||||
if (reg->parent_id)
|
||||
verbose_a("parent_id=%d", reg->parent_id);
|
||||
if (type_is_non_owning_ref(reg->type))
|
||||
verbose_a("%s", "non_own_ref");
|
||||
if (type_is_map_ptr(t)) {
|
||||
@@ -768,21 +768,19 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifie
|
||||
verbose(env, "=dynptr_%s(", dynptr_type_str(reg->dynptr.type));
|
||||
if (reg->id)
|
||||
verbose_a("id=%d", reg->id);
|
||||
if (reg->ref_obj_id)
|
||||
verbose_a("ref_id=%d", reg->ref_obj_id);
|
||||
if (reg->dynptr_id)
|
||||
verbose_a("dynptr_id=%d", reg->dynptr_id);
|
||||
if (reg->parent_id)
|
||||
verbose_a("parent_id=%d", reg->parent_id);
|
||||
verbose(env, ")");
|
||||
break;
|
||||
case STACK_ITER:
|
||||
/* only main slot has ref_obj_id set; skip others */
|
||||
if (!reg->ref_obj_id)
|
||||
/* only main slot has id set; skip others */
|
||||
if (!reg->id)
|
||||
continue;
|
||||
|
||||
verbose(env, " fp%d=iter_%s(ref_id=%d,state=%s,depth=%u)",
|
||||
verbose(env, " fp%d=iter_%s(id=%d,state=%s,depth=%u)",
|
||||
(-i - 1) * BPF_REG_SIZE,
|
||||
iter_type_str(reg->iter.btf, reg->iter.btf_id),
|
||||
reg->ref_obj_id, iter_state_str(reg->iter.state),
|
||||
reg->id, iter_state_str(reg->iter.state),
|
||||
reg->iter.depth);
|
||||
break;
|
||||
case STACK_MISC:
|
||||
|
||||
@@ -489,7 +489,7 @@ static bool regs_exact(const struct bpf_reg_state *rold,
|
||||
{
|
||||
return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
|
||||
check_ids(rold->id, rcur->id, idmap) &&
|
||||
check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
|
||||
check_ids(rold->parent_id, rcur->parent_id, idmap);
|
||||
}
|
||||
|
||||
enum exact_level {
|
||||
@@ -614,7 +614,7 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
|
||||
range_within(rold, rcur) &&
|
||||
tnum_in(rold->var_off, rcur->var_off) &&
|
||||
check_ids(rold->id, rcur->id, idmap) &&
|
||||
check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
|
||||
check_ids(rold->parent_id, rcur->parent_id, idmap);
|
||||
case PTR_TO_PACKET_META:
|
||||
case PTR_TO_PACKET:
|
||||
/* We must have at least as much range as the old ptr
|
||||
@@ -794,7 +794,8 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
|
||||
cur_reg = &cur->stack[spi].spilled_ptr;
|
||||
if (old_reg->dynptr.type != cur_reg->dynptr.type ||
|
||||
old_reg->dynptr.first_slot != cur_reg->dynptr.first_slot ||
|
||||
!check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap))
|
||||
!check_ids(old_reg->id, cur_reg->id, idmap) ||
|
||||
!check_ids(old_reg->parent_id, cur_reg->parent_id, idmap))
|
||||
return false;
|
||||
break;
|
||||
case STACK_ITER:
|
||||
@@ -810,13 +811,13 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
|
||||
old_reg->iter.btf_id != cur_reg->iter.btf_id ||
|
||||
old_reg->iter.state != cur_reg->iter.state ||
|
||||
/* ignore {old_reg,cur_reg}->iter.depth, see above */
|
||||
!check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap))
|
||||
!check_ids(old_reg->id, cur_reg->id, idmap))
|
||||
return false;
|
||||
break;
|
||||
case STACK_IRQ_FLAG:
|
||||
old_reg = &old->stack[spi].spilled_ptr;
|
||||
cur_reg = &cur->stack[spi].spilled_ptr;
|
||||
if (!check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap) ||
|
||||
if (!check_ids(old_reg->id, cur_reg->id, idmap) ||
|
||||
old_reg->irq.kfunc_class != cur_reg->irq.kfunc_class)
|
||||
return false;
|
||||
break;
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -13,8 +13,8 @@ static struct {
|
||||
const char *err_msg;
|
||||
} spin_lock_fail_tests[] = {
|
||||
{ "lock_id_kptr_preserve",
|
||||
"[0-9]\\+: (bf) r1 = r0 ; R0=ptr_foo(id=2,ref_obj_id=2)"
|
||||
" R1=ptr_foo(id=2,ref_obj_id=2) refs=2\n"
|
||||
"[0-9]\\+: (bf) r1 = r0 ; R0=ptr_foo(id=2)"
|
||||
" R1=ptr_foo(id=2) refs=2\n"
|
||||
"[0-9]\\+: (85) call bpf_this_cpu_ptr#154\n"
|
||||
"R1 type=ptr_ expected=percpu_ptr_" },
|
||||
{ "lock_id_global_zero",
|
||||
|
||||
@@ -78,7 +78,7 @@ static int get_map_val_dynptr(struct bpf_dynptr *ptr)
|
||||
* bpf_ringbuf_submit/discard_dynptr call
|
||||
*/
|
||||
SEC("?raw_tp")
|
||||
__failure __msg("Unreleased reference id=2")
|
||||
__failure __msg("Unreleased reference id=1")
|
||||
int ringbuf_missing_release1(void *ctx)
|
||||
{
|
||||
struct bpf_dynptr ptr = {};
|
||||
@@ -91,7 +91,7 @@ int ringbuf_missing_release1(void *ctx)
|
||||
}
|
||||
|
||||
SEC("?raw_tp")
|
||||
__failure __msg("Unreleased reference id=4")
|
||||
__failure __msg("Unreleased reference id=3")
|
||||
int ringbuf_missing_release2(void *ctx)
|
||||
{
|
||||
struct bpf_dynptr ptr1, ptr2;
|
||||
|
||||
@@ -30,7 +30,7 @@ int force_clang_to_emit_btf_for_externs(void *ctx)
|
||||
|
||||
SEC("?raw_tp")
|
||||
__success __log_level(2)
|
||||
__msg("fp-8=iter_num(ref_id=1,state=active,depth=0)")
|
||||
__msg("fp-8=iter_num(id=1,state=active,depth=0)")
|
||||
int create_and_destroy(void *ctx)
|
||||
{
|
||||
struct bpf_iter_num iter;
|
||||
@@ -196,7 +196,7 @@ int leak_iter_from_subprog_fail(void *ctx)
|
||||
|
||||
SEC("?raw_tp")
|
||||
__success __log_level(2)
|
||||
__msg("fp-8=iter_num(ref_id=1,state=active,depth=0)")
|
||||
__msg("fp-8=iter_num(id=1,state=active,depth=0)")
|
||||
int valid_stack_reuse(void *ctx)
|
||||
{
|
||||
struct bpf_iter_num iter;
|
||||
|
||||
@@ -20,8 +20,8 @@ __s64 res_empty;
|
||||
|
||||
SEC("raw_tp/sys_enter")
|
||||
__success __log_level(2)
|
||||
__msg("fp-16=iter_testmod_seq(ref_id=1,state=active,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(ref_id=1,state=drained,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(id=1,state=active,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(id=1,state=drained,depth=0)")
|
||||
__msg("call bpf_iter_testmod_seq_destroy")
|
||||
int testmod_seq_empty(const void *ctx)
|
||||
{
|
||||
@@ -38,8 +38,8 @@ __s64 res_full;
|
||||
|
||||
SEC("raw_tp/sys_enter")
|
||||
__success __log_level(2)
|
||||
__msg("fp-16=iter_testmod_seq(ref_id=1,state=active,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(ref_id=1,state=drained,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(id=1,state=active,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(id=1,state=drained,depth=0)")
|
||||
__msg("call bpf_iter_testmod_seq_destroy")
|
||||
int testmod_seq_full(const void *ctx)
|
||||
{
|
||||
@@ -58,8 +58,8 @@ static volatile int zero = 0;
|
||||
|
||||
SEC("raw_tp/sys_enter")
|
||||
__success __log_level(2)
|
||||
__msg("fp-16=iter_testmod_seq(ref_id=1,state=active,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(ref_id=1,state=drained,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(id=1,state=active,depth=0)")
|
||||
__msg("fp-16=iter_testmod_seq(id=1,state=drained,depth=0)")
|
||||
__msg("call bpf_iter_testmod_seq_destroy")
|
||||
int testmod_seq_truncated(const void *ctx)
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user