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

Improve handling of unmerged ETL files. #291

Merged
merged 1 commit into from
Jul 13, 2024
Merged

Improve handling of unmerged ETL files. #291

merged 1 commit into from
Jul 13, 2024

Conversation

mstange
Copy link
Owner

@mstange mstange commented Jul 13, 2024

We still merge during recording. But this is one step on the way to a world where we no longer need to do it. We could probably stop merging right now for the case where we only have a kernel session and no user session.

This commit tweaks how we buffer the ImageID information before we process the MSNT_SystemTrace/Image information.
And it adds handling to get the CodeId + DebugId from the binary, if we're importing an unmerged ETL file.

I've removed a fair amount of code that we had kept around for adding kernel driver handling. It doesn't seem to be needed - I get MSNT_SystemTrace/Image/DCStart events for all the same kernel modules that I also see in the merged ETL.

@vvuk Do you think we still need any of the code in winutils.rs?

We still merge during recording. But this is one step on the way
to a world where we no longer need to do it. We could probably
stop merging right now for the case where we only have a kernel
session and no user session.

This commit tweaks how we buffer the ImageID information before we
process the MSNT_SystemTrace/Image information.
And it adds handling to get the CodeId + DebugId from the binary,
if we're importing an unmerged ETL file.

I've removed a fair amount of code that we had kept around for
adding kernel driver handling. It doesn't seem to be needed - I get
MSNT_SystemTrace/Image/DCStart events for all the same kernel modules
that I also see in the merged ETL.
@mstange mstange enabled auto-merge July 13, 2024 23:37
@mstange
Copy link
Owner Author

mstange commented Jul 13, 2024

@vvuk Do you think we still need any of the code in winutils.rs?

Oh, we still call get_dos_device_mappings(), so that part needs to stay. But I think iter_kernel_drivers() can go. And enable_debug_privilege() should go anyway.

@mstange mstange merged commit 529a846 into main Jul 13, 2024
15 checks passed
@vvuk
Copy link
Collaborator

vvuk commented Jul 16, 2024

This looks good. And agreed that iter_kernel_drivers() + enable_debug_privilege() can go away. For debug privilege, I can maybe see a future world where samply may want to inspect a target process directly, but we can bring enable privs when needed.

@mstange
Copy link
Owner Author

mstange commented Jul 16, 2024

Thanks for confirming!

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.

2 participants