-
Notifications
You must be signed in to change notification settings - Fork 12
[ciqlts8_6] bpf: Fix ringbuf memory type confusion when passing to helpers #371
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
Conversation
jira VULN-68 cve CVE-2021-4204 commit-author Daniel Borkmann <daniel@iogearbox.net> commit a672b2e The bpf_ringbuf_submit() and bpf_ringbuf_discard() have ARG_PTR_TO_ALLOC_MEM in their bpf_func_proto definition as their first argument, and thus both expect the result from a prior bpf_ringbuf_reserve() call which has a return type of RET_PTR_TO_ALLOC_MEM_OR_NULL. While the non-NULL memory from bpf_ringbuf_reserve() can be passed to other helpers, the two sinks (bpf_ringbuf_submit(), bpf_ringbuf_discard()) right now only enforce a register type of PTR_TO_MEM. This can lead to potential type confusion since it would allow other PTR_TO_MEM memory to be passed into the two sinks which did not come from bpf_ringbuf_reserve(). Add a new MEM_ALLOC composable type attribute for PTR_TO_MEM, and enforce that: - bpf_ringbuf_reserve() returns NULL or PTR_TO_MEM | MEM_ALLOC - bpf_ringbuf_submit() and bpf_ringbuf_discard() only take PTR_TO_MEM | MEM_ALLOC but not plain PTR_TO_MEM arguments via ARG_PTR_TO_ALLOC_MEM - however, other helpers might treat PTR_TO_MEM | MEM_ALLOC as plain PTR_TO_MEM to populate the memory area when they use ARG_PTR_TO_{UNINIT_,}MEM in their func proto description Fixes: 457f443 ("bpf: Implement BPF ring buffer and verifier support for it") Reported-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: John Fastabend <john.fastabend@gmail.com> Acked-by: Alexei Starovoitov <ast@kernel.org> (cherry picked from commit a672b2e) Signed-off-by: Pratham Patel <ppatel@ciq.com>
The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥌 Nice work tracking down the fix.
Sorry I am trying to keep this so its easy to identify the user branches and their age so they can be deleted when needed. |
That's totally fine by me. I was stating the reason for the commit hash mismatch between the commit pushed and the one in the build log, nothing more. :) |
Sorry, Maple's comment on the ticket reminded me that I forgot to share the result of running the test from LTP. The test wouldn't run for two reasons. The first reason was that the test would be skipped if the kernel version was lower than diff --git a/testcases/kernel/syscalls/bpf/bpf_prog06.c b/testcases/kernel/syscalls/bpf/bpf_prog06.c
index f701e9599..6d2ca3a81 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog06.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog06.c
@@ -132,7 +132,7 @@ static struct tst_test test = {
.timeout = 20,
.setup = setup,
.test_all = run,
- .min_kver = "5.8",
+ .min_kver = "4.10",
.taint_check = TST_TAINT_W | TST_TAINT_D,
.caps = (struct tst_cap []) {
TST_CAP(TST_CAP_DROP, CAP_SYS_ADMIN), But then, the binary kept dropping the capabilities. It would do that even when executed as root (not diff --git a/lib/tst_capability.c b/lib/tst_capability.c
index cb0502e2e..bb3947f99 100644
--- a/lib/tst_capability.c
+++ b/lib/tst_capability.c
@@ -63,7 +63,6 @@ void tst_cap_action(struct tst_cap *cap)
switch (act) {
case TST_CAP_DROP:
- do_cap_drop(pE, mask, cap);
break;
case TST_CAP_REQ:
do_cap_req(pP, pE, mask, cap); And finally, we have binary execution.
Since the BPF program exited with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit message
Kernel build logs
build.log
Kselftests
It appears that the
vmx_preemption_timer_test
kselftest doesn't fail anymore.kselftest-before.log
kselftest-after.log