frida / frida-rust

Frida Rust bindings
Other
177 stars 46 forks source link

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

Open muniategui opened 2 weeks ago

muniategui commented 2 weeks ago
  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 commented 2 weeks ago

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 https://github.com/frida/frida-rust/issues/145

s1341 commented 1 week ago

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 commented 1 week ago

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 :(