[vbox-dev] [Patch] Fix for bug #10947

[Ramshankar]

Hi,

Thank you for the patch. I'll take a look at it next week.

Regards,
Ram.

On 08/11/13 22:13, Konrad Kuźmiński wrote:
> Hi,
>
> I've made a fix for this bug https://www.virtualbox.org/ticket/10947.
> Mentioned ticket isn't very descriptive so I will try to explain this
> issue a little bit more in detail. First of all this problem occurs only
> when VT-x is enabled. Basically some instructions don't generate
> expected single step exception after they are executed with the trap
> flag being set. The behaviour is observed that such instructions are
> executed under the control of the guest system but single step exception
> is generated after the next instruction. This is a well known bug
> amongst malware researchers and malware authors who can easily take
> advantage of this fact in order to detect virtualized environment.
>
> It turns out that the problem lies in the way VirtualBox handles some VM
> exits initiated by the execution of certain instructions. Several
> instructions can never be executed in VMX non-root operation and those
> need to be emulated and skipped within VM exit handlers by adjusting
> RIP. Unfortunately the code lacks necessary check for the trap flag
> being set, so it doesn't inject expected exception into the guest.
>
> Here's the fix:
> *** src\VBox\VMM\VMMR0\HMVMXR0_original.cpp    2013-11-01
> 18:58:26.000000000 +0100
> --- src\VBox\VMM\VMMR0\HMVMXR0_fixed.cpp        2013-11-08
> 20:24:30.578125000 +0100
> ***************
> *** 8166,8181 ****
> --- 8166,8190 ----
>    DECLINLINE(int) hmR0VmxAdvanceGuestRip(PVMCPU pVCpu, PCPUMCTX
> pMixedCtx, PVMXTRANSIENT pVmxTransient)
>    {
>        int rc = hmR0VmxReadExitInstrLenVmcs(pVCpu, pVmxTransient);
>        rc    |= hmR0VmxSaveGuestRip(pVCpu, pMixedCtx);
>        AssertRCReturn(rc, rc);
>
>        pMixedCtx->rip += pVmxTransient->cbInstr;
>        VMCPU_HMCF_SET(pVCpu, HM_CHANGED_GUEST_RIP);
> +
> +     X86EFLAGS Eflags;
> +     rc = VMXReadVmcs32(VMX_VMCS_GUEST_RFLAGS, &Eflags.u32);
> +     AssertRCReturn(rc, rc);
> +     if (Eflags.Bits.u1TF)
> +     {
> +         hmR0VmxSetPendingXcptDB(pVCpu, pMixedCtx);
> +     }
> +
>        return rc;
>    }
>
> This fix ensures correct handling of mentioned condition in all 13
> affected VM exit handlers: VMX_EXIT_CPUID, VMX_EXIT_RDTSC,
> VMX_EXIT_RDTSCP, VMX_EXIT_RDPMC, VMX_EXIT_MOV_CRX, VMX_EXIT_MOV_DRX,
> VMX_EXIT_MWAIT, VMX_EXIT_MONITOR, VMX_EXIT_RDMSR, VMX_EXIT_WRMSR,
> VMX_EXIT_INVD, VMX_EXIT_INVLPG, VMX_EXIT_WBINVD.
>
> In the attachment I provided a simple program which can be used to test
> this condition on 2 representative instructions: CPUID and RDTSC. I
> picked those because they don't require CPL = 0.
>
> I release this patch and test program under MIT license.
>
> Best regards,
> Konrad
>
>
> _______________________________________________
> vbox-dev mailing list
> vbox-dev at virtualbox.org
> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>