-
Notifications
You must be signed in to change notification settings - Fork 431
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(e2e): treat corner cases in ds_writer.go #4554
Merged
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
rscampos
approved these changes
Jan 28, 2025
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
geyslan
added a commit
to geyslan/tracee
that referenced
this pull request
Feb 10, 2025
fix(epbf): fix handling of compat tasks in syscall checkers fix(ebpf): treat sched_process_exit corner cases The sched_process_exit event may be triggered by a standard exit, such as a syscall, or by alternative kernel paths, making it unsafe to assume that it is always associated with a syscall exit. do_exit and do_exit_group, while typically invoked by the exit and exit_group syscalls, can also be reached through internal kernel mechanisms such as signal handling. A concrete example of this occurs when a syscall returns, enters signal handling, and subsequently calls do_exit after get_signal. Both get_signal and do_exit involve tracepoints. A real execution flow illustrating this scenario in the kernel is as follows: entry_SYSCALL_64 ├── do_syscall_64 ├── syscall_exit_to_user_mode ├── __syscall_exit_to_user_mode_work ├── exit_to_user_mode_prepare ├── exit_to_user_mode_loop ├── arch_do_signal_or_restart ├── get_signal (has signal_deliver tracepoint) ├── do_group_exit └── do_exit (has sched_process_exit tracepoint) feat(events): convert syscall arg to name at processing stage (aquasecurity#4563) Since signatures now receive unparsed event arguments, conversion of syscall IDs to names became an issue since the signature has no way to know whether the event was generated on an x86_64 or ARM64 system. To solve this, we convert the syscall ID argument to its name at the event processing stage, so it happens regardless of argument parsing. This is applied to the `suspicious_syscall_source` and `stack_pivot` events. fix: get exit code and signal values When a process exits normally via exit(n), the exit code is stored in the upper byte (exit_code << 8). The lower byte is used for signal information if the process was terminated by a signal. Also, align the type of exit_code used at struct task_struct. fix(ebpf): revise thread stack identification logic Identifying thread stacks by a VMA is unreliable because VMA splitting and joining may cause the searched VMA to not align nicely with the thread stack VMA. Instead, we can identify a thread stack by an address, making the identification straight-forward. fix(pipeline): fix stack-addresses not working (aquasecurity#4579) fix: type parsing net_tcp_connect The code attempted to cast "type" as a string, which caused issues when the actual value was of a different type. So this commit changes the type to int32, aligning it with parsers.SOCK_STREAM.Value(). test: add test for event net_tcp_connect perf(proc): use formatters for procfs file paths Since the type of the converted primitive is already known, formatter helpers should be used to construct procfs file paths instead of relying on `fmt.Sprintf`. Using `fmt.Sprintf` is relatively costly due to its internal formatting logic, which is unnecessary for simple path construction. perf(proc): introduce ReadFile for /proc `os.ReadFile` is not efficient for reading files in `/proc` because it attempts to determine the file size before reading. This step is unnecessary for `/proc` files, as they are virtual files with sizes that are often reported as unknown or `0`. `proc.ReadFile` is a new function designed specifically for reading files in `/proc`. It reads directly into a buffer and is more efficient than `os.ReadFile` because it allows tuning the initial buffer size to better suit the characteristics of `/proc` files. Running tool: /home/gg/.goenv/versions/1.22.4/bin/go test -benchmem -run=^$ -tags ebpf -bench ^BenchmarkReadFile$ github.com/aquasecurity/tracee/pkg/utils/proc -benchtime=10000000x goos: linux goarch: amd64 pkg: github.com/aquasecurity/tracee/pkg/utils/proc cpu: AMD Ryzen 9 7950X 16-Core Processor BenchmarkReadFile/ProcFSReadFile/Empty_File-32 10000000 3525 ns/op 408 B/op 4 allocs/op BenchmarkReadFile/OsReadFile/Empty_File-32 10000000 4070 ns/op 872 B/op 5 allocs/op BenchmarkReadFile/ProcFSReadFile/Small_File-32 10000000 3961 ns/op 408 B/op 4 allocs/op BenchmarkReadFile/OsReadFile/Small_File-32 10000000 4538 ns/op 872 B/op 5 allocs/op BenchmarkReadFile/ProcFSReadFile/Exact_Buffer_Size-32 10000000 4229 ns/op 920 B/op 5 allocs/op BenchmarkReadFile/OsReadFile/Exact_Buffer_Size-32 10000000 4523 ns/op 872 B/op 5 allocs/op BenchmarkReadFile/ProcFSReadFile_/proc/self/stat-32 10000000 4043 ns/op 408 B/op 4 allocs/op BenchmarkReadFile/OsReadFile_/proc/self/stat-32 10000000 4585 ns/op 872 B/op 5 allocs/op PASS ok github.com/aquasecurity/tracee/pkg/utils/proc 334.751s perf(proc): improve stat file parsing Remove the use of library functions to parse the stat file and instead parse it manually (on the fly) to reduce the number of allocations and improve performance. chore(proc): align parsing of stat field with the formats size This also align parsing sizes with the formats to avoid wrong parsing of the stat file. The internal fields are represented aligned with the actual kernel fields to avoid any confusion (signed/unsigned). perf(proctree/proc): align fields to real size Propagate values based on its real size which in most cases is smaller than int (64-bit). This change reduces the memory footprint or at least the stress on the stack/heap. perf(proc): improve status file parsing Remove the use of library functions to parse the status file and instead parse it manually (on the fly) to reduce the number of allocations and improve performance. perf(proc): improve ns Reduce ProcNS memory footprint by using the right member type sizes - namespace id is an uint32, since it is the inode number in struct ns_common. This change also improves the performance of GetAllProcNS(), GetProcNS() and GetMountNSFirstProcesses(). chore: introduce builders with specific fields - NewProcStatFields() - NewThreadStatFields() - NewProcStatusFields() - NewThreadStatusFields() perf(proctree): remove stat call Calling stat on /proc/<pid> would only increase the window for process termination between the stat call and the read of the file. This also replaces fmt.Sprintf with string concatenation and strconv.FormatInt for better performance. perf(proctree)!: rearrange struct fields Mind the padding and the alignment of the fields to avoid wasting memory. BREAKING CHANGE: invoked_from_kernel, sched_process_exec event arg, is now a bool. perf(proctree): centralize child and thread Maps Previously, each Process maintained its own maps for children and threads, leading to significant overhead in the process tree. This commit moves those maps into the ProcessTree, which now centrally manages the children and threads for every process. Additionally, this change allows us to simplify the Process struct by removing the dedicated mutex that was solely used for protecting the individual maps. chore(proctree): use atomic types
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.
Detected while debugging https://github.com/aquasecurity/tracee/actions/runs/12953849577/job/36134460471#step:4:1268
1. Explain what the PR does
66dbdb4 fix(e2e): treat corner cases in ds_writer.go
2. Explain how to test it
3. Other comments