-
Notifications
You must be signed in to change notification settings - Fork 360
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
bpf: fmodret override on security_ hooks is available from 5.7 #1349
Conversation
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
looks good, could that be done with some feature check as well? I'd think Redhat kernel might have this backported even if the kernel version is 4.18.. but we can start with this for sure
Agreed. |
pkg/sensors/tracing/generickprobe.go
Outdated
// https://lore.kernel.org/all/[email protected]/ | ||
if !f.Syscall && | ||
(strings.HasPrefix(f.Call, "security_") == false || !kernels.MinKernelVersion("5.7")) { | ||
return fmt.Errorf("Error override action can be used only with syscalls or on security_ hooks with a minimum kernel version 5.7") |
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.
Until we have a feature check, would it make sense to have a flag so that we can ignore the kernel check here? As @olsajiri said, some vendor kernels backport many patches and the kernel version does not necessarily reflect the feature set of the kernel, and it would be nice to have a way to override the kernel check.
We can leave this as a follow up though.
Oh I see redhat kernels, so in that case I will convert this patch to be: document where override applies syscalls, security_ hooks with the corresponding kernels version or patch, and let it fail for other cases. Thoughts? |
You mean not have the kernel check? Yes, I think that's fine from my side. |
19b6437
to
d102401
Compare
f6b3487
to
83b4075
Compare
Let's improve override action documentation and try to be precise on security_ hooks overriding being possible starting from kernel 5.7 If some distro has backported the patch to allow fmod_ret override on security_ hooks then all good, otherwise if it fails we are covered by the documentation. Also change the caution point to be more addressed to kernel developers if they want to leverage tetragon kprobe capabilities and use error injections, as they should get it right in the first place. Rerefence: https://lore.kernel.org/all/[email protected]/ Signed-off-by: Djalal Harouni <[email protected]>
83b4075
to
4b33adc
Compare
Yes, improved the doc. If no objection will merge it later |
Thanks! |
The override on security_ hook is available starting from 5.7, so check before loading the policy.
https://lore.kernel.org/all/[email protected]/