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

Fixes for stack with allow_ptr_leaks #8133

Open
wants to merge 4 commits into
base: bpf-next_base
Choose a base branch
from

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: Fixes for stack with allow_ptr_leaks
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=912895

@kernel-patches-daemon-bpf
Copy link
Author

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

kkdwivedi and others added 4 commits November 27, 2024 13:25
Inside mark_stack_slot_misc, we should not upgrade STACK_INVALID to
STACK_MISC when allow_ptr_leaks is false, since invalid contents
shouldn't be read unless the program has the relevant capabilities.
The relaxation only makes sense when env->allow_ptr_leaks is true.

Currently, the condition is inverted (i.e. checking for true instead of
false), simply invert it to restore correct behavior.

Update error strings of selftests relying on current behavior's verifier
output.

Fixes: eaf18fe ("bpf: preserve STACK_ZERO slots on partial reg spills")
Reported-by: Tao Lyu <[email protected]>
Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
When CAP_PERFMON and CAP_SYS_ADMIN (allow_ptr_leaks) are disabled, the
verifier aims to reject partial overwrite on an 8-byte stack slot that
contains a spilled pointer.

However, in such a scenario, it rejects all partial stack overwrites as
long as the targeted stack slot is a spilled register, because it does
not check if the stack slot is a spilled pointer.

Incomplete checks will result in the rejection of valid programs, which
spill narrower scalar values onto scalar slots, as shown below.

0: R1=ctx() R10=fp0
; asm volatile ( @ repro.bpf.c:679
0: (7a) *(u64 *)(r10 -8) = 1          ; R10=fp0 fp-8_w=1
1: (62) *(u32 *)(r10 -8) = 1
attempt to corrupt spilled pointer on stack
processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0.

Fix this by expanding the check to not consider spilled scalar registers
when rejecting the write into the stack.

Previous discussion on this patch is at link [0].

  [0]: https://lore.kernel.org/bpf/[email protected]

Fixes: ab125ed ("bpf: fix check for attempt to corrupt spilled pointer")
Signed-off-by: Tao Lyu <[email protected]>
Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
Ensure that when CAP_PERFMON is dropped, and the verifier sees
allow_ptr_leaks as false, we are not permitted to read from a
STACK_INVALID slot. Without the fix, the test will report unexpected
success in loading.

Since we need to control the capabilities when loading this test to only
retain CAP_BPF, refactor support added to do the same for
test_verifier_mtu and reuse it for this selftest to avoid copy-paste.

Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
Add a test case to verify that without CAP_PERFMON, the test now
succeeds instead of failing due to a verification error.

Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants