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

[core, constants] : add OS and framework supported architectures #1247

Merged

Conversation

Abyss-W4tcher
Copy link
Contributor

Hi,

To increase general scalability, in the multi-architectures handling task, I would like to propose a way to unify and remove "string hardcoded" values all around the framework. Indeed, if a new architecture is added, most of the plugins will need to be updated to support this new layer. If there are a few cases where a plugin would work on Intel but not an AArch64 (although I do not see right now any reason), the plugin could still use a custom set of supported architectures.

With this PR, the plugins could later be updated from :

architectures=["Intel32", "Intel64"],

to :

architectures=WINDOWS_ARCHS,

When a new (CPU) architecture is implemented, this should allow to reduce the amount of small changes in each plugin, and unify everything.


This is also a starting point to resolve the hardcoded intel.Intel checks, which are in fact CPU layers checks.

Going from :

# Never stack on top of an intel layer
# FIXME: Find a way to improve this check
if isinstance(layer, intel.Intel):
return None
to :

# This "check" can also be put in an global utility
if any([isinstance(layer, l) for l in LINUX_ARCHS_LAYERS]): 
   return None 

or equivalent. This can be applied easily inside plugins checks.

ps : For this automagic bit, it can be a bit trickier when virtual CPU layers stacking occurs, as discussed here : #1088 (comment).


I created a new file, as inserting the code in constants.py resulted in circular imports.

Thanks by advance for the reviews :)

@gcmoreira
Copy link
Contributor

@Abyss-W4tcher I like your idea.

Just a minor note from your comment above: isinstance accepts a tuple. So you should be able to do:

# This "check" can also be put in an global utility
if isinstance(layer, tuple(LINUX_ARCHS_LAYERS)):
   return None 

Alternatively, if you want to go a bit further, you could also change all the definitions in volatility3/framework/constants/architectures.py to tuples. The issue with this is that ModuleRequirement and TranslationLayerRequirement require a list. However, this seems over killing to me, as a quick look at the code suggests they aren't using any specific functionality of a list. Maybe it's a good time to update that as well, but it's up to you and ikelos.

@ikelos
Copy link
Member

ikelos commented Sep 2, 2024

Hiya, thanks for this (and making it small and easily reviewable rather than part of a mammoth pull request!). 5:)

I'm happy with the idea and I think it should work out fine. We will just need to be very careful over the implementation. This area wasn't fully fleshed out when volatility was first designed so it was mostly as a placeholder we knew we'd need to update. @gcmoreira is right, isinstance does accept a tuple (although you can turn a list into a tuple at time of use, so there's not much between them).

I'm just trying to think through issues we might run into, and the main one I can think of would be automagic somehow managing to fulfil the wrong architecture, but that should be pretty difficult. I don't think the IDs between windows system of different architectures would match, and Linux includes the arch in the uname which we use as the unique identifier. So yep, happy to merge this...

@ikelos ikelos merged commit a40ac64 into volatilityfoundation:develop Sep 2, 2024
14 checks passed
gcmoreira added a commit to gcmoreira/volatility3 that referenced this pull request Sep 18, 2024
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