Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor verifier prune and jump point handling #4135

Closed
wants to merge 4 commits into from

Conversation

kernel-patches-bot
Copy link

Pull request for series with
subject: Refactor verifier prune and jump point handling
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084

@kernel-patches-bot
Copy link
Author

Upstream branch: 78b037b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: bc067ca
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: bc067ca
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: f16a7aa
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: 7068194
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: 1910676
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: c0c852d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: 8972e18
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: 41d76c7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: 578ce69
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=701084
version: 1

@kernel-patches-bot
Copy link
Author

Upstream branch: aa67961
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=702333
version: 2

@kernel-patches-bot
Copy link
Author

Upstream branch: c21dc52
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=702333
version: 2

@kernel-patches-bot
Copy link
Author

Upstream branch: 235d2ef
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=702333
version: 2

@kernel-patches-bot
Copy link
Author

Upstream branch: 156ed20
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=702333
version: 2

BPF verifier marks some instructions as prune points. Currently these
prune points serve two purposes.

It's a point where verifier tries to find previously verified state and
check current state's equivalence to short circuit verification for
current code path.

But also currently it's a point where jump history, used for precision
backtracking, is updated. This is done so that non-linear flow of
execution could be properly backtracked.

Such coupling is coincidental and unnecessary. Some prune points are not
part of some non-linear jump path, so don't need update of jump history.
On the other hand, not all instructions which have to be recorded in
jump history necessarily are good prune points.

This patch splits prune and jump points into independent flags.
Currently all prune points are marked as jump points to minimize amount
of changes in this patch, but next patch will perform some optimization
of prune vs jmp point placement.

No functional changes are intended.

Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
@kernel-patches-bot
Copy link
Author

Upstream branch: d8939cb
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=702333
version: 2

Jump history updating and state equivalence checks are conceptually
independent, so move push_jmp_history() out of is_state_visited(). Also
make a decision whether to perform state equivalence checks or not one
layer higher in do_check(), keeping is_state_visited() unconditionally
performing state checks.

push_jmp_history() should be performed after state checks. There is just
one small non-uniformity. When is_state_visited() finds already
validated equivalent state, it propagates precision marks to current
state's parent chain. For this to work correctly, jump history has to be
updated, so is_state_visited() is doing that internally.

But if no equivalent verified state is found, jump history has to be
updated in a newly cloned child state, so is_jmp_point()
+ push_jmp_history() is performed after is_state_visited() exited with
zero result, which means "proceed with validation".

This change has no functional changes. It's not strictly necessary, but
feels right to decouple these two processes.

Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Don't mark some instructions as jump points when there are actually no
jumps and instructions are just processed sequentially. Such case is
handled naturally by precision backtracking logic without the need to
update jump history. See get_prev_insn_idx(). It goes back linearly by
one instruction, unless current top of jmp_history is pointing to
current instruction. In such case we use `st->jmp_history[cnt - 1].prev_idx`
to find instruction from which we jumped to the current instruction
non-linearly.

Also remove both jump and prune point marking for instruction right
after unconditional jumps, as program flow can get to the instruction
right after unconditional jump instruction only if there is a jump to
that instruction from somewhere else in the program. In such case we'll
mark such instruction as prune/jump point because it's a destination of
a jump.

This change has no changes in terms of number of instructions or states
processes across Cilium and selftests programs.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
@kernel-patches-bot
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=702333 irrelevant now. Closing PR.

@kernel-patches-bot kernel-patches-bot deleted the series/701084=>bpf-next branch December 7, 2022 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants