From 7bb756249a54a2163d5cb65f35b64912451f0752 Mon Sep 17 00:00:00 2001
From: lazymio <mio@lazym.io>
Date: Wed, 22 Dec 2021 20:36:56 +0100
Subject: [PATCH] Better design of cpuid instruction hook

---
 include/unicorn/x86.h          |  8 ++++++++
 qemu/target/i386/misc_helper.c | 23 +++++++++++++++--------
 tests/unit/test_x86.c          |  5 ++++-
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/include/unicorn/x86.h b/include/unicorn/x86.h
index d58de722..a4afcd9c 100644
--- a/include/unicorn/x86.h
+++ b/include/unicorn/x86.h
@@ -75,6 +75,14 @@ typedef struct uc_x86_msr {
 // @user_data: user data passed to tracing APIs.
 typedef void (*uc_cb_insn_syscall_t)(struct uc_struct *uc, void *user_data);
 
+// Callback function for tracing cpuid (for uc_hook_intr())
+// @user_data: user data passed to tracing APIs.
+//
+// @return: true indicates the callback overwrites the cpuid instruction while
+// false
+//          indicates cpuid instruction will still be executed.
+typedef int (*uc_cb_insn_cpuid_t)(struct uc_struct *uc, void *user_data);
+
 //> X86 registers
 typedef enum uc_x86_reg {
     UC_X86_REG_INVALID = 0,
diff --git a/qemu/target/i386/misc_helper.c b/qemu/target/i386/misc_helper.c
index f9924368..f04f0237 100644
--- a/qemu/target/i386/misc_helper.c
+++ b/qemu/target/i386/misc_helper.c
@@ -106,16 +106,10 @@ void helper_cpuid(CPUX86State *env)
 {
     uint32_t eax, ebx, ecx, edx;
     struct hook *hook;
+    int skip_cpuid = 0;
 
     cpu_svm_check_intercept_param(env, SVM_EXIT_CPUID, 0, GETPC());
 
-    cpu_x86_cpuid(env, (uint32_t)env->regs[R_EAX], (uint32_t)env->regs[R_ECX],
-                  &eax, &ebx, &ecx, &edx);
-    env->regs[R_EAX] = eax;
-    env->regs[R_EBX] = ebx;
-    env->regs[R_ECX] = ecx;
-    env->regs[R_EDX] = edx;
-
     // Unicorn: call registered CPUID hooks
     HOOK_FOREACH_VAR_DECLARE;
     HOOK_FOREACH(env->uc, hook, UC_HOOK_INSN) {
@@ -123,13 +117,26 @@ void helper_cpuid(CPUX86State *env)
             continue;
         if (!HOOK_BOUND_CHECK(hook, env->eip))
             continue;
+        
+        // Multiple cpuid callbacks returning different values is undefined.
+        // true -> skip the cpuid instruction
         if (hook->insn == UC_X86_INS_CPUID)
-            ((uc_cb_insn_syscall_t)hook->callback)(env->uc, hook->user_data);
+            skip_cpuid = ((uc_cb_insn_cpuid_t)hook->callback)(env->uc, hook->user_data);
 
         // the last callback may already asked to stop emulation
         if (env->uc->stop_request)
             break;
     }
+
+    if (!skip_cpuid) {
+        cpu_x86_cpuid(env, (uint32_t)env->regs[R_EAX], (uint32_t)env->regs[R_ECX],
+                    &eax, &ebx, &ecx, &edx);
+        env->regs[R_EAX] = eax;
+        env->regs[R_EBX] = ebx;
+        env->regs[R_ECX] = ecx;
+        env->regs[R_EDX] = edx;
+    }
+    
 }
 
 target_ulong helper_read_crN(CPUX86State *env, int reg)
diff --git a/tests/unit/test_x86.c b/tests/unit/test_x86.c
index e8bb68a0..ccf98e06 100644
--- a/tests/unit/test_x86.c
+++ b/tests/unit/test_x86.c
@@ -601,11 +601,14 @@ static void test_x86_sysenter()
     OK(uc_close(uc));
 }
 
-static void test_x86_hook_cpuid_callback(uc_engine *uc, void *data)
+static int test_x86_hook_cpuid_callback(uc_engine *uc, void *data)
 {
     int reg = 7;
 
     OK(uc_reg_write(uc, UC_X86_REG_EAX, &reg));
+
+    // Overwrite the cpuid instruction.
+    return 1;
 }
 
 static void test_x86_hook_cpuid()