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 symbol multi addrs #3802

Merged
merged 9 commits into from
Jan 18, 2024
Merged

Fix symbol multi addrs #3802

merged 9 commits into from
Jan 18, 2024

Conversation

rafaeldtinoco
Copy link
Contributor

@rafaeldtinoco rafaeldtinoco commented Jan 11, 2024

commit 68a961a (HEAD -> fix-symbol-multi-addrs)
Author: Rafael David Tinoco [email protected]
Date: Thu Jan 18 03:03:08 2024

chore(tests): fix intermittent issues with bpf_attach e2e test

commit 07d9593
Author: Rafael David Tinoco [email protected]
Date: Thu Jan 18 02:59:43 2024

fix(ebpf): make single address symbols attaches by name

Previous commits made kprobe/kretprobe attachments to be attached using
the symbol addresses only. This is okay but events such as "bpf_attach"
symbol name arguments are zeroed and tracee uses itself to test if bpf
attachments were made.

Thinking a bit more, there is no problem in having the bpf attachments using
symbol names, as long as there aren't 2 addresses for the same symbol.

commit 450d3d3
Author: Rafael David Tinoco [email protected]
Date: Wed Jan 17 14:45:07 2024

fix(ebpf): allow ebpf prog attach to multi addr symbols

Changes probes interface "tracing" type so kprobe and kretprobes can be
attached using the kernel symbol addresses instead of names. This solves the
problem when the symbol has multiple addresses and the kernel refuses the
attachment (newer kernels) because of that.

https://github.com/aquasecurity/tracee/issues/3653#issuecomment-1832642225

Fixes: #3653

commit d9bdc0b
Author: Rafael David Tinoco [email protected]
Date: Wed Jan 17 10:36:44 2024

chore(ebpf): simply probe attachment

Follow recent decision about having "declarative" and "immutable" probes
and add internal probe dependencies to each syscall event declaration.

commit 8f0b203
Author: Rafael David Tinoco [email protected]
Date: Wed Jan 17 09:21:11 2024

chore: adjust TODO comment for kconfig values initialization

commit 76890aa
Author: Rafael David Tinoco [email protected]
Date: Sat Jan 13 14:35:04 2024

chore(refactor): refactor pkg/ebpf/ksymbols and associated logic

commit 2f33192
Author: Rafael David Tinoco [email protected]
Date: Thu Jan 11 19:26:35 2024

fix(utils): fix kallsyms package for multi address symbols

Fixes: #3798

Related: https://github.com/aquasecurity/libbpfgo/pull/399

commit 4b0bc93
Author: Rafael David Tinoco [email protected]
Date: Wed Jan 17 15:19:54 2024

chore(golang): update to latest libbpfgo for kallsyms fix

commit 645fc4d
Author: Rafael David Tinoco [email protected]
Date: Wed Jan 17 15:27:10 2024

fix(tests): disable unattended-upgrades from AMIs

@rafaeldtinoco rafaeldtinoco marked this pull request as draft January 11, 2024 22:30
@rafaeldtinoco rafaeldtinoco marked this pull request as ready for review January 15, 2024 12:13
@rafaeldtinoco
Copy link
Contributor Author

I'm still missing the fix for #3653. Will be pushing soon.

@rafaeldtinoco
Copy link
Contributor Author

rafaeldtinoco commented Jan 17, 2024

Note for reviewer: be aware that, due to some other duties before my last day, I won't have much, if any, time to change this PR too much (due to some other needs yet to be addressed). It seems that I was able to address and fix the issue and do some minor and good refactors.

So, unless, you're willing to take ownership of the PR for yourself, I suggest that this is merged even if not perfect for your eyes (avoid nits and things alike), just so the work isn't lost (and any improvement can be done later with code already merged).

My 2 cents only.

Follow recent decision about having "declarative" and "immutable" probes
and add internal probe dependencies to each syscall event declaration.
Changes probes interface "tracing" type so kprobe and kretprobes can be
attached using the kernel symbol addresses instead of names. This solves the
problem when the symbol has multiple addresses and the kernel refuses the
attachment (newer kernels) because of that.

#3653 (comment)

Fixes: #3653
Previous commits made kprobe/kretprobe attachments to be attached using
the symbol addresses only. This is okay but events such as "bpf_attach"
symbol name arguments are zeroed and tracee uses itself to test if bpf
attachments were made.

Thinking a bit more, there is no problem in having the bpf attachments using
symbol names, as long as there aren't 2 addresses for the same symbol.
Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

LGTM

@rafaeldtinoco
Copy link
Contributor Author

Passed all tests.

@rafaeldtinoco rafaeldtinoco merged commit e750c7b into aquasecurity:main Jan 18, 2024
30 checks passed
@rafaeldtinoco rafaeldtinoco deleted the fix-symbol-multi-addrs branch January 18, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants