do_smm_enter and helper_rsm sets the env->dr, but does not sync the
values with cpu_x86_update_dr7. A malicious kernel may control the
instruction pointer in SMM by setting a breakpoint on the SMI
entry point, and after do_smm_enter cpu->breakpoints contains the
stale breakpoint; and because IDT is not reloaded upon SMI entry,
the debug exception handler controlled by the malicious kernel
is invoked.

Fixes: 01df040b5247 ("x86: Debug register emulation (Jan Kiszka)")
Reported-by: unvariant.winter@gmail.com
Signed-off-by: YiFei Zhu <zhuyifei@google.com>
---
 target/i386/tcg/system/smm_helper.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/i386/tcg/system/smm_helper.c b/target/i386/tcg/system/smm_helper.c
index 251eb7856c..fb028a8272 100644
--- a/target/i386/tcg/system/smm_helper.c
+++ b/target/i386/tcg/system/smm_helper.c
@@ -168,7 +168,7 @@ void do_smm_enter(X86CPU *cpu)
                        env->cr[0] & ~(CR0_PE_MASK | CR0_EM_MASK | CR0_TS_MASK |
                                       CR0_PG_MASK));
     cpu_x86_update_cr4(env, 0);
-    env->dr[7] = 0x00000400;
+    helper_set_dr(env, 7, 0x00000400);
 
     cpu_x86_load_seg_cache(env, R_CS, (env->smbase >> 4) & 0xffff, env->smbase,
                            0xffffffff,
@@ -233,8 +233,8 @@ void helper_rsm(CPUX86State *env)
     env->eip = x86_ldq_phys(cs, sm_state + 0x7f78);
     cpu_load_eflags(env, x86_ldl_phys(cs, sm_state + 0x7f70),
                     ~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C | DF_MASK));
-    env->dr[6] = x86_ldl_phys(cs, sm_state + 0x7f68);
-    env->dr[7] = x86_ldl_phys(cs, sm_state + 0x7f60);
+    helper_set_dr(env, 6, x86_ldl_phys(cs, sm_state + 0x7f68));
+    helper_set_dr(env, 7, x86_ldl_phys(cs, sm_state + 0x7f60));
 
     cpu_x86_update_cr4(env, x86_ldl_phys(cs, sm_state + 0x7f48));
     cpu_x86_update_cr3(env, x86_ldq_phys(cs, sm_state + 0x7f50));
@@ -268,8 +268,8 @@ void helper_rsm(CPUX86State *env)
     env->regs[R_EDX] = x86_ldl_phys(cs, sm_state + 0x7fd8);
     env->regs[R_ECX] = x86_ldl_phys(cs, sm_state + 0x7fd4);
     env->regs[R_EAX] = x86_ldl_phys(cs, sm_state + 0x7fd0);
-    env->dr[6] = x86_ldl_phys(cs, sm_state + 0x7fcc);
-    env->dr[7] = x86_ldl_phys(cs, sm_state + 0x7fc8);
+    helper_set_dr(env, 6, x86_ldl_phys(cs, sm_state + 0x7fcc));
+    helper_set_dr(env, 7, x86_ldl_phys(cs, sm_state + 0x7fc8));
 
     env->tr.selector = x86_ldl_phys(cs, sm_state + 0x7fc4) & 0xffff;
     env->tr.base = x86_ldl_phys(cs, sm_state + 0x7f64);
-- 
2.51.0.536.g15c5d4f767-goog

On 9/25/25 12:30, YiFei Zhu wrote:
> do_smm_enter and helper_rsm sets the env->dr, but does not sync the
> values with cpu_x86_update_dr7. A malicious kernel may control the
> instruction pointer in SMM by setting a breakpoint on the SMI
> entry point, and after do_smm_enter cpu->breakpoints contains the
> stale breakpoint; and because IDT is not reloaded upon SMI entry,
> the debug exception handler controlled by the malicious kernel
> is invoked.
> 
> Fixes: 01df040b5247 ("x86: Debug register emulation (Jan Kiszka)")
> Reported-by: unvariant.winter@gmail.com
> Signed-off-by: YiFei Zhu <zhuyifei@google.com>

Applied, thanks!

Paolo

> ---
>   target/i386/tcg/system/smm_helper.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target/i386/tcg/system/smm_helper.c b/target/i386/tcg/system/smm_helper.c
> index 251eb7856c..fb028a8272 100644
> --- a/target/i386/tcg/system/smm_helper.c
> +++ b/target/i386/tcg/system/smm_helper.c
> @@ -168,7 +168,7 @@ void do_smm_enter(X86CPU *cpu)
>                          env->cr[0] & ~(CR0_PE_MASK | CR0_EM_MASK | CR0_TS_MASK |
>                                         CR0_PG_MASK));
>       cpu_x86_update_cr4(env, 0);
> -    env->dr[7] = 0x00000400;
> +    helper_set_dr(env, 7, 0x00000400);
>   
>       cpu_x86_load_seg_cache(env, R_CS, (env->smbase >> 4) & 0xffff, env->smbase,
>                              0xffffffff,
> @@ -233,8 +233,8 @@ void helper_rsm(CPUX86State *env)
>       env->eip = x86_ldq_phys(cs, sm_state + 0x7f78);
>       cpu_load_eflags(env, x86_ldl_phys(cs, sm_state + 0x7f70),
>                       ~(CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C | DF_MASK));
> -    env->dr[6] = x86_ldl_phys(cs, sm_state + 0x7f68);
> -    env->dr[7] = x86_ldl_phys(cs, sm_state + 0x7f60);
> +    helper_set_dr(env, 6, x86_ldl_phys(cs, sm_state + 0x7f68));
> +    helper_set_dr(env, 7, x86_ldl_phys(cs, sm_state + 0x7f60));
>   
>       cpu_x86_update_cr4(env, x86_ldl_phys(cs, sm_state + 0x7f48));
>       cpu_x86_update_cr3(env, x86_ldq_phys(cs, sm_state + 0x7f50));
> @@ -268,8 +268,8 @@ void helper_rsm(CPUX86State *env)
>       env->regs[R_EDX] = x86_ldl_phys(cs, sm_state + 0x7fd8);
>       env->regs[R_ECX] = x86_ldl_phys(cs, sm_state + 0x7fd4);
>       env->regs[R_EAX] = x86_ldl_phys(cs, sm_state + 0x7fd0);
> -    env->dr[6] = x86_ldl_phys(cs, sm_state + 0x7fcc);
> -    env->dr[7] = x86_ldl_phys(cs, sm_state + 0x7fc8);
> +    helper_set_dr(env, 6, x86_ldl_phys(cs, sm_state + 0x7fcc));
> +    helper_set_dr(env, 7, x86_ldl_phys(cs, sm_state + 0x7fc8));
>   
>       env->tr.selector = x86_ldl_phys(cs, sm_state + 0x7fc4) & 0xffff;
>       env->tr.base = x86_ldl_phys(cs, sm_state + 0x7f64);