tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'bpf-verifier-improve-state-pruning-for-scalar-registers'

Puranjay Mohan says:

====================
bpf: Improve state pruning for scalar registers

V2: https://lore.kernel.org/all/20260203022229.1630849-1-puranjay@kernel.org/
Changes in V3:
- Fix spelling mistakes in commit logs (AI)
- Fix an incorrect comment in the selftest added in patch 5 (AI)
- Improve the title of patch 5

V1: https://lore.kernel.org/all/20260202104414.3103323-1-puranjay@kernel.org/
Changes in V2:
- Collected acked by Eduard
- Removed some unnecessary comments
- Added a selftest for id=0 equivalence in Patch 5

This series improves BPF verifier state pruning by relaxing scalar ID
equivalence requirements. Scalar register IDs are used to track
relationships between registers for bounds propagation. However, once
an ID becomes "singular" (only one register/stack slot carries it), it
can no longer participate in bounds propagation and becomes stale.
These stale IDs can prevent pruning of otherwise equivalent states.

The series addresses this in four patches:

Patch 1: Assign IDs on stack fills to ensure stack slots have IDs
before being read into registers, preparing for the singular ID
clearing in patch 2.

Patch 2: Clear IDs that appear only once before caching, as they cannot
contribute to bounds propagation.

Patch 3: Relax maybe_widen_reg() to only compare value-tracking fields
(bounds, tnum, var_off) rather than also requiring ID matches. Two
scalars with identical value constraints but different IDs represent
the same abstract value and don't need widening.

Patch 4: Relax scalar ID equivalence in state comparison by treating
rold->id == 0 as "independent". If the old state didn't rely on ID
relationships for a register, any linking in the current state only
adds constraints and is safe to accept for pruning.

Patch 5: Add a selftest to show the exact case being handled by Patch 4

I ran veristat on BPF programs from sched_ext, meta's internal programs,
and on selftest programs, showing programs with insn diff > 5%:

Scx Progs
File Program States (A) States (B) States (DIFF) Insns (A) Insns (B) Insns (DIFF)
------------------ ------------------- ---------- ---------- ------------- --------- --------- ---------------
scx_rusty.bpf.o rusty_set_cpumask 320 230 -90 (-28.12%) 4478 3259 -1219 (-27.22%)
scx_bpfland.bpf.o bpfland_select_cpu 55 49 -6 (-10.91%) 691 618 -73 (-10.56%)
scx_beerland.bpf.o beerland_select_cpu 27 25 -2 (-7.41%) 320 295 -25 (-7.81%)
scx_p2dq.bpf.o p2dq_init 265 250 -15 (-5.66%) 3423 3233 -190 (-5.55%)
scx_layered.bpf.o layered_enqueue 1461 1386 -75 (-5.13%) 14541 13792 -749 (-5.15%)

FB Progs
File Program States (A) States (B) States (DIFF) Insns (A) Insns (B) Insns (DIFF)
------------ ------------------- ---------- ---------- -------------- --------- --------- ---------------
bpf007.bpf.o bpfj_free 1726 1342 -384 (-22.25%) 25671 19096 -6575 (-25.61%)
bpf041.bpf.o armr_net_block_init 22373 20411 -1962 (-8.77%) 651697 602873 -48824 (-7.49%)
bpf227.bpf.o layered_quiescent 28 26 -2 (-7.14%) 365 340 -25 (-6.85%)
bpf248.bpf.o p2dq_init 263 248 -15 (-5.70%) 3370 3159 -211 (-6.26%)
bpf254.bpf.o p2dq_init 263 248 -15 (-5.70%) 3388 3177 -211 (-6.23%)
bpf241.bpf.o p2dq_init 264 249 -15 (-5.68%) 3428 3240 -188 (-5.48%)
bpf230.bpf.o p2dq_init 287 271 -16 (-5.57%) 3666 3431 -235 (-6.41%)
bpf251.bpf.o lavd_cpu_offline 321 316 -5 (-1.56%) 6221 5891 -330 (-5.30%)
bpf251.bpf.o lavd_cpu_online 321 316 -5 (-1.56%) 6219 5889 -330 (-5.31%)

Selftest Progs
File Program States (A) States (B) States (DIFF) Insns (A) Insns (B) Insns (DIFF)
---------------------------------- ----------------- ---------- ---------- ------------- --------- --------- ---------------
verifier_iterating_callbacks.bpf.o test2 4 2 -2 (-50.00%) 29 18 -11 (-37.93%)
verifier_iterating_callbacks.bpf.o test3 4 2 -2 (-50.00%) 31 19 -12 (-38.71%)
strobemeta_bpf_loop.bpf.o on_event 318 221 -97 (-30.50%) 3938 2755 -1183 (-30.04%)
bpf_qdisc_fq.bpf.o bpf_fq_dequeue 133 105 -28 (-21.05%) 1686 1385 -301 (-17.85%)
iters.bpf.o delayed_read_mark 6 5 -1 (-16.67%) 60 46 -14 (-23.33%)
arena_strsearch.bpf.o arena_strsearch 107 106 -1 (-0.93%) 1394 1258 -136 (-9.76%)
====================

Link: https://patch.msgid.link/20260203165102.2302462-1-puranjay@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Alexei Starovoitov f941479a d95d76aa

+199 -25
+5 -2
include/linux/bpf_verifier.h
··· 697 697 }; 698 698 699 699 struct bpf_idset { 700 - u32 count; 701 - u32 ids[BPF_ID_MAP_SIZE]; 700 + u32 num_ids; 701 + struct { 702 + u32 id; 703 + u32 cnt; 704 + } entries[BPF_ID_MAP_SIZE]; 702 705 }; 703 706 704 707 /* see verifier.c:compute_scc_callchain() */
+144 -20
kernel/bpf/verifier.c
··· 5518 5518 */ 5519 5519 s32 subreg_def = state->regs[dst_regno].subreg_def; 5520 5520 5521 + if (env->bpf_capable && size == 4 && spill_size == 4 && 5522 + get_reg_width(reg) <= 32) 5523 + /* Ensure stack slot has an ID to build a relation 5524 + * with the destination register on fill. 5525 + */ 5526 + assign_scalar_id_before_mov(env, reg); 5521 5527 copy_register_state(&state->regs[dst_regno], reg); 5522 5528 state->regs[dst_regno].subreg_def = subreg_def; 5523 5529 ··· 5569 5563 } 5570 5564 } else if (dst_regno >= 0) { 5571 5565 /* restore register state from stack */ 5566 + if (env->bpf_capable) 5567 + /* Ensure stack slot has an ID to build a relation 5568 + * with the destination register on fill. 5569 + */ 5570 + assign_scalar_id_before_mov(env, reg); 5572 5571 copy_register_state(&state->regs[dst_regno], reg); 5573 5572 /* mark reg as written since spilled pointer state likely 5574 5573 * has its liveness marks cleared by is_state_visited() ··· 8995 8984 const struct bpf_reg_state *rcur, 8996 8985 struct bpf_idmap *idmap); 8997 8986 8987 + /* 8988 + * Check if scalar registers are exact for the purpose of not widening. 8989 + * More lenient than regs_exact() 8990 + */ 8991 + static bool scalars_exact_for_widen(const struct bpf_reg_state *rold, 8992 + const struct bpf_reg_state *rcur) 8993 + { 8994 + return !memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)); 8995 + } 8996 + 8998 8997 static void maybe_widen_reg(struct bpf_verifier_env *env, 8999 - struct bpf_reg_state *rold, struct bpf_reg_state *rcur, 9000 - struct bpf_idmap *idmap) 8998 + struct bpf_reg_state *rold, struct bpf_reg_state *rcur) 9001 8999 { 9002 9000 if (rold->type != SCALAR_VALUE) 9003 9001 return; 9004 9002 if (rold->type != rcur->type) 9005 9003 return; 9006 - if (rold->precise || rcur->precise || regs_exact(rold, rcur, idmap)) 9004 + if (rold->precise || rcur->precise || scalars_exact_for_widen(rold, rcur)) 9007 9005 return; 9008 9006 __mark_reg_unknown(env, rcur); 9009 9007 } ··· 9024 9004 struct bpf_func_state *fold, *fcur; 9025 9005 int i, fr, num_slots; 9026 9006 9027 - reset_idmap_scratch(env); 9028 9007 for (fr = old->curframe; fr >= 0; fr--) { 9029 9008 fold = old->frame[fr]; 9030 9009 fcur = cur->frame[fr]; ··· 9031 9012 for (i = 0; i < MAX_BPF_REG; i++) 9032 9013 maybe_widen_reg(env, 9033 9014 &fold->regs[i], 9034 - &fcur->regs[i], 9035 - &env->idmap_scratch); 9015 + &fcur->regs[i]); 9036 9016 9037 9017 num_slots = min(fold->allocated_stack / BPF_REG_SIZE, 9038 9018 fcur->allocated_stack / BPF_REG_SIZE); ··· 9042 9024 9043 9025 maybe_widen_reg(env, 9044 9026 &fold->stack[i].spilled_ptr, 9045 - &fcur->stack[i].spilled_ptr, 9046 - &env->idmap_scratch); 9027 + &fcur->stack[i].spilled_ptr); 9047 9028 } 9048 9029 } 9049 9030 return 0; ··· 19387 19370 return false; 19388 19371 } 19389 19372 19390 - /* Similar to check_ids(), but allocate a unique temporary ID 19391 - * for 'old_id' or 'cur_id' of zero. 19392 - * This makes pairs like '0 vs unique ID', 'unique ID vs 0' valid. 19373 + /* 19374 + * Compare scalar register IDs for state equivalence. 19375 + * 19376 + * When old_id == 0, the old register is independent - not linked to any 19377 + * other register. Any linking in the current state only adds constraints, 19378 + * making it more restrictive. Since the old state didn't rely on any ID 19379 + * relationships for this register, it's always safe to accept cur regardless 19380 + * of its ID. Hence, return true immediately. 19381 + * 19382 + * When old_id != 0 but cur_id == 0, we need to ensure that different 19383 + * independent registers in cur don't incorrectly satisfy the ID matching 19384 + * requirements of linked registers in old. 19385 + * 19386 + * Example: if old has r6.id=X and r7.id=X (linked), but cur has r6.id=0 19387 + * and r7.id=0 (both independent), without temp IDs both would map old_id=X 19388 + * to cur_id=0 and pass. With temp IDs: r6 maps X->temp1, r7 tries to map 19389 + * X->temp2, but X is already mapped to temp1, so the check fails correctly. 19393 19390 */ 19394 19391 static bool check_scalar_ids(u32 old_id, u32 cur_id, struct bpf_idmap *idmap) 19395 19392 { 19396 - old_id = old_id ? old_id : ++idmap->tmp_id_gen; 19393 + if (!old_id) 19394 + return true; 19395 + 19397 19396 cur_id = cur_id ? cur_id : ++idmap->tmp_id_gen; 19398 19397 19399 19398 return check_ids(old_id, cur_id, idmap); ··· 19483 19450 * doesn't meant that the states are DONE. The verifier has to compare 19484 19451 * the callsites 19485 19452 */ 19453 + 19454 + /* Find id in idset and increment its count, or add new entry */ 19455 + static void idset_cnt_inc(struct bpf_idset *idset, u32 id) 19456 + { 19457 + u32 i; 19458 + 19459 + for (i = 0; i < idset->num_ids; i++) { 19460 + if (idset->entries[i].id == id) { 19461 + idset->entries[i].cnt++; 19462 + return; 19463 + } 19464 + } 19465 + /* New id */ 19466 + if (idset->num_ids < BPF_ID_MAP_SIZE) { 19467 + idset->entries[idset->num_ids].id = id; 19468 + idset->entries[idset->num_ids].cnt = 1; 19469 + idset->num_ids++; 19470 + } 19471 + } 19472 + 19473 + /* Find id in idset and return its count, or 0 if not found */ 19474 + static u32 idset_cnt_get(struct bpf_idset *idset, u32 id) 19475 + { 19476 + u32 i; 19477 + 19478 + for (i = 0; i < idset->num_ids; i++) { 19479 + if (idset->entries[i].id == id) 19480 + return idset->entries[i].cnt; 19481 + } 19482 + return 0; 19483 + } 19484 + 19485 + /* 19486 + * Clear singular scalar ids in a state. 19487 + * A register with a non-zero id is called singular if no other register shares 19488 + * the same base id. Such registers can be treated as independent (id=0). 19489 + */ 19490 + static void clear_singular_ids(struct bpf_verifier_env *env, 19491 + struct bpf_verifier_state *st) 19492 + { 19493 + struct bpf_idset *idset = &env->idset_scratch; 19494 + struct bpf_func_state *func; 19495 + struct bpf_reg_state *reg; 19496 + 19497 + idset->num_ids = 0; 19498 + 19499 + bpf_for_each_reg_in_vstate(st, func, reg, ({ 19500 + if (reg->type != SCALAR_VALUE) 19501 + continue; 19502 + if (!reg->id) 19503 + continue; 19504 + idset_cnt_inc(idset, reg->id & ~BPF_ADD_CONST); 19505 + })); 19506 + 19507 + bpf_for_each_reg_in_vstate(st, func, reg, ({ 19508 + if (reg->type != SCALAR_VALUE) 19509 + continue; 19510 + if (!reg->id) 19511 + continue; 19512 + if (idset_cnt_get(idset, reg->id & ~BPF_ADD_CONST) == 1) { 19513 + reg->id = 0; 19514 + reg->off = 0; 19515 + } 19516 + })); 19517 + } 19518 + 19486 19519 static void clean_live_states(struct bpf_verifier_env *env, int insn, 19487 19520 struct bpf_verifier_state *cur) 19488 19521 { ··· 19634 19535 } 19635 19536 if (!rold->precise && exact == NOT_EXACT) 19636 19537 return true; 19637 - if ((rold->id & BPF_ADD_CONST) != (rcur->id & BPF_ADD_CONST)) 19638 - return false; 19639 - if ((rold->id & BPF_ADD_CONST) && (rold->off != rcur->off)) 19640 - return false; 19641 - /* Why check_ids() for scalar registers? 19538 + /* 19539 + * Linked register tracking uses rold->id to detect relationships. 19540 + * When rold->id == 0, the register is independent and any linking 19541 + * in rcur only adds constraints. When rold->id != 0, we must verify 19542 + * id mapping and (for BPF_ADD_CONST) offset consistency. 19543 + * 19544 + * +------------------+-----------+------------------+---------------+ 19545 + * | | rold->id | rold + ADD_CONST | rold->id == 0 | 19546 + * |------------------+-----------+------------------+---------------| 19547 + * | rcur->id | range,ids | false | range | 19548 + * | rcur + ADD_CONST | false | range,ids,off | range | 19549 + * | rcur->id == 0 | range,ids | false | range | 19550 + * +------------------+-----------+------------------+---------------+ 19551 + * 19552 + * Why check_ids() for scalar registers? 19642 19553 * 19643 19554 * Consider the following BPF code: 19644 19555 * 1: r6 = ... unbound scalar, ID=a ... ··· 19672 19563 * --- 19673 19564 * Also verify that new value satisfies old value range knowledge. 19674 19565 */ 19675 - return range_within(rold, rcur) && 19676 - tnum_in(rold->var_off, rcur->var_off) && 19677 - check_scalar_ids(rold->id, rcur->id, idmap); 19566 + 19567 + /* ADD_CONST mismatch: different linking semantics */ 19568 + if ((rold->id & BPF_ADD_CONST) && !(rcur->id & BPF_ADD_CONST)) 19569 + return false; 19570 + 19571 + if (rold->id && !(rold->id & BPF_ADD_CONST) && (rcur->id & BPF_ADD_CONST)) 19572 + return false; 19573 + 19574 + /* Both have offset linkage: offsets must match */ 19575 + if ((rold->id & BPF_ADD_CONST) && rold->off != rcur->off) 19576 + return false; 19577 + 19578 + if (!check_scalar_ids(rold->id, rcur->id, idmap)) 19579 + return false; 19580 + 19581 + return range_within(rold, rcur) && tnum_in(rold->var_off, rcur->var_off); 19678 19582 case PTR_TO_MAP_KEY: 19679 19583 case PTR_TO_MAP_VALUE: 19680 19584 case PTR_TO_MEM: ··· 20569 20447 /* forget precise markings we inherited, see __mark_chain_precision */ 20570 20448 if (env->bpf_capable) 20571 20449 mark_all_scalars_imprecise(env, cur); 20450 + 20451 + clear_singular_ids(env, cur); 20572 20452 20573 20453 /* add new state to the head of linked list */ 20574 20454 new = &new_sl->state;
+50 -3
tools/testing/selftests/bpf/progs/verifier_scalar_ids.c
··· 715 715 : __clobber_all); 716 716 } 717 717 718 + /* Check that two registers with 0 scalar IDs in a verified state can be mapped 719 + * to the same scalar ID in current state. 720 + */ 721 + SEC("socket") 722 + __success __log_level(2) 723 + /* The states should be equivalent on reaching insn 12. 724 + */ 725 + __msg("12: safe") 726 + __msg("processed 17 insns") 727 + __flag(BPF_F_TEST_STATE_FREQ) 728 + __naked void two_nil_old_ids_one_cur_id(void) 729 + { 730 + asm volatile ( 731 + /* Give unique scalar IDs to r{6,7} */ 732 + "call %[bpf_ktime_get_ns];" 733 + "r0 &= 0xff;" 734 + "r6 = r0;" 735 + "r6 *= 1;" 736 + "call %[bpf_ktime_get_ns];" 737 + "r0 &= 0xff;" 738 + "r7 = r0;" 739 + "r7 *= 1;" 740 + "r0 = 0;" 741 + /* Maybe make r{6,7} IDs identical */ 742 + "if r6 > r7 goto l0_%=;" 743 + "goto l1_%=;" 744 + "l0_%=:" 745 + "r6 = r7;" 746 + "l1_%=:" 747 + /* Mark r{6,7} precise. 748 + * Get here in two states: 749 + * - first: r6{.id=0}, r7{.id=0} (cached state) 750 + * - second: r6{.id=A}, r7{.id=A} 751 + * Verifier considers such states equivalent. 752 + * Thus "exit;" would be verified only once. 753 + */ 754 + "r2 = r10;" 755 + "r2 += r6;" 756 + "r2 += r7;" 757 + "exit;" 758 + : 759 + : __imm(bpf_ktime_get_ns) 760 + : __clobber_all); 761 + } 762 + 718 763 /* Check that two different scalar IDs in a verified state can't be 719 764 * mapped to the same scalar ID in current state. 720 765 */ ··· 768 723 /* The exit instruction should be reachable from two states, 769 724 * use two matches and "processed .. insns" to ensure this. 770 725 */ 771 - __msg("13: (95) exit") 772 - __msg("13: (95) exit") 773 - __msg("processed 18 insns") 726 + __msg("15: (95) exit") 727 + __msg("15: (95) exit") 728 + __msg("processed 20 insns") 774 729 __flag(BPF_F_TEST_STATE_FREQ) 775 730 __naked void two_old_ids_one_cur_id(void) 776 731 { ··· 779 734 "call %[bpf_ktime_get_ns];" 780 735 "r0 &= 0xff;" 781 736 "r6 = r0;" 737 + "r8 = r0;" 782 738 "call %[bpf_ktime_get_ns];" 783 739 "r0 &= 0xff;" 784 740 "r7 = r0;" 741 + "r9 = r0;" 785 742 "r0 = 0;" 786 743 /* Maybe make r{6,7} IDs identical */ 787 744 "if r6 > r7 goto l0_%=;"