draft: sanitize_check_bounds test #8966
Open
+10
−4
Conversation
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
luisgerhorst
force-pushed
the
scb
branch
3 times, most recently
from
9028adc to
7e17351
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
2 times, most recently
from
49174bd to
f3a4188
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
3 times, most recently
from
9857a0f to
b93b30b
Compare
luisgerhorst
force-pushed
the
scb
branch
2 times, most recently
from
3ecbfab to
0f543ec
Compare
As is, it appears as if ptr alu is allowed for everything except
PTR_TO_{STACK,MAP_VALUE} if one only reads sanitize_check_bounds().
However, this is misleading as it only works together with
retrieve_ptr_limit() and the two must be kept in sync. This patch
documents the interdependency and adds a check to ensure they stay in
sync.
Because the preceeding switch returns -EACCES for every opcode except
for ADD/SUB, the sanitize_needed() following the sanitize_check_bounds()
call is always true if reached. This means, unless
sanitize_check_bounds() detected that the pointer goes OOB because of
the ADD/SUB (and returns -EACCES), sanitize_ptr_alu() always executes
after sanitize_check_bounds().
In sanitize_ptr_alu(commit_window = true), we always run
retrieve_ptr_limit() unless:
* can_skip_alu_sanititation() is true, i.e., noteably
`BPF_SRC(insn->code) == BPF_K`. BPF_K is fine because it means that
there is no scalar register (which could be subject to spec. scalar
confusion due to v4) that goes into the alu op. The ptr reg can not be
subject to v4-based value confusion due to the nospec added.
* We are on a speculative path (`vstate->speculative`). This is because
there are no alu sanitization limits to be learned from speculative
paths.
retrieve_ptr_limit() only allows the ALU op if the involved ptr reg (can
be either rhs or lhs for ADD) is PTR_TO_STACK or PTR_TO_MAP_VALUE.
Otherwise it returns -EOPNOTSUPP.
In summary, sanitize_check_bounds() returning 0 for
non-PTR_TO_{STACK,MAP_VALUE} is fine because retrieve_ptr_limit() still
prevents the unsafe ops.
We allow unsanitized ptr arith with ADD/SUB for
* ptr -=/+= imm32, i.e. `BPF_SRC(insn->code) == BPF_K`
* PTR_TO_{STACK,MAP_VALUE} -= scalar
* PTR_TO_{STACK,MAP_VALUE} += scalar
* scalar += PTR_TO_{STACK,MAP_VALUE}
if the requirements from retrieve_ptr_limit() AND
sanitize_check_bounds() hold.
To document this interdependency and ensure they stay in sync, add a
verifier_bug_if().
Signed-off-by: Luis Gerhorst <[email protected]>
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
2 times, most recently
from
f4bae1b to
b8dcdbb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.