History

bpf: Support 32-bit scalar spills in stacksafe()

v1->v2: updated comments v1: https://lore.kernel.org/bpf/20260322225124.14005-1-alexei.starovoitov@gmail.com/ The commit 6efbde200bf3 ("bpf: Handle scalar spill vs all MISC in stacksafe()") in stacksafe() only recognized full 64-bit scalar spills when comparing stack states for equivalence during state pruning and missed 32-bit scalar spill. When 32-bit scalar is spilled the check_stack_write_fixed_off() -> save_register_state() calls mark_stack_slot_misc() for slot[0-3], which preserves STACK_INVALID and STACK_ZERO (on a fresh stack slot[0-3] remain STACK_INVALID), sets slot[4-7] = STACK_SPILL, and updates spilled_ptr. The im=4 path is only reached when im=0 fails: The loop at im=0 already attempts the 64-bit scalar-spill/all-MISC check. If it matches, i advances by 7, skipping the entire 8-byte slot. So im=4 is only reached when bytes 0-3 are neither a scalar spill nor all-MISC — they must pass individual byte-by-byte comparison first. Then bytes 4-7 get the scalar-unit treatment. is_spilled_scalar_after(stack, 4): slot_type[4] == STACK_SPILL from a 64-bit spill would have been caught at im=0 (unless it's a pointer spill, in which case spilled_ptr.type != SCALAR_VALUE -> returns false at im=4 too). A partial overwrite of a 64-bit spill invalidates the entire slot in check_stack_write_fixed_off(). is_stack_misc_after(stack, 4): Only checks bytes 4-7 are MISC/INVALID, returns &unbound_reg. Comparing two unbound regs via regsafe() is safe. Changes to cilium programs: File Program Insns (A) Insns (B) Insns (DIFF) _______________ _________________________________ _________ _________ ________________ bpf_host.o cil_host_policy 49351 45811 -3540 (-7.17%) bpf_host.o cil_to_host 2384 2270 -114 (-4.78%) bpf_host.o cil_to_netdev 112051 100269 -11782 (-10.51%) bpf_host.o tail_handle_ipv4_cont_from_host 61175 60910 -265 (-0.43%) bpf_host.o tail_handle_ipv4_cont_from_netdev 9381 8873 -508 (-5.42%) bpf_host.o tail_handle_ipv4_from_host 12994 7066 -5928 (-45.62%) bpf_host.o tail_handle_ipv4_from_netdev 85015 59875 -25140 (-29.57%) bpf_host.o tail_handle_ipv6_cont_from_host 24732 23527 -1205 (-4.87%) bpf_host.o tail_handle_ipv6_cont_from_netdev 9463 8953 -510 (-5.39%) bpf_host.o tail_handle_ipv6_from_host 12477 11787 -690 (-5.53%) bpf_host.o tail_handle_ipv6_from_netdev 30814 30017 -797 (-2.59%) bpf_host.o tail_handle_nat_fwd_ipv4 8943 8860 -83 (-0.93%) bpf_host.o tail_handle_snat_fwd_ipv4 64716 61625 -3091 (-4.78%) bpf_host.o tail_handle_snat_fwd_ipv6 48299 30797 -17502 (-36.24%) bpf_host.o tail_ipv4_host_policy_ingress 21591 20017 -1574 (-7.29%) bpf_host.o tail_ipv6_host_policy_ingress 21177 20693 -484 (-2.29%) bpf_host.o tail_nodeport_nat_egress_ipv4 16588 16543 -45 (-0.27%) bpf_host.o tail_nodeport_nat_ingress_ipv4 39200 36116 -3084 (-7.87%) bpf_host.o tail_nodeport_nat_ingress_ipv6 50102 48003 -2099 (-4.19%) bpf_lxc.o tail_handle_ipv4_cont 113092 96891 -16201 (-14.33%) bpf_lxc.o tail_handle_ipv6 6727 6701 -26 (-0.39%) bpf_lxc.o tail_handle_ipv6_cont 25567 21805 -3762 (-14.71%) bpf_lxc.o tail_ipv4_ct_egress 28843 15970 -12873 (-44.63%) bpf_lxc.o tail_ipv4_ct_ingress 16691 10213 -6478 (-38.81%) bpf_lxc.o tail_ipv4_ct_ingress_policy_only 16691 10213 -6478 (-38.81%) bpf_lxc.o tail_ipv4_policy 6776 6622 -154 (-2.27%) bpf_lxc.o tail_ipv4_to_endpoint 7523 7219 -304 (-4.04%) bpf_lxc.o tail_ipv6_ct_egress 10275 9999 -276 (-2.69%) bpf_lxc.o tail_ipv6_ct_ingress 6466 6438 -28 (-0.43%) bpf_lxc.o tail_ipv6_ct_ingress_policy_only 6466 6438 -28 (-0.43%) bpf_lxc.o tail_ipv6_policy 6859 5159 -1700 (-24.78%) bpf_lxc.o tail_ipv6_to_endpoint 7039 4427 -2612 (-37.11%) bpf_lxc.o tail_nodeport_ipv6_dsr 1175 1033 -142 (-12.09%) bpf_lxc.o tail_nodeport_nat_egress_ipv4 16318 16292 -26 (-0.16%) bpf_lxc.o tail_nodeport_nat_ingress_ipv4 18907 18490 -417 (-2.21%) bpf_lxc.o tail_nodeport_nat_ingress_ipv6 14624 14556 -68 (-0.46%) bpf_lxc.o tail_nodeport_rev_dnat_ipv4 4776 4588 -188 (-3.94%) bpf_overlay.o tail_handle_inter_cluster_revsnat 15733 15498 -235 (-1.49%) bpf_overlay.o tail_handle_ipv4 124682 105717 -18965 (-15.21%) bpf_overlay.o tail_handle_ipv6 16201 15801 -400 (-2.47%) bpf_overlay.o tail_handle_snat_fwd_ipv4 21280 19323 -1957 (-9.20%) bpf_overlay.o tail_handle_snat_fwd_ipv6 20824 20822 -2 (-0.01%) bpf_overlay.o tail_nodeport_ipv6_dsr 1175 1033 -142 (-12.09%) bpf_overlay.o tail_nodeport_nat_egress_ipv4 16293 16267 -26 (-0.16%) bpf_overlay.o tail_nodeport_nat_ingress_ipv4 20841 20737 -104 (-0.50%) bpf_overlay.o tail_nodeport_nat_ingress_ipv6 14678 14629 -49 (-0.33%) bpf_sock.o cil_sock4_connect 1678 1623 -55 (-3.28%) bpf_sock.o cil_sock4_sendmsg 1791 1736 -55 (-3.07%) bpf_sock.o cil_sock6_connect 3641 3600 -41 (-1.13%) bpf_sock.o cil_sock6_recvmsg 2048 1899 -149 (-7.28%) bpf_sock.o cil_sock6_sendmsg 3755 3721 -34 (-0.91%) bpf_wireguard.o tail_handle_ipv4 31180 27484 -3696 (-11.85%) bpf_wireguard.o tail_handle_ipv6 12095 11760 -335 (-2.77%) bpf_wireguard.o tail_nodeport_ipv6_dsr 1232 1094 -138 (-11.20%) bpf_wireguard.o tail_nodeport_nat_egress_ipv4 16071 16061 -10 (-0.06%) bpf_wireguard.o tail_nodeport_nat_ingress_ipv4 20804 20565 -239 (-1.15%) bpf_wireguard.o tail_nodeport_nat_ingress_ipv6 13490 12224 -1266 (-9.38%) bpf_xdp.o tail_lb_ipv4 49695 42673 -7022 (-14.13%) bpf_xdp.o tail_lb_ipv6 122683 87896 -34787 (-28.36%) bpf_xdp.o tail_nodeport_ipv6_dsr 1833 1862 +29 (+1.58%) bpf_xdp.o tail_nodeport_nat_egress_ipv4 6999 6990 -9 (-0.13%) bpf_xdp.o tail_nodeport_nat_ingress_ipv4 28903 28780 -123 (-0.43%) bpf_xdp.o tail_nodeport_nat_ingress_ipv6 200361 197771 -2590 (-1.29%) bpf_xdp.o tail_nodeport_rev_dnat_ipv4 4606 4454 -152 (-3.30%) Changes to sched-ext: File Program Insns (A) Insns (B) Insns (DIFF) _________________________ ________________ _________ _________ _______________ scx_arena_selftests.bpf.o arena_selftest 236305 236251 -54 (-0.02%) scx_chaos.bpf.o chaos_dispatch 12282 8013 -4269 (-34.76%) scx_chaos.bpf.o chaos_enqueue 11398 7126 -4272 (-37.48%) scx_chaos.bpf.o chaos_init 3854 3828 -26 (-0.67%) scx_flash.bpf.o flash_init 1015 979 -36 (-3.55%) scx_flatcg.bpf.o fcg_dispatch 1143 1100 -43 (-3.76%) scx_lavd.bpf.o lavd_enqueue 35487 35472 -15 (-0.04%) scx_lavd.bpf.o lavd_init 21127 21107 -20 (-0.09%) scx_p2dq.bpf.o p2dq_enqueue 10210 7854 -2356 (-23.08%) scx_p2dq.bpf.o p2dq_init 3233 3207 -26 (-0.80%) scx_qmap.bpf.o qmap_init 20285 20230 -55 (-0.27%) scx_rusty.bpf.o rusty_select_cpu 1165 1148 -17 (-1.46%) scxtop.bpf.o on_sched_switch 2369 2355 -14 (-0.59%)

Alexei Starovoitov committed 7 months ago

Commit 596bef11 parent 02bcf8e1 file · +21 −17Browse files

Showing the changes to kernel (1 of 1 file). Show all

Unified · Split

kernel/bpf/verifier.c+21−17
@@ -1372,9 +1372,9 @@ static bool is_spilled_scalar_reg(const struct bpf_stack_state *stack) · 1371 unmodified lines
1372 stack->spilled_ptr.type == SCALAR_VALUE;1372 stack->spilled_ptr.type == SCALAR_VALUE;
1373}1373}
1374 1374
1375static bool is_spilled_scalar_reg64(const struct bpf_stack_state *stack)1375static bool is_spilled_scalar_after(const struct bpf_stack_state *stack, int im)
1376{1376{
1377 return stack->slot_type[0] == STACK_SPILL &&1377 return stack->slot_type[im] == STACK_SPILL &&
1378 stack->spilled_ptr.type == SCALAR_VALUE;1378 stack->spilled_ptr.type == SCALAR_VALUE;
1379}1379}
1380 1380
@@ -20020,12 +20020,12 @@ static __init int unbound_reg_init(void) · 18639 unmodified lines
20020}20020}
20021late_initcall(unbound_reg_init);20021late_initcall(unbound_reg_init);
20022 20022
20023static bool is_stack_all_misc(struct bpf_verifier_env *env,20023static bool is_stack_misc_after(struct bpf_verifier_env *env,
20024 struct bpf_stack_state *stack)20024 struct bpf_stack_state *stack, int im)
20025{20025{
20026 u32 i;20026 u32 i;
20027 20027
20028 for (i = 0; i < ARRAY_SIZE(stack->slot_type); ++i) {20028 for (i = im; i < ARRAY_SIZE(stack->slot_type); ++i) {
20029 if ((stack->slot_type[i] == STACK_MISC) ||20029 if ((stack->slot_type[i] == STACK_MISC) ||
20030 (stack->slot_type[i] == STACK_INVALID && env->allow_uninit_stack))20030 (stack->slot_type[i] == STACK_INVALID && env->allow_uninit_stack))
20031 continue;20031 continue;
@@ -20036,12 +20036,12 @@ static bool is_stack_all_misc(struct bpf_verifier_env *env, · 4 unmodified lines
20036}20036}
20037 20037
20038static struct bpf_reg_state *scalar_reg_for_stack(struct bpf_verifier_env *env,20038static struct bpf_reg_state *scalar_reg_for_stack(struct bpf_verifier_env *env,
20039 struct bpf_stack_state *stack)20039 struct bpf_stack_state *stack, int im)
20040{20040{
20041 if (is_spilled_scalar_reg64(stack))20041 if (is_spilled_scalar_after(stack, im))
20042 return &stack->spilled_ptr;20042 return &stack->spilled_ptr;
20043 20043
20044 if (is_stack_all_misc(env, stack))20044 if (is_stack_misc_after(env, stack, im))
20045 return &unbound_reg;20045 return &unbound_reg;
20046 20046
20047 return NULL;20047 return NULL;
@@ -20059,6 +20059,7 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old, · 11 unmodified lines
20059 */20059 */
20060 for (i = 0; i < old->allocated_stack; i++) {20060 for (i = 0; i < old->allocated_stack; i++) {
20061 struct bpf_reg_state *old_reg, *cur_reg;20061 struct bpf_reg_state *old_reg, *cur_reg;
20062 int im = i % BPF_REG_SIZE;
20062 20063
20063 spi = i / BPF_REG_SIZE;20064 spi = i / BPF_REG_SIZE;
20064 20065
@@ -20081,18 +20082,21 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old, · 16 unmodified lines
20081 if (i >= cur->allocated_stack)20082 if (i >= cur->allocated_stack)
20082 return false;20083 return false;
20083 20084
20084 /* 64-bit scalar spill vs all slots MISC and vice versa.20085 /*
20085 * Load from all slots MISC produces unbound scalar.20086 * 64 and 32-bit scalar spills vs MISC/INVALID slots and vice versa.
20087 * Load from MISC/INVALID slots produces unbound scalar.
20086 * Construct a fake register for such stack and call20088 * Construct a fake register for such stack and call
20087 * regsafe() to ensure scalar ids are compared.20089 * regsafe() to ensure scalar ids are compared.
20088 */20090 */
20089 old_reg = scalar_reg_for_stack(env, &old->stack[spi]);20091 if (im == 0 || im == 4) {
20090 cur_reg = scalar_reg_for_stack(env, &cur->stack[spi]);20092 old_reg = scalar_reg_for_stack(env, &old->stack[spi], im);
20091 if (old_reg && cur_reg) {20093 cur_reg = scalar_reg_for_stack(env, &cur->stack[spi], im);
20092 if (!regsafe(env, old_reg, cur_reg, idmap, exact))20094 if (old_reg && cur_reg) {
20093 return false;20095 if (!regsafe(env, old_reg, cur_reg, idmap, exact))
20094 i += BPF_REG_SIZE - 1;20096 return false;
20095 continue;20097 i += (im == 0 ? BPF_REG_SIZE - 1 : 3);
20098 continue;
20099 }
20096 }20100 }
20097 20101
20098 /* if old state was safe with misc data in the stack20102 /* if old state was safe with misc data in the stack