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

Calling 2 times Frida::obtain() cause ACCESS_VIOLATION #149

Open
muniategui opened this issue Sep 6, 2024 · 3 comments
Open

Calling 2 times Frida::obtain() cause ACCESS_VIOLATION #149

muniategui opened this issue Sep 6, 2024 · 3 comments

Comments

@muniategui
Copy link

  let firda_runtime = unsafe {
        Frida::obtain();
        Frida::obtain()
    };

or anything similar such as

  let firda_runtime = unsafe {Frida::obtain()};
  let firda_runtime = unsafe {Frida::obtain()};

Causes STATUS_ACCESS_VIOLATION (using WIndows 10 as OS)

I am trying to use it in a application which use asyn so basically when the task is called 2 times it crashes. Tried in a normal code like:

fn main(){
  let firda_runtime = unsafe {Frida::obtain()};
  let firda_runtime = unsafe {Frida::obtain()};
}

And it ends in the same crash

@ajwerner
Copy link
Contributor

ajwerner commented Sep 6, 2024

I think we should probably do something like the following below. Then, internally, all the objects that rely on a Frida should just hold a reference to one, and we should make it all Clone.

struct FridaSingleton {}

impl FridaSingleton {
    fn new() -> Self {
        unsafe { frida_sys::frida_init() };
        FridaSingleton {}
    }
}

impl Drop for FridaSingleton {
    fn drop(&mut self) {
        unsafe { frida_sys::frida_deinit() }
    }
}

static THE_ONE_TRUE_FRIDA: Mutex<Option<Arc<FridaSingleton>>> = Mutex::new(None);

#[derive(Clone)]
pub struct Frida {
    inner: Option<Arc<FridaSingleton>>,
}

impl Frida {
    pub fn obtain() -> Self {
        let mut singleton = THE_ONE_TRUE_FRIDA.lock().unwrap();
        let v = singleton.get_or_insert_with(|| Arc::new(FridaSingleton::new()));
        Self {
            inner: Some(v.clone()),
        }
    }
}

impl Drop for Frida {
    fn drop(&mut self) {
        let Some(inner) = self.inner.take() else {
            panic!("programming error!")
        };
        drop(inner);
        let mut singleton = THE_ONE_TRUE_FRIDA.lock().unwrap();
        let Some(v) = singleton.take_if(|v| Arc::strong_count(v) == 1) else {
            return;
        };
        match Arc::try_unwrap(v) {
            Ok(v) => drop(v),
            Err(_v) => panic!("programming error!"),
        }
    }
}

This relates to #145

@s1341
Copy link
Contributor

s1341 commented Sep 8, 2024

If you wrap it in an Arc, doesn't that ensure drop semantics on its own. Do you really need to implement the Drop manually?

@ajwerner
Copy link
Contributor

ajwerner commented Sep 9, 2024

If you wrap it in an Arc, doesn't that ensure drop semantics on its own. Do you really need to implement the Drop manually?

You need to do this because static THE_ONE_TRUE_FRIDA is holding a reference. You might think that a valid approach would be to stick a Weak in the static rather than an Arc, but sadly in rust the weak references will prevent the value from being dropped until all weaks are dropped :(

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

No branches or pull requests

3 participants