Skip to content
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 hooked_syscall in older kernels #3601

Merged
merged 2 commits into from
Oct 26, 2023

Conversation

OriGlassman
Copy link
Collaborator

@OriGlassman OriGlassman commented Oct 22, 2023

1. Explain what the PR does

support hooked_syscall event in older kernels

2. Explain how to test it

tracee -e=hooked_syscall

3. Other comments

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Oct 23, 2023

I needed to remove set "sysctl" back to amd64 or I would get a false positive in v5.4:

vagrant@focal:baseline$ uname -a
Linux focal 5.4.0-165-generic #182-Ubuntu SMP Mon Oct 2 19:43:28 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
vagrant@focal:baseline$ git diff
diff --git a/pkg/events/core_amd64.go b/pkg/events/core_amd64.go
index 4a262d7b3..dd81a31e8 100644
--- a/pkg/events/core_amd64.go
+++ b/pkg/events/core_amd64.go
@@ -1063,7 +1063,7 @@ var SyscallSymbolNames = map[ID]string{
        153: "vhangup",
        154: "modify_ldt",
        155: "pivot_root",
-       156: SyscallNotImplemented + "sysctl",
+       156: "sysctl",
        157: "prctl",
        158: "arch_prctl",
        159: "adjtimex",

But then in 6.1 I get:

rafaeldtinoco@rugged:baseline$ uname -a
Linux rugged 6.1.55-1-lts #1 SMP PREEMPT_DYNAMIC Sat, 23 Sep 2023 16:57:15 +0000 x86_64 GNU/Linux
{"level":"error","ts":1698032006.0274613,"msg":"Error populating expected syscall table array: symbol not found: system___x64_sys_sysctl"}

Maybe we should add a logic to pick available syscalls from kallsyms or, at least, instead of returning on errors at hookedSyscallTableRoutine() we could differentiate non existing symbol errors and just debug log them. WDYT ?

vagrant@focal:baseline$ sudo cat /proc/kallsyms | grep sys_sysctl
ffffffff852a6130 T __ia32_sys_sysctl
ffffffff852a6210 T __ia32_compat_sys_sysctl
ffffffff852a6300 T __x64_sys_sysctl
ffffffff852a63e0 T __x32_compat_sys_sysctl

The detection worked alright in v5.4:

vagrant@focal:baseline$ sudo ./dist/tracee --output json --events hooked_syscall
{"timestamp":1698030203329758411,"threadStartTime":1698030202626935491,"processorId":3,"processId":14353,"cgroupId":4294967297,"threadId":14368,"parentProcessId":14352,"hostProcessId":14353,"hostThreadId":14368,"hostParentProcessId":14352,"userId":0,"mountNamespace":4026531840,"pidNamespace":4026531836,"processName":"tracee","executable":{"path":""},"hostName":"focal","containerId":"","container":{},"kubernetes":{},"eventId":"2017","eventName":"hooked_syscall","matchedPolicies":[""],"argsNum":4,"returnValue":0,"syscall":"","stackAddresses":[0],"contextFlags":{"containerStarted":false,"isCompat":false},"threadEntityId":2445849516,"processEntityId":2475953850,"parentEntityId":3254647851,"args":[{"name":"syscall","type":"const char*","value":"uname"},{"name":"address","type":"const char*","value":"ffffffffc0816000"},{"name":"function","type":"const char*","value":"hooked_uname"},{"name":"owner","type":"const char*","value":"hijack"}]}

NOTE: I'm going to add a test for this event as well, just so we're good its cleared for all kernels and E2E.

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Oct 23, 2023

Need to fix: #3602 in order to add the hooked_syscall test. I'll check it tomorrow (#3602 (comment))

@OriGlassman OriGlassman force-pushed the fix_old_ker_syscall branch 3 times, most recently from 8324273 to edf2cfe Compare October 25, 2023 10:24
Copy link
Contributor

@rafaeldtinoco rafaeldtinoco left a comment

Choose a reason for hiding this comment

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

LGTM. Lets try this version now. The syscalls per version look good as well.

@rafaeldtinoco rafaeldtinoco merged commit 6549f1e into aquasecurity:main Oct 26, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants