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

Include bpftrace by src part2 pkg bpftrace #3916

Conversation

christoph-zededa
Copy link
Contributor

@christoph-zededa christoph-zededa commented May 13, 2024

has to be changed once #3915 is merged

@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch from 933dcf5 to 34defc2 Compare May 13, 2024 17:40
@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch from 34defc2 to 96d5986 Compare May 13, 2024 17:40
@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch from 96d5986 to 85df835 Compare May 21, 2024 14:21
@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch 5 times, most recently from 7b7d5fb to 8f41b93 Compare May 21, 2024 16:28
@christoph-zededa christoph-zededa marked this pull request as ready for review May 21, 2024 16:33
@eriknordmark eriknordmark requested a review from shjala May 21, 2024 21:38
@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch from 1e2fb26 to 852680e Compare May 22, 2024 12:46
@eriknordmark
Copy link
Contributor

It would be good to have a security review with @shjala once he is back - to make sure we are aware of how to handle arbitrary ebpf code or whether we will instead somehow provide approved and signed ebpf programs.

@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch from 852680e to ba62d9a Compare May 28, 2024 09:48
@shjala
Copy link
Member

shjala commented Jun 4, 2024

@eriknordmark we had a discussion with Christoph, the main concerns are two helper functions 1) bpf_override_return which can be disabled via CONFIG_BPF_KPROBE_OVERRIDE kernel config and 2) bpf_probe_write_user which can be disabled by setting kernel lockdown mode to confidentiality but that might cause some problems as it comes with a bunch more restrictions like blocking direct PCI access or blocking raw io port access which might cause problem for Qemu, so a small patch for kernel to disable those helper functions unconditionally might be more appropriate.

The proposed changes will unblock the PR for now...

@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch from ba62d9a to 74fec93 Compare June 5, 2024 09:43
@christoph-zededa christoph-zededa marked this pull request as draft June 5, 2024 09:43
@christoph-zededa
Copy link
Contributor Author

Moved to draft state until I have done the security changes for the kernel.

@christoph-zededa
Copy link
Contributor Author

@eriknordmark we had a discussion with Christoph, the main concerns are two helper functions 1) bpf_override_return which can be disabled via CONFIG_BPF_KPROBE_OVERRIDE kernel config and 2) bpf_probe_write_user which can be disabled by setting kernel lockdown mode to confidentiality but that might cause some problems as it comes with a bunch more restrictions like blocking direct PCI access or blocking raw io port access which might cause problem for Qemu, so a small patch for kernel to disable those helper functions unconditionally might be more appropriate.

The proposed changes will unblock the PR for now...

PRs for the kernel created here: https://github.com/lf-edge/eve-kernel/pulls?q=is%3Apr+is%3Aopen+Restrict+BPF+usage

@christoph-zededa christoph-zededa marked this pull request as ready for review June 25, 2024 14:15
@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch from 74fec93 to 3642c6e Compare June 25, 2024 14:15
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have build.yml with network disabled.

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
@christoph-zededa christoph-zededa force-pushed the include_bpftrace_by_src_part2_pkg_bpftrace branch from 3642c6e to 1249085 Compare June 25, 2024 15:02
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eriknordmark eriknordmark merged commit 87d0507 into lf-edge:master Jun 25, 2024
23 of 32 checks passed
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

Successfully merging this pull request may close these issues.

3 participants