Re: [PATCH bpf-next v3 3/9] bpf: Preserve reg->id of pointer objects after null-check

Andrii Nakryiko
2026-04-24T00:04:41Z

On Tue, Apr 21, 2026 at 3:10 PM Amery Hung <ameryhung@gmail.com> wrote:
>
> Preserve reg->id of pointer objects after null-checking the register so
> that children objects derived from it can still refer to it in the new
> object relationship tracking mechanism introduced in a later patch. This
> change incurs a slight increase in the number of states in one selftest
> bpf object, rbtree_search.bpf.o. For Meta bpf objects, the increase of
> states is also negligible.
>
> Selftest BPF objects with insns_diff > 0
>
> Program Insns (A) Insns (B) Insns (DIFF) States (A) States (B) States (DIFF)
> ------------------------ --------- --------- -------------- ---------- ---------- -------------
> rbtree_search 6820 7326 +506 (+7.42%) 379 398 +19 (+5.01%)
>
> Meta BPF objects with insns_diff > 0
>
> Program Insns (A) Insns (B) Insns (DIFF) States (A) States (B) States (DIFF)
> ------------------------ --------- --------- -------------- ---------- ---------- -------------
> ned_imex_be_tclass 52 57 +5 (+9.62%) 5 6 +1 (+20.00%)
> ned_imex_be_tclass 52 57 +5 (+9.62%) 5 6 +1 (+20.00%)
> ned_skop_auto_flowlabel 523 526 +3 (+0.57%) 39 40 +1 (+2.56%)
> ned_skop_mss 289 292 +3 (+1.04%) 20 20 +0 (+0.00%)
> ned_skopt_bet_classifier 78 82 +4 (+5.13%) 8 8 +0 (+0.00%)
> dctcp_update_alpha 252 320 +68 (+26.98%) 21 27 +6 (+28.57%)
> dctcp_update_alpha 252 320 +68 (+26.98%) 21 27 +6 (+28.57%)
> ned_ts_func 119 126 +7 (+5.88%) 6 7 +1 (+16.67%)
> tw_egress 1119 1128 +9 (+0.80%) 95 96 +1 (+1.05%)
> tw_ingress 1128 1137 +9 (+0.80%) 95 96 +1 (+1.05%)
> tw_tproxy_router 4380 4465 +85 (+1.94%) 114 118 +4 (+3.51%)
> tw_tproxy_router4 3093 3170 +77 (+2.49%) 83 88 +5 (+6.02%)
> ttls_tc_ingress 34656 35717 +1061 (+3.06%) 936 970 +34 (+3.63%)
> tw_twfw_egress 222327 222338 +11 (+0.00%) 10563 10564 +1 (+0.01%)
> tw_twfw_ingress 78295 78299 +4 (+0.01%) 3825 3826 +1 (+0.03%)
> tw_twfw_tc_eg 222839 222859 +20 (+0.01%) 10584 10585 +1 (+0.01%)
> tw_twfw_tc_in 78295 78299 +4 (+0.01%) 3825 3826 +1 (+0.03%)
> tw_twfw_egress 8080 8085 +5 (+0.06%) 456 456 +0 (+0.00%)
> tw_twfw_ingress 8053 8056 +3 (+0.04%) 454 454 +0 (+0.00%)
> tw_twfw_tc_eg 8154 8174 +20 (+0.25%) 456 457 +1 (+0.22%)
> tw_twfw_tc_in 8060 8063 +3 (+0.04%) 455 455 +0 (+0.00%)
> tw_twfw_egress 222327 222338 +11 (+0.00%) 10563 10564 +1 (+0.01%)
> tw_twfw_ingress 78295 78299 +4 (+0.01%) 3825 3826 +1 (+0.03%)
> tw_twfw_tc_eg 222839 222859 +20 (+0.01%) 10584 10585 +1 (+0.01%)
> tw_twfw_tc_in 78295 78299 +4 (+0.01%) 3825 3826 +1 (+0.03%)
> tw_twfw_egress 8080 8085 +5 (+0.06%) 456 456 +0 (+0.00%)
> tw_twfw_ingress 8053 8056 +3 (+0.04%) 454 454 +0 (+0.00%)
> tw_twfw_tc_eg 8154 8174 +20 (+0.25%) 456 457 +1 (+0.22%)
> tw_twfw_tc_in 8060 8063 +3 (+0.04%) 455 455 +0 (+0.00%)
>
> Looking into rbtree_search, the reason for such increase is that the
> verifier has to explore the main loop shown below for one more iteration
> until state pruning decides the current state is safe.
>
> long rbtree_search(void *ctx)
> {
> ...
> bpf_spin_lock(&glock0);
> rb_n = bpf_rbtree_root(&groot0);
> while (can_loop) {
> if (!rb_n) {
> bpf_spin_unlock(&glock0);
> return __LINE__;
> }
>
> n = rb_entry(rb_n, struct node_data, r0);
> if (lookup_key == n->key0)
> break;
> if (nr_gc < NR_NODES)
> gc_ns[nr_gc++] = rb_n;
> if (lookup_key < n->key0)
> rb_n = bpf_rbtree_left(&groot0, rb_n);
> else
> rb_n = bpf_rbtree_right(&groot0, rb_n);
> }
> ...
> }
>
> Below is what the verifier sees at the start of each iteration
> (65: may_goto) after preserving id of rb_n. Without id of rb_n, the
> verifier stops exploring the loop at iter 16.
>
> rb_n gc_ns[15]
> iter 15 257 257
>
> iter 16 290 257 rb_n: idmap add 257->290
> gc_ns[15]: check 257 != 290 --> state not equal
>
> iter 17 325 257 rb_n: idmap add 290->325
> gc_ns[15]: idmap add 257->257 --> state safe
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> kernel/bpf/verifier.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>

totally seems worth doing

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 93003a2a96b0..0313b7d5f6c9 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -15886,15 +15886,10 @@ static void mark_ptr_or_null_reg(struct bpf_func_state *state,
>
> mark_ptr_not_null_reg(reg);
>
> - if (!reg_may_point_to_spin_lock(reg)) {
> - /* For not-NULL ptr, reg->ref_obj_id will be reset
> - * in release_reference().
> - *
> - * reg->id is still used by spin_lock ptr. Other
> - * than spin_lock ptr type, reg->id can be reset.
> - */
> - reg->id = 0;
> - }
> + /*
> + * reg->id is preserved for object relationship tracking
> + * and spin_lock lock state tracking
> + */
> }
> }
>
> --
> 2.52.0
>