Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

tetragon: clone namespace improvements #2695

Merged
merged 2 commits into from
Jul 19, 2024
Merged

Conversation

tixxdz
Copy link
Member

@tixxdz tixxdz commented Jul 18, 2024

store thread leader namespaces at fork and reduce false positives

tixxdz added 2 commits July 18, 2024 19:06
Store the thread leader namespaces during fork so we can check later
if they changed, as right now they are only stored late during execv
which will point to a new exec_id entry anyway.

Right now during fork they are zeroed in the execve_map which make it
unreliable to detect if they changed between the fork and the final
execve, they will always be reported as if they changed which could be
a false positive report.

While we are it improve how we fetch and store capabilities.

Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Djalal Harouni <tixxdz@gmail.com>
@tixxdz tixxdz added the release-note/minor This PR introduces a minor user-visible change label Jul 18, 2024
@tixxdz tixxdz requested a review from a team as a code owner July 18, 2024 21:51
@tixxdz tixxdz requested a review from tpapagian July 18, 2024 21:51
@tixxdz tixxdz changed the title WIP: clone ns fixes tetragon: clone namespace improvements Jul 18, 2024
@tixxdz tixxdz requested review from jrfastab and kevsecurity July 18, 2024 22:25
@tixxdz
Copy link
Member Author

tixxdz commented Jul 18, 2024

CI failure seems unrelated

Comment on lines -69 to -71
curr->caps.permitted = caps.permitted;
curr->caps.effective = caps.effective;
curr->caps.inheritable = caps.inheritable;
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to that PR, but these assignments seems to be redundant as we did get_current_subj_caps? And this is the reason that you removed those now, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

This just minor optimization, since old code was reading in new var caps then copying back to curr->caps.$field, see next line in the patch, now we read directly into curr->caps , so just saved some cycles.

@tixxdz
Copy link
Member Author

tixxdz commented Jul 19, 2024

Thank you @tpapagian

@tixxdz tixxdz merged commit 87ec91c into main Jul 19, 2024
45 of 46 checks passed
@tixxdz tixxdz deleted the pr/tixxdz/2024-07-clone-ns-fixes branch July 19, 2024 10:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
release-note/minor This PR introduces a minor user-visible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants