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

Changes to frida-gum to support nostd #83

Closed
wants to merge 1 commit into from

Conversation

WorksButNotTested
Copy link
Contributor

Changes to support building frida-rust as no_std. A couple of APIs are excluded from the module_map when built with the nostd feature, because of their dependency on sdt::path::Path which I couldn't find a no_std replacement for. But it should affect the majority of use cases.

@meme
Copy link
Collaborator

meme commented Feb 23, 2023

This is a lot of changes, and adds a lot of maintenance burden. Can you help me understand why no_std is necessary?

@WorksButNotTested
Copy link
Contributor Author

Hi. Thanks for the feedback. Most of the changes relate to changes to the “use” blocks at the top of files. Some of them were from tidying up (there were instances where some use statements were grouped into a single block, but others not). I don’t think there were many actual code changes.

The rust std library is quite large and in turn has a dependency on large parts of libc. Many parts of it are duplicated in the core libraries. Since Frida-rust doesn’t need the features of the standard library it unnecessarily imposes this dependency on downstream consumers.

Although I accept this won’t be a problem for many conventional rust applications, it seems sensible in any case to minimise the complexity of code injected into a target process, to both minimise code size and minimise interaction between Frida and the target.

In my case, my application requires stringent control of the address space which isn’t possible when using rusts standard library.

Perhaps the changes could be smaller and simpler if the dependency on std could be dropped all together rather than it being an optional feature?

@meme
Copy link
Collaborator

meme commented Feb 23, 2023

Thanks for the explanation. It is clearer to me now.

Perhaps the changes could be smaller and simpler if the dependency on std could be dropped all together rather than it being an optional feature?

I think this is completely reasonable, actually. Instead of having an std feature though, we can just put module map behind a feature (perhaps module_map which internally uses std like you mentioned)

However, my reservations don't quite stop here. I would also really like to avoid adding dependencies as part of this migration. Namely, I don't understand the purpose of no-std-compat (perhaps we don't need it if we move to dropping std altogether). And under no circumstances do I want forks of crates, e.g. thiserror-no-std as a dependency. If we have to drop thiserror, so be it.

I hope this is reasonable. I am open to feedback, though I can see the value of this feature now (and this feature, as a default).

@WorksButNotTested
Copy link
Contributor Author

WorksButNotTested commented Feb 23, 2023

The only issue with the ModuleMap is the dependency on Path (to compare the filename of a path). Which isn't part of the core library. So another option could be just using string processing to do that job, but not sure whether that would affect non-unix platforms and I had no means to test and didn't want to introduce a regression.

no-std-compat is intended to reduce the number of changes to uses statements required to handle the switch to using types within the core libraries rather than in the std library. If we were to migrate wholesale to nostd then this dependency would go away. I included thiserror-no-std to minimize the changes required to support both std and nostd configuations with minimal changes. It seems like it isn't used to extensively though, so it should be possible to come up with an alternative approach.

Do note that I have only considered moving frida-gum and not frida-core to nostd. I've not really though about the merits of the latter since it isn't affecting what I'm trying to build. Also there is a problem I have come across (rust-lang/rust#107845) where cargo will attempt to link against the std library even for a nostd crate if one of its dependencies happens to depend on std. We should also move to version 2 of the resolver (as described in the linked issue too).

All sounds perfectly reasonable. I'm happy to go away and re-work things, just let me know what is the best direction to follow.

@meme
Copy link
Collaborator

meme commented Feb 23, 2023

The only issue with the ModuleMap is the dependency on Path

For now, I think we should just feature gate it. Then we can come back later.

If we were to migrate wholesale to nostd then this dependency would go away

This makes sense, we can drop it then because everything will be core after your changes (minus a few oddities like ModuleMap). Same goes for thiserror - it should be possible to drop it, we can just manually perform the derives ourselves. If you have any issues with this one let me know and I can try to help.

Do note that I have only considered moving frida-gum and not frida-core to nostd

Yup, makes sense. I don't think there is a good reason for moving frida-core to nostd, it is meant to run in hosted environments.

@WorksButNotTested
Copy link
Contributor Author

Are you happy if I just feature gate the parts of module map that deal with paths? It would be very useful to keep the parts which don’t in the nostd build.

@meme
Copy link
Collaborator

meme commented Feb 23, 2023

Sure, that works for me.

@WorksButNotTested
Copy link
Contributor Author

Great. Leave it with me for a bit.

@WorksButNotTested
Copy link
Contributor Author

Superseded by #85

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