rust: Remove self inside a hook lead to SIGSEGV

[bet4it]

use unicorn_engine::unicorn_const::{Arch, Mode, Permission, SECOND_SCALE};
use unicorn_engine::{Unicorn, UcHookId};

static mut STEP_HOOK: Option<UcHookId> = None;

fn step_hook(uc: &mut Unicorn<()>, _addr: u64, _size: u32) {
    unsafe {
        if let Some(step_hook) = STEP_HOOK {
            uc.remove_hook(step_hook)
                .expect("Failed to remove step hook");
            STEP_HOOK = None
        }
    }
}

fn main() {
    let x86_code = [
        0x48, 0xB8, 0xEF, 0xBE, 0xAD, 0xDE, 0x00, 0x00, 0x00, 0x00, 0x0F, 0x05,
    ];
    let mut uc = unicorn_engine::Unicorn::new(Arch::X86, Mode::MODE_32)
        .expect("failed to initialize unicorn instance");
    assert_eq!(uc.mem_map(0x1000, 0x4000, Permission::ALL), Ok(()));
    assert_eq!(uc.mem_write(0x1000, &x86_code), Ok(()));
    unsafe {
        STEP_HOOK = Some(
            uc.add_code_hook(1, 0, step_hook)
                .expect("Failed to add code hook"),
        )
    }
    let _ = uc.emu_start(
        0x1000,
        (0x1000 + x86_code.len()) as u64,
        10 * SECOND_SCALE,
        0,
    );
}

Refer #1543.

[wtdcode]

@amaanq ideas?

[PhilippTakacs]

can you test following patch:

diff --git a/uc.c b/uc.c
index 75b89a01..31475ac5 100644
--- a/uc.c
+++ b/uc.c
@@ -2053,6 +2053,8 @@ uc_err uc_hook_del(uc_engine *uc, uc_hook hh)
             hook->to_delete = true;
             uc->hooks_count[i]--;
             hook_append(&uc->hooks_to_del, hook);
+            if (hook->type == UC_HOOK_CODE)
+                break_translation_loop(uc);
         }
     }
 

I'm not yet finished reading the code and understanding the problem, but I assume the problem is that code hooks are directly inserted in the translation block and the check is only done in the block generation. So the code hook is called after it was deleted.

Alternative we could add a trampoline function for code hooks which checks the to_delete member of the hook.

[bet4it]

can you test following patch:

diff --git a/uc.c b/uc.c
index 75b89a01..31475ac5 100644
--- a/uc.c
+++ b/uc.c
@@ -2053,6 +2053,8 @@ uc_err uc_hook_del(uc_engine *uc, uc_hook hh)
             hook->to_delete = true;
             uc->hooks_count[i]--;
             hook_append(&uc->hooks_to_del, hook);
+            if (hook->type == UC_HOOK_CODE)
+                break_translation_loop(uc);
         }
     }
 

This works! Great!

[PhilippTakacs]

Good.

For my fix: I currently have not that much time to make a full PR, so if someone else is interested just take my change and open a PR. But first check if block hooks are also effected and write some tests. Also consider adding the change of #2246.