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

Fix check for context propagation support #600

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

myhro
Copy link
Contributor

@myhro myhro commented Feb 2, 2024

Hi. I was investigating the issue #390, based on the report that it doesn't work on Debian 11 (Bullseye), which should have a supported kernel:

$ cat /etc/os-release | head -5
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"
NAME="Debian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
$ uname -a
Linux bullseye 5.10.0-27-amd64 #1 SMP Debian 5.10.205-2 (2023-12-31) x86_64 GNU/Linux
$ grep CONFIG_DEBUG_INFO_BTF /boot/config-5.10.0-27-amd64
CONFIG_DEBUG_INFO_BTF=y

I managed to start Beyla after recompiling it and was able to trace C programs, like the Apache HTTP Server. The problem is that Go binaries like the testserver weren't working:

time=2024-02-02T01:49:22.338Z level=ERROR msg="couldn't trace process.
Stopping process tracer" component=ebpf.ProcessTracer
path=/home/myhro/github.com/grafana/beyla/test/integration/components/testserver/testserver
pid=563 error="loading and assigning BPF objects: field
UprobeHttp2FramerWriteHeadersReturns: program
uprobe_http2FramerWriteHeaders_returns: load program: invalid argument: 73:
(57) r7 &= 15: ; (truncated, 987 line(s) omitted)"

After debugging for a while, trying to understand what uprobe_http2FramerWriteHeaders_returns had different than other BPF programs, I realized that the VM was running with Secure Boot enabled (which is the default on LXD/Incus). This, in turn, enables the kernel integrity mode:

$ cat /sys/kernel/security/lockdown
none [integrity] confidentiality

As documented, in cases like that the context propagation might not work. What made it a bit more interesting is the fact that while yes, the lockdown check for the bpf_probe_write_user() helper was indeed introduced on kernel 5.14, it was also backported to the 5.10 tree as well.

Based on these findings, I'm proposing a fix for the context propagation support check.

P.s.: I also checked 5.4 (the previous LTS) and 5.9 versions and didn't find the patch in their trees.

While the lockdown check for the 'bpf_probe_write_user()' helper was
indeed introduced on 5.14[1], it was also backported to the 5.10 tree[2]
as well.

[1]: torvalds/linux@51e1bb9
[2]: https://elixir.bootlin.com/linux/v5.10.59/A/ident/LOCKDOWN_BPF_WRITE_USER
@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (447c8b9) 79.68% compared to head (096ed22) 80.34%.

Files Patch % Lines
pkg/internal/ebpf/common/common.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #600      +/-   ##
==========================================
+ Coverage   79.68%   80.34%   +0.65%     
==========================================
  Files          70       70              
  Lines        5936     5936              
==========================================
+ Hits         4730     4769      +39     
+ Misses        981      940      -41     
- Partials      225      227       +2     
Flag Coverage Δ
integration-test 70.12% <0.00%> (+0.70%) ⬆️
unittests 44.72% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks so much Tiago, good catch!

@grcevski grcevski merged commit 96c774a into grafana:main Feb 2, 2024
5 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