Fix missing parent issue in clone events #1708
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tpapagian
added
the
release-note/bug
Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
tpapagian
force-pushed
the
pr/apapag/clone_fix
branch
from
c56bdf4
to
6cfc1b3
Compare
tpapagian
changed the title
tpapagian
force-pushed
the
pr/apapag/clone_fix
branch
from
6cfc1b3
to
41ca507
Compare
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
tpapagian
force-pushed
the
pr/apapag/clone_fix
branch
2 times, most recently
from
cc6f6c5
to
f38b799
Compare
Clone events are handled in event_wake_up_new_task program. In the case of __event_find_parent failure, we do not send an event to the user. On the other hand, execve_map_get has already added a new entry in execve_map. This causes an inconsistency between execve_map and the user space process cache that may lead for events to go through the eventcache and waing of a process that never arrives. To solve this, we try to find our parent in the beginning of the program. If we fail to do so, we simply stop the execution of this program (and do not add any entries in the execve_map). This commit also does some refactoring to cleanup the code. Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
In Tetragon we do not report process_exec and process_exit events for kernel threads. When searching for the parent of a kernel thread, we failed to do so and we simply ignore that. This patch optimizes that path as we abort early by checking the task_struct's flags. Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
tpapagian
force-pushed
the
pr/apapag/clone_fix
branch
from
f38b799
to
05d0638
Compare
kkourt
approved these changes
Nov 8, 2023
| * cannot find it's parent in the execve_map. | ||
| */ | ||
| parent = __event_find_parent(task); | ||
| if (!parent) |
There was a problem hiding this comment.
Would be nice to have a counter for this! I'm curious if it will ever happen, when the second patch is also applied.
That being said, since we are not sending an event if we don't find the parent, there is no point in adding an entry to the map. So this patch makes sense to me as is.
There was a problem hiding this comment.
Yes, I have in mind to add a counter for that (and other cases in exec/exit events) to check if this happens in a follow-up PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clone events are handled in the
event_wake_up_new_taskprogram. In the case of__event_find_parentfailure, we do not send an event to the user. On the other hand,execve_map_gethas already added a new entry inexecve_map. This causes an inconsistency betweenexecve_mapand the user space process cache that may lead events to go through the event cache and wait for a process that never arrives.To solve this, we try to find our parent at the beginning of the program. If we fail to do so, we simply stop the execution of this program (and do not add any entries in the
execve_map). This commit also does some refactoring to clean up the code.The last commit checks for kernel threads during clone events and avoid searching for parent early during the process.