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 init_perf_buf error handling #49

Conversation

josedonizetti
Copy link
Contributor

While investigating different capabilities to avoid sudo, I get

./tracee-ebpf 
libbpf: failed to mmap perf buffer on cpu #7: Operation not permitted
TIME             UID    COMM             PID     TID     RET              EVENT                ARGS
[signal SIGSEGV: segmentation violation code=0x1 addr=0x3f pc=0x713b73]

runtime stack:
runtime.throw(0x7e9fe6, 0x2a)
	/usr/local/go/src/runtime/panic.go:1117 +0x72
runtime.sigpanic()
	/usr/local/go/src/runtime/signal_unix.go:718 +0x2e5
...

The PR fixes error handling to properly stop the service instead of starting it without the perf buffers.

./tracee-ebpf 
libbpf: failed to mmap perf buffer on cpu #7: Operation not permitted
Failed to initialize perf buffer!
2021/08/05 14:27:37 error creating Tracee: error initializing file_writes perf map: failed to initialize perf buffer

@rafaeldtinoco
Copy link
Contributor

Hello @josedonizetti , thanks for the commit. While the change is appropriate...

I'm curious how you got into that situation. If it was because of a code change AND a capability drop test (using capsh for example), done simultaneously, would you mind sharing the reproducer ?

Currently if I drop cap_sys_admin I get insufficient privileges to run. If I drop cap_bpf, cap_sys_admin allows me to run bpf (likely for backwards compatibility).

My thought: if we are breaking something because of a development effort maybe we should do all changes together.

Again, obviously the fix is appropriate, just wanted to get documented how to get there.

@rafaeldtinoco
Copy link
Contributor

BTW, I see that your suggestion is against libbpfgo, so I guess it is right to be done separately anyway! Thank you!

@josedonizetti
Copy link
Contributor Author

@rafaeldtinoco To simulate it is a mix of changes, I started by changing tracee-ebpf, my change was in order to address the issue.

If the kernel was at least 5.8, I was testing for CAP_BPF, CAP_PERFMON, and CAP_NET_ADMIN instead of testing CAP_SYS_ADMIN.

With this change in place, I build a new tracee-ebpf and changed it capabilities: sudo setcap cap_net_admin,cap_ebpf,cap_perfmon=pe ./dist/tracee-ebpf

Then trying to execute the command without sudo triggered the error reported.

@rafaeldtinoco
Copy link
Contributor

Thank you for the feedback!

Copy link
Contributor

@rafaeldtinoco rafaeldtinoco left a comment

Choose a reason for hiding this comment

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

Correct way of dealing with ERR_PTR() from __perf_buffer_new().

@rafaeldtinoco rafaeldtinoco merged commit 391db95 into aquasecurity:main Aug 6, 2021
@josedonizetti josedonizetti deleted the fix-init_perf_buf-error-handling branch August 6, 2021 14:22
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.

2 participants