bpf: consolidate pointer offset tracking in var_off#11044
Closed
kernel-patches-daemon-bpf[bot] wants to merge 5 commits intobpf-next_basefrom
Closed
bpf: consolidate pointer offset tracking in var_off#11044kernel-patches-daemon-bpf[bot] wants to merge 5 commits intobpf-next_basefrom
kernel-patches-daemon-bpf[bot] wants to merge 5 commits intobpf-next_basefrom
Conversation
Author
|
Upstream branch: a86c608 |
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
b0043d3 to
3d9d955
Compare
Author
|
Upstream branch: a86c608 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/1053690=>bpf-next
branch
from
2901df0 to
dc4450b
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
3d9d955 to
8c8442a
Compare
Author
|
Upstream branch: 561085f |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/1053690=>bpf-next
branch
from
dc4450b to
6ac520c
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
8c8442a to
a4a9811
Compare
check_reg_sane_offset() is used when verifying operations like: dst_reg += src_reg ^ ^ | '-------- scalar '------------------- pointer To verify range for both dst_reg and src_reg. Split it in two parts: - one to check a pointer offset - another to check scalar offset This would be useful for further refactoring. Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
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 <eddyz87@gmail.com>
Previous commit changed BPF verifier to track pointer offsets entirely in bpf_reg_state->var_off. This commit removes references to bpf_reg_state->off in netronome code. - In most of the places `.off` field is used as a part of a sum: `reg->off + reg->var_off.value`. In such code the `reg->off` addend is removed. - Function jit.c:cross_mem_access() uses `.off` but ignores `.var_off` to compute memory access offset. Here access to `.off` is replaced with access to `.var_off.value`. verifier.c:nfp_bpf_check_ptr() rejects programs for which !tnum_is_const(reg->var_off), so the above change will read a valid constant. Not sure why `.var_off` was ignored in this function previously. Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
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 <eddyz87@gmail.com>
Author
|
Upstream branch: f632de6 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/1053690=>bpf-next
branch
from
6ac520c to
daa9eb9
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
a4a9811 to
2650068
Compare
Author
|
At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1053690 irrelevant now. Closing PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: bpf: consolidate pointer offset tracking in var_off
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1053690