Skip to content

Fix get_name API, fix malfind#1596

Merged
ikelos merged 4 commits intodevelopfrom
vma_get_name_fixes
Feb 7, 2025
Merged

Fix get_name API, fix malfind#1596
ikelos merged 4 commits intodevelopfrom
vma_get_name_fixes

Conversation

@atcuno
Copy link
Contributor

@atcuno atcuno commented Jan 31, 2025

The get_name API of vm_area_struct instances could (and did in testing) throw invalid address exception, which doesn't match how other get_* APIs work and really defeats the purpose of them. So I fixed the API to properly return None on these errors and updated the typing to reflect that (it was missing before). I also changed the callers of this API to properly display a renderer value when the name isn't found.

malfind for vol3 was also strange in that it didn't show the path of code injection regions, which is a key artifact. Given that malfind uses get_name, I added the path fixes for it in along with its API fixing.

Sample: ggmemday1

Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

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

Well, not sure how you managed it, but looks like you managed to scrape by without really needing a version bump on anything! I would still request at least a PATCH version bump on LinuxUtilities before I merge it, so we can more quickly identify what might have changed if people start experiencing issues...

if vma.is_suspicious(proc_layer) and vma_name != "[vdso]":
data = proc_layer.read(vma.vm_start, 64, pad=True)
yield vma, data
yield vma, vma_name, data
Copy link
Member

Choose a reason for hiding this comment

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

It's an internal/non-exposed function, so this change is fine, but could we get a return type in the function signature please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

fname = "Anonymous Mapping"
return fname

def get_name(self, context, task) -> Optional[str]:
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, there wasn't a return type defined, so theoretically you could make a case for it being ok, but I'd at least bump the PATCH number, so we know that changes were made and be able to differentiate if strange things start being reported please. Hopefully this is in line to be moved to its own separate module in the same style as the ftrace stuff at some point...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok where do I bump the PATCH number for this for now?

@ikelos ikelos added the awaiting-author-response This issue/pull request needs attention from the original author label Feb 1, 2025
@ikelos ikelos merged commit 44ae4f1 into develop Feb 7, 2025
24 checks passed
@ikelos ikelos deleted the vma_get_name_fixes branch February 7, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-author-response This issue/pull request needs attention from the original author parity-release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants