-
Notifications
You must be signed in to change notification settings - Fork 412
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
event: set_task_comm #1811
event: set_task_comm #1811
Conversation
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.
You should probably use task_rename tracepoint which is more stable than this kprobe:
https://elixir.bootlin.com/linux/v5.18.2/source/fs/exec.c#L1231
d2f1214
to
d759255
Compare
thanks @yanivagman ! |
pkg/ebpf/c/tracee.bpf.c
Outdated
// this tracepoint is invoked for task renaming - this also happens when a process first starts. | ||
// so check if this is new execution or not. | ||
// this arg was inspired by __set_task_comm() - https://elixir.bootlin.com/linux/v5.18.2/source/fs/exec.c#L1228. | ||
bool exec = (get_task_host_pid(tsk) == get_task_host_tgid(tsk)); |
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.
Is that always true?
What if the main thread (which has tgid) renamed itself?
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.
your'e right, good point.
i think the way to tackle this is to create a map indicating an exec. this map would be updated in sched_process_fork
, and deleted from in sched_process_exec
and task_rename
.
another is option is to go back to using kprobe __set_task_comm
(https://elixir.bootlin.com/linux/latest/source/fs/exec.c#L1228) which gets this information as an argument.
WDYT?
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.
can't we just check if the current syscall is execve/execveat?
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.
yes we can
d759255
to
9955ee3
Compare
create new event indicating process name change
9955ee3
to
4863a90
Compare
if (data.options & OPT_SHOW_SYSCALL) { | ||
syscall_data_t *sys = bpf_map_lookup_elem(&syscall_data_map, &data.context.host_tid); | ||
if (sys) { | ||
save_to_submit_buf(&data, (void *) &sys->id, sizeof(int), 2); |
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.
As you now send it as an argument, let's add this to the argument parser to get the syscall name
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.
i've added it to argprinters.go
. should i do it elsewhere?
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.
sorry, I missed that
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.
LGTM
create new event indicating process name change
Initial Checklist
Description (git log)
using
__set_task_comm
kprobe to indicate event of process name changeFixes: #1810
Type of change
How Has This Been Tested?
Tests being included in this PR:
Reproduce the test by running:
Final Checklist:
Pick "Bug Fix" or "Feature", delete the other and mark appropriate checks.
Git Log Checklist:
My commits logs have: