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

Widening is too aggressive #441

Open
dthaler opened this issue Jan 3, 2023 · 2 comments
Open

Widening is too aggressive #441

dthaler opened this issue Jan 3, 2023 · 2 comments

Comments

@dthaler
Copy link
Contributor

dthaler commented Jan 3, 2023

As @caballa wrote in PR #423:

the problem is that the widening is too aggressive because it jumps directly to -oo or +oo. In ebpf programs we expect that loops are bounded to relatively small numbers so during widening we could jump to smaller numbers (e.g., 1000, 5000, etc) and of course jumping to +oo if convergence is not achieved after few iterations. In this way, we wouldn't give up so early by assuming that arithmetic operations can overflow.
In Crab, we call it widening with thresholds and in fact Prevail borrrowed the code (https://github.com/vbpf/ebpf-verifier/blob/main/src/crab/thresholds.hpp) but AFIK the code is unused. That code actually tries to learn some good integers to jump in during widening by looking at the code. But it might be enough something much simpler. It might be enough to change slightly the interval and zones domains to jump to a small (predefined) number of integers before jump to +oo. With that I think we should handle this example.

Before the fix in PR #423, valid programs passed verification, but now fail because of this issue so it is perceived as a regression.

vbpf/ebpf-samples#32 has a sample program that illustrates this (slightly more complex than Jorge's "essense" in PR #423 so that cmake won't unroll the loop).

@dthaler
Copy link
Contributor Author

dthaler commented Jan 3, 2023

This appears to be related to the TODO comment in
https://github.com/vbpf/ebpf-verifier/blob/main/src/crab/split_dbm.hpp#L337

@dthaler
Copy link
Contributor Author

dthaler commented Jan 3, 2023

SplitDBM::widen() gives the crab log:

Before widening:
DBM 1
[
    meta_offset=[-4098, 0],
    packet_size=[0, 65534],
    r1.type=number,
    r10.stack_offset=512, r10.type=stack, r10.value=[512, 2147418112],
    r2.type=number, r2.value=[0, 1],
    r3.type=number,
    r4.type=number, r4.value=1,
    s[480...487].value=0, s[488...495].value=0, s[496...503].value=0, s[504...511].value=0]
DBM 2
[
    meta_offset=[-4098, 0],
    packet_size=[0, 65534],
    r1.type=number,
    r10.stack_offset=512, r10.type=stack, r10.value=[512, 2147418112],
    r2.type=number, r2.value=[0, 2],
    r3.type=number,
    r4.type=number, r4.value=1,
    s[480...487].value=0, s[488...495].value=0, s[496...503].value=0, s[504...511].value=0]
Result widening:
[
    meta_offset=[-4098, 0],
    packet_size=[0, 65534],
    r1.type=number,
    r10.stack_offset=512, r10.type=stack, r10.value=[512, 2147418112],
    r2.type=number, r2.value=[0, +oo],
    r3.type=number,
    r4.type=number, r4.value=1,
    s[480...487].value=0, s[488...495].value=0, s[496...503].value=0, s[504...511].value=0]

so [0,1] to {0,2] widens to [0, +oo].

Alan-Jowett pushed a commit to Alan-Jowett/ebpf-verifier that referenced this issue Oct 15, 2024
vbpf#441)

Bumps [external/bpf_conformance](https://github.com/Alan-Jowett/bpf_conformance) from `1bc5d19` to `0eed408`.
- [Release notes](https://github.com/Alan-Jowett/bpf_conformance/releases)
- [Commits](Alan-Jowett/bpf_conformance@1bc5d19...0eed408)

---
updated-dependencies:
- dependency-name: external/bpf_conformance
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant