From 022ac075088366b62e130da5e1b200bc93a47191 Mon Sep 17 00:00:00 2001 From: Eduard Zingerman Date: Thu, 12 Feb 2026 13:34:22 -0800 Subject: bpf: use reg->var_off instead of reg->off for pointers This commit consolidates static and varying pointer offset tracking logic. All offsets are now represented solely using `.var_off` and min/max fields. The reasons are twofold: - This simplifies pointer tracking code, as each relevant function needs to check the `.var_off` field anyway. - It makes it easier to widen pointer registers for the purpose of loop convergence checks, by forgoing the `regsafe()` logic demanding `.off` fields to be identical. The changes are spread across many functions and are hard to group into smaller patches. Some of the logical changes include: - Checks in __check_ptr_off_reg() are reordered so that the tnum_is_const() check is done before operating on reg->var_off.value. - check_packet_access() now uses check_mem_region_access() to handle possible 'off' overflow cases. - In check_helper_mem_access() utility functions like check_packet_access() are now called with 'off=0', as these utility functions now account for the complete register offset range. - In check_reg_type() a call to __check_ptr_off_reg() is added before a call to btf_struct_ids_match(). This prevents btf_struct_ids_match() from potentially working on non-constant reg->var_off.value. - regsafe() is relaxed to avoid comparing '.off' field for pointers. As a precaution, the changes are verified in [1] by adding a pass checking that no pointer has non-zero '.off' field on each do_check_insn() iteration. [1] https://github.com/eddyz87/bpf/tree/ptrs-off-migration Notable selftests changes: - `.var_off` value changed because it now combines static and varying offsets. Affected tests: - linked_list/incorrect_node_var_off - linked_list/incorrect_head_var_off2 - verifier_align/packet_variable_offset - Overflowing `smax_value` bound leads to a pointer with big negative or positive offset to be rejected immediately (previously overflowing `rX += const` instruction updated `.off` field avoiding the overflow). Affected tests: - verifier_align/dubious_pointer_arithmetic - verifier_bounds/var_off_insn_off_test1 - Invalid access to packet now reports full offset inside a packet. Affected tests: - verifier_direct_packet_access/test23_x_pkt_ptr_4 - A change in check_mem_region_access() behavior: when register `.smin_value` is negative, it reports "rX min value is negative..." before calling into __check_mem_access() which reports "invalid access to ...". In the tests below, the `.off` field was negative, while `.smin_value` remained positive. This is no longer the case after the changes in this commit. Affected tests: - verifier_gotox/jump_table_invalid_mem_acceess_neg - verifier_helper_packet_access/test15_cls_helper_fail_sub - verifier_helper_value_access/imm_out_of_bound_2 - verifier_helper_value_access/reg_out_of_bound_2 - verifier_meta_access/meta_access_test2 - verifier_value_ptr_arith/known_scalar_from_different_maps - lower_oob_arith_test_1 - value_ptr_known_scalar_3 - access_value_ptr_known_scalar - Usage of check_mem_region_access() instead of __check_mem_access() in check_packet_access() changes the reported message from "rX offset is outside ..." to "rX min/max value is outside ...". Affected tests: - verifier_xdp_direct_packet_access/* - In check_func_arg_reg_off() the check for zero offset now operates on `.var_off` field instead of `.off` field. For tests where the pattern looks like `kfunc(reg_with_var_off, ...)`, this changes the reported error: - previously the error "variable ... access ... disallowed" was reported by __check_ptr_off_reg(); - now "R1 must have zero offset ..." is reported by check_func_arg_reg_off() itself. Affected tests: - verifier/calls.c "calls: invalid kfunc call: PTR_TO_BTF_ID with variable offset" Signed-off-by: Eduard Zingerman Link: https://lore.kernel.org/r/20260212-ptrs-off-migration-v2-2-00820e4d3438@gmail.com Signed-off-by: Alexei Starovoitov --- include/linux/bpf_verifier.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'include/linux') diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index ef8e45a362d9..a97bdbf3a07b 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -38,8 +38,7 @@ struct bpf_reg_state { /* Ordering of fields matters. See states_equal() */ enum bpf_reg_type type; /* - * Fixed part of pointer offset, pointer types only. - * Or constant delta between "linked" scalars with the same ID. + * Constant delta between "linked" scalars with the same ID. */ s32 off; union { -- cgit v1.2.3 From 3d91c618aca403a7f7d2064272f528a97b849475 Mon Sep 17 00:00:00 2001 From: Eduard Zingerman Date: Thu, 12 Feb 2026 13:34:24 -0800 Subject: bpf: rename bpf_reg_state->off to bpf_reg_state->delta This field is now used only for linked scalar registers tracking. Rename it to 'delta' to better describe it's purpose: constant delta between "linked" scalars with the same ID. Signed-off-by: Eduard Zingerman Link: https://lore.kernel.org/r/20260212-ptrs-off-migration-v2-4-00820e4d3438@gmail.com Signed-off-by: Alexei Starovoitov --- include/linux/bpf_verifier.h | 6 +++--- kernel/bpf/log.c | 8 ++++---- kernel/bpf/verifier.c | 18 +++++++++--------- 3 files changed, 16 insertions(+), 16 deletions(-) (limited to 'include/linux') diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index a97bdbf3a07b..c1e30096ea7b 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -40,7 +40,7 @@ struct bpf_reg_state { /* * Constant delta between "linked" scalars with the same ID. */ - s32 off; + s32 delta; union { /* valid when type == PTR_TO_PACKET */ int range; @@ -145,9 +145,9 @@ struct bpf_reg_state { * Upper bit of ID is used to remember relationship between "linked" * registers. Example: * r1 = r2; both will have r1->id == r2->id == N - * r1 += 10; r1->id == N | BPF_ADD_CONST and r1->off == 10 + * r1 += 10; r1->id == N | BPF_ADD_CONST and r1->delta == 10 * r3 = r2; both will have r3->id == r2->id == N - * w3 += 10; r3->id == N | BPF_ADD_CONST32 and r3->off == 10 + * w3 += 10; r3->id == N | BPF_ADD_CONST32 and r3->delta == 10 */ #define BPF_ADD_CONST64 (1U << 31) #define BPF_ADD_CONST32 (1U << 30) diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c index 39a731392d65..37d72b052192 100644 --- a/kernel/bpf/log.c +++ b/kernel/bpf/log.c @@ -694,7 +694,7 @@ static void print_reg_state(struct bpf_verifier_env *env, if (state->frameno != reg->frameno) verbose(env, "[%d]", reg->frameno); if (tnum_is_const(reg->var_off)) { - verbose_snum(env, reg->var_off.value + reg->off); + verbose_snum(env, reg->var_off.value + reg->delta); return; } } @@ -704,7 +704,7 @@ static void print_reg_state(struct bpf_verifier_env *env, if (reg->id) verbose_a("id=%d", reg->id & ~BPF_ADD_CONST); if (reg->id & BPF_ADD_CONST) - verbose(env, "%+d", reg->off); + verbose(env, "%+d", reg->delta); if (reg->ref_obj_id) verbose_a("ref_obj_id=%d", reg->ref_obj_id); if (type_is_non_owning_ref(reg->type)) @@ -716,9 +716,9 @@ static void print_reg_state(struct bpf_verifier_env *env, reg->map_ptr->key_size, reg->map_ptr->value_size); } - if (t != SCALAR_VALUE && reg->off) { + if (t != SCALAR_VALUE && reg->delta) { verbose_a("off="); - verbose_snum(env, reg->off); + verbose_snum(env, reg->delta); } if (type_is_pkt_pointer(t)) { verbose_a("r="); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 2c5794dad668..0162f946032f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -5095,7 +5095,7 @@ static void assign_scalar_id_before_mov(struct bpf_verifier_env *env, * Cleared it, since multiple rX += const are not supported. */ src_reg->id = 0; - src_reg->off = 0; + src_reg->delta = 0; } if (!src_reg->id && !tnum_is_const(src_reg->var_off)) @@ -16219,14 +16219,14 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env, * we cannot accumulate another val into rx->off. */ clear_id: - dst_reg->off = 0; + dst_reg->delta = 0; dst_reg->id = 0; } else { if (alu32) dst_reg->id |= BPF_ADD_CONST32; else dst_reg->id |= BPF_ADD_CONST64; - dst_reg->off = off; + dst_reg->delta = off; } } else { /* @@ -17305,18 +17305,18 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s if ((reg->id & ~BPF_ADD_CONST) != (known_reg->id & ~BPF_ADD_CONST)) continue; if ((!(reg->id & BPF_ADD_CONST) && !(known_reg->id & BPF_ADD_CONST)) || - reg->off == known_reg->off) { + reg->delta == known_reg->delta) { s32 saved_subreg_def = reg->subreg_def; copy_register_state(reg, known_reg); reg->subreg_def = saved_subreg_def; } else { s32 saved_subreg_def = reg->subreg_def; - s32 saved_off = reg->off; + s32 saved_off = reg->delta; u32 saved_id = reg->id; fake_reg.type = SCALAR_VALUE; - __mark_reg_known(&fake_reg, (s64)reg->off - (s64)known_reg->off); + __mark_reg_known(&fake_reg, (s64)reg->delta - (s64)known_reg->delta); /* reg = known_reg; reg += delta */ copy_register_state(reg, known_reg); @@ -17324,7 +17324,7 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s * Must preserve off, id and subreg_def flag, * otherwise another sync_linked_regs() will be incorrect. */ - reg->off = saved_off; + reg->delta = saved_off; reg->id = saved_id; reg->subreg_def = saved_subreg_def; @@ -19629,7 +19629,7 @@ static void clear_singular_ids(struct bpf_verifier_env *env, continue; if (idset_cnt_get(idset, reg->id & ~BPF_ADD_CONST) == 1) { reg->id = 0; - reg->off = 0; + reg->delta = 0; } })); } @@ -19766,7 +19766,7 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold, return false; /* Both have offset linkage: offsets must match */ - if ((rold->id & BPF_ADD_CONST) && rold->off != rcur->off) + if ((rold->id & BPF_ADD_CONST) && rold->delta != rcur->delta) return false; if (!check_scalar_ids(rold->id, rcur->id, idmap)) -- cgit v1.2.3