Skip to content

Commit 308c7a0

Browse files
ameryhungAlexei Starovoitov
authored andcommitted
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: 870c285 ("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>
1 parent 06d518a commit 308c7a0

11 files changed

Lines changed: 338 additions & 360 deletions

File tree

include/linux/bpf.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,7 +1062,7 @@ struct bpf_insn_access_aux {
10621062
struct {
10631063
struct btf *btf;
10641064
u32 btf_id;
1065-
u32 ref_obj_id;
1065+
u32 ref_id;
10661066
};
10671067
};
10681068
struct bpf_verifier_log *log; /* for verbose logs */
@@ -1631,7 +1631,7 @@ struct bpf_ctx_arg_aux {
16311631
enum bpf_reg_type reg_type;
16321632
struct btf *btf;
16331633
u32 btf_id;
1634-
u32 ref_obj_id;
1634+
u32 ref_id;
16351635
bool refcounted;
16361636
};
16371637

include/linux/bpf_verifier.h

Lines changed: 28 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ struct bpf_reg_state {
6666

6767
struct { /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
6868
u32 mem_size;
69-
u32 dynptr_id; /* for dynptr slices */
7069
};
7170

7271
/* For dynptr stack slots */
@@ -148,46 +147,14 @@ struct bpf_reg_state {
148147
#define BPF_ADD_CONST32 (1U << 30)
149148
#define BPF_ADD_CONST (BPF_ADD_CONST64 | BPF_ADD_CONST32)
150149
u32 id;
151-
/* PTR_TO_SOCKET and PTR_TO_TCP_SOCK could be a ptr returned
152-
* from a pointer-cast helper, bpf_sk_fullsock() and
153-
* bpf_tcp_sock().
154-
*
155-
* Consider the following where "sk" is a reference counted
156-
* pointer returned from "sk = bpf_sk_lookup_tcp();":
157-
*
158-
* 1: sk = bpf_sk_lookup_tcp();
159-
* 2: if (!sk) { return 0; }
160-
* 3: fullsock = bpf_sk_fullsock(sk);
161-
* 4: if (!fullsock) { bpf_sk_release(sk); return 0; }
162-
* 5: tp = bpf_tcp_sock(fullsock);
163-
* 6: if (!tp) { bpf_sk_release(sk); return 0; }
164-
* 7: bpf_sk_release(sk);
165-
* 8: snd_cwnd = tp->snd_cwnd; // verifier will complain
166-
*
167-
* After bpf_sk_release(sk) at line 7, both "fullsock" ptr and
168-
* "tp" ptr should be invalidated also. In order to do that,
169-
* the reg holding "fullsock" and "sk" need to remember
170-
* the original refcounted ptr id (i.e. sk_reg->id) in ref_obj_id
171-
* such that the verifier can reset all regs which have
172-
* ref_obj_id matching the sk_reg->id.
173-
*
174-
* sk_reg->ref_obj_id is set to sk_reg->id at line 1.
175-
* sk_reg->id will stay as NULL-marking purpose only.
176-
* After NULL-marking is done, sk_reg->id can be reset to 0.
177-
*
178-
* After "fullsock = bpf_sk_fullsock(sk);" at line 3,
179-
* fullsock_reg->ref_obj_id is set to sk_reg->ref_obj_id.
180-
*
181-
* After "tp = bpf_tcp_sock(fullsock);" at line 5,
182-
* tp_reg->ref_obj_id is set to fullsock_reg->ref_obj_id
183-
* which is the same as sk_reg->ref_obj_id.
184-
*
185-
* From the verifier perspective, if sk, fullsock and tp
186-
* are not NULL, they are the same ptr with different
187-
* reg->type. In particular, bpf_sk_release(tp) is also
188-
* allowed and has the same effect as bpf_sk_release(sk).
150+
/*
151+
* Tracks the parent object this register was derived from.
152+
* Used for cascading invalidation: when the parent object is
153+
* released or invalidated, all registers with matching parent_id
154+
* are also invalidated. For example, a slice from bpf_dynptr_data()
155+
* gets parent_id set to the dynptr's id.
189156
*/
190-
u32 ref_obj_id;
157+
u32 parent_id;
191158
/* Inside the callee two registers can be both PTR_TO_STACK like
192159
* R1=fp-8 and R2=fp-8, but one of them points to this function stack
193160
* while another to the caller's stack. To differentiate them 'frameno'
@@ -364,10 +331,14 @@ struct bpf_reference_state {
364331
* is used purely to inform the user of a reference leak.
365332
*/
366333
int insn_idx;
367-
/* Use to keep track of the source object of a lock, to ensure
368-
* it matches on unlock.
369-
*/
370-
void *ptr;
334+
union {
335+
/* For REF_TYPE_PTR */
336+
int parent_id;
337+
/* Use to keep track of the source object of a lock, to ensure
338+
* it matches on unlock.
339+
*/
340+
void *ptr;
341+
};
371342
};
372343

373344
struct bpf_retval_range {
@@ -585,23 +556,26 @@ bpf_get_spilled_stack_arg(int slot, struct bpf_func_state *frame)
585556
iter < frame->out_stack_arg_cnt; \
586557
iter++, reg = bpf_get_spilled_stack_arg(iter, frame))
587558

588-
#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr) \
559+
#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __stack, __mask, __expr) \
589560
({ \
590561
struct bpf_verifier_state *___vstate = __vst; \
591562
int ___i, ___j; \
592563
for (___i = 0; ___i <= ___vstate->curframe; ___i++) { \
593564
struct bpf_reg_state *___regs; \
594565
__state = ___vstate->frame[___i]; \
595566
___regs = __state->regs; \
567+
__stack = NULL; \
596568
for (___j = 0; ___j < MAX_BPF_REG; ___j++) { \
597569
__reg = &___regs[___j]; \
598570
(void)(__expr); \
599571
} \
600572
bpf_for_each_spilled_reg(___j, __state, __reg, __mask) { \
601573
if (!__reg) \
602574
continue; \
575+
__stack = &__state->stack[___j]; \
603576
(void)(__expr); \
604577
} \
578+
__stack = NULL; \
605579
bpf_for_each_spilled_stack_arg(___j, __state, __reg) { \
606580
if (!__reg) \
607581
continue; \
@@ -611,8 +585,13 @@ bpf_get_spilled_stack_arg(int slot, struct bpf_func_state *frame)
611585
})
612586

613587
/* Invoke __expr over regsiters in __vst, setting __state and __reg */
614-
#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
615-
bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, 1 << STACK_SPILL, __expr)
588+
#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
589+
({ \
590+
struct bpf_stack_state * ___stack; \
591+
(void)___stack; \
592+
bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, ___stack,\
593+
1 << STACK_SPILL, __expr); \
594+
})
616595

617596
/* linked list of verifier states used to prune search */
618597
struct bpf_verifier_state_list {
@@ -1442,7 +1421,7 @@ struct bpf_map_desc {
14421421
struct bpf_dynptr_desc {
14431422
enum bpf_dynptr_type type;
14441423
u32 id;
1445-
u32 ref_obj_id;
1424+
u32 parent_id;
14461425
};
14471426

14481427
struct bpf_kfunc_call_arg_meta {
@@ -1453,7 +1432,7 @@ struct bpf_kfunc_call_arg_meta {
14531432
const struct btf_type *func_proto;
14541433
const char *func_name;
14551434
/* Out parameters */
1456-
u32 ref_obj_id;
1435+
u32 id;
14571436
u8 release_regno;
14581437
bool r0_rdonly;
14591438
u32 ret_btf_id;

kernel/bpf/btf.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6957,7 +6957,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
69576957
info->reg_type = ctx_arg_info->reg_type;
69586958
info->btf = ctx_arg_info->btf ? : btf_vmlinux;
69596959
info->btf_id = ctx_arg_info->btf_id;
6960-
info->ref_obj_id = ctx_arg_info->ref_obj_id;
6960+
info->ref_id = ctx_arg_info->ref_id;
69616961
return true;
69626962
}
69636963
}

kernel/bpf/fixups.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
870870
case PTR_TO_BTF_ID:
871871
case PTR_TO_BTF_ID | PTR_UNTRUSTED:
872872
/* PTR_TO_BTF_ID | MEM_ALLOC always has a valid lifetime, unlike
873-
* PTR_TO_BTF_ID, and an active ref_obj_id, but the same cannot
873+
* PTR_TO_BTF_ID, and an active referenced id, but the same cannot
874874
* be said once it is marked PTR_UNTRUSTED, hence we must handle
875875
* any faults for loads into such types. BPF_WRITE is disallowed
876876
* for this case.

kernel/bpf/log.c

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -665,8 +665,8 @@ static void print_reg_state(struct bpf_verifier_env *env,
665665
verbose_a("id=%d", reg->id & ~BPF_ADD_CONST);
666666
if (reg->id & BPF_ADD_CONST)
667667
verbose(env, "%+d", reg->delta);
668-
if (reg->ref_obj_id)
669-
verbose_a("ref_obj_id=%d", reg->ref_obj_id);
668+
if (reg->parent_id)
669+
verbose_a("parent_id=%d", reg->parent_id);
670670
if (type_is_non_owning_ref(reg->type))
671671
verbose_a("%s", "non_own_ref");
672672
if (type_is_map_ptr(t)) {
@@ -768,21 +768,19 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifie
768768
verbose(env, "=dynptr_%s(", dynptr_type_str(reg->dynptr.type));
769769
if (reg->id)
770770
verbose_a("id=%d", reg->id);
771-
if (reg->ref_obj_id)
772-
verbose_a("ref_id=%d", reg->ref_obj_id);
773-
if (reg->dynptr_id)
774-
verbose_a("dynptr_id=%d", reg->dynptr_id);
771+
if (reg->parent_id)
772+
verbose_a("parent_id=%d", reg->parent_id);
775773
verbose(env, ")");
776774
break;
777775
case STACK_ITER:
778-
/* only main slot has ref_obj_id set; skip others */
779-
if (!reg->ref_obj_id)
776+
/* only main slot has id set; skip others */
777+
if (!reg->id)
780778
continue;
781779

782-
verbose(env, " fp%d=iter_%s(ref_id=%d,state=%s,depth=%u)",
780+
verbose(env, " fp%d=iter_%s(id=%d,state=%s,depth=%u)",
783781
(-i - 1) * BPF_REG_SIZE,
784782
iter_type_str(reg->iter.btf, reg->iter.btf_id),
785-
reg->ref_obj_id, iter_state_str(reg->iter.state),
783+
reg->id, iter_state_str(reg->iter.state),
786784
reg->iter.depth);
787785
break;
788786
case STACK_MISC:

kernel/bpf/states.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ static bool regs_exact(const struct bpf_reg_state *rold,
489489
{
490490
return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
491491
check_ids(rold->id, rcur->id, idmap) &&
492-
check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
492+
check_ids(rold->parent_id, rcur->parent_id, idmap);
493493
}
494494

495495
enum exact_level {
@@ -614,7 +614,7 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
614614
range_within(rold, rcur) &&
615615
tnum_in(rold->var_off, rcur->var_off) &&
616616
check_ids(rold->id, rcur->id, idmap) &&
617-
check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
617+
check_ids(rold->parent_id, rcur->parent_id, idmap);
618618
case PTR_TO_PACKET_META:
619619
case PTR_TO_PACKET:
620620
/* 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,
794794
cur_reg = &cur->stack[spi].spilled_ptr;
795795
if (old_reg->dynptr.type != cur_reg->dynptr.type ||
796796
old_reg->dynptr.first_slot != cur_reg->dynptr.first_slot ||
797-
!check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap))
797+
!check_ids(old_reg->id, cur_reg->id, idmap) ||
798+
!check_ids(old_reg->parent_id, cur_reg->parent_id, idmap))
798799
return false;
799800
break;
800801
case STACK_ITER:
@@ -810,13 +811,13 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
810811
old_reg->iter.btf_id != cur_reg->iter.btf_id ||
811812
old_reg->iter.state != cur_reg->iter.state ||
812813
/* ignore {old_reg,cur_reg}->iter.depth, see above */
813-
!check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap))
814+
!check_ids(old_reg->id, cur_reg->id, idmap))
814815
return false;
815816
break;
816817
case STACK_IRQ_FLAG:
817818
old_reg = &old->stack[spi].spilled_ptr;
818819
cur_reg = &cur->stack[spi].spilled_ptr;
819-
if (!check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap) ||
820+
if (!check_ids(old_reg->id, cur_reg->id, idmap) ||
820821
old_reg->irq.kfunc_class != cur_reg->irq.kfunc_class)
821822
return false;
822823
break;

0 commit comments

Comments
 (0)