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

syscalls: rework syscall arguments processing #1807

Merged
merged 10 commits into from
Sep 18, 2024

Conversation

1ndahous3
Copy link
Contributor

@1ndahous3 1ndahous3 commented Sep 13, 2024

Reworked static information about syscall arguments, each argument type is now associated with an exact size (to remove extra bytes in values ​​obtained from registers/stack) and other characteristics.

(size bug was detected for type FS_INFORMATION_CLASS: FsInformationClass=0x4500000003)

Removed previous workarounds for certain types (see #1302).

Bonus: pointer types are now associated with the types they point to - this is in preparation for the upcoming feature to reflectively print arguments by pointers (not just for PPVOID in transform_value), see #1756.

A few syscalls have also been implemented (usually using Windows *_CLASS class enums, this is an attempt to cover all existing *_CLASS types.

Implemented a check test suite for "syscalls": it checks if information for all argument types is present (to prevent a runtime fail).

@drakvuf-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@tklengyel
Copy link
Owner

@drakvuf-jenkins Test this please

@tklengyel
Copy link
Owner

@drakvuf-jenkins Retest this please

@tklengyel
Copy link
Owner

tklengyel commented Sep 13, 2024

On Linux we get

drakvuf: ../src/plugins/syscalls/syscalls.cpp:189: uint64_t syscalls_base::mask_value(const arg_t &, uint64_t): Assertion `false && "Unknown size for type"' failed.`

@1ndahous3
Copy link
Contributor Author

1ndahous3 commented Sep 14, 2024

Yes, the assert worked as I expected - it found a bug: printing a Linux syscall with an argument defined as VOID.

Also I missed the point that Linux syscalls were also described by arguments with uppercase names, but I specified them for Windows OS.

I am going to add primitive types for Linux syscalls and not mix them with Windows types - this will also help to better understand what their real type and size are. And of course, replace void types with real ones.

@1ndahous3 1ndahous3 marked this pull request as draft September 14, 2024 16:25
@tklengyel
Copy link
Owner

@drakvuf-jenkins Test this please

@1ndahous3 1ndahous3 marked this pull request as ready for review September 16, 2024 08:05
@1ndahous3
Copy link
Contributor Author

@tklengyel all done, but it seems that "check" tests are not run during CI (*check.cpp files).

@tklengyel
Copy link
Owner

@tklengyel all done, but it seems that "check" tests are not run during CI (*check.cpp files).

Feel free to add a ci step for that

@1ndahous3
Copy link
Contributor Author

@tklengyel all done:

make  check-TESTS
make[3]: Entering directory '/home/runner/work/drakvuf/drakvuf/src/plugins'
make[4]: Entering directory '/home/runner/work/drakvuf/drakvuf/src/plugins'
PASS: plugin_utils_check
PASS: syscalls/check
PASS: procmon/check
PASS: output_format/check
============================================================================
Testsuite summary for DRAKVUF 1.1-git20240918093834+0cc7657-1
============================================================================
# TOTAL: 4
# PASS:  4
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

@tklengyel
Copy link
Owner

@drakvuf-jenkins Test this please

@tklengyel
Copy link
Owner

@drakvuf-jenkins Test this please

@tklengyel tklengyel merged commit a7a799f into tklengyel:main Sep 18, 2024
41 checks passed
@tklengyel
Copy link
Owner

Thanks!

@1ndahous3 1ndahous3 deleted the syscalls_arg_type_info branch September 18, 2024 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants