[vbox-dev] [Patch] Fix for bug #10947
Konrad Kuźmiński
kuzminski.konrad at gmail.com
Tue Nov 12 20:06:38 GMT 2013
Hi,
I agree that your approach seems to be better. I played around with the
patch and it solves the problem. I share your concerns regarding STI/MOV SS
scenarios but I wasn't able to observe any unexpected behavior in a few
different cases. Also I couldn't find any logical reason standing behind
"if (fBlockSti || fBlockMovSS)" check. It seems to be an unnecessary
constraint which is a direct cause of the bug. I hope you will come to the
same conclusions after your tests.
Regards,
Konrad
2013/11/12 Ramshankar <ramshankar.venkataraman at oracle.com>
> On 11/12/2013 05:13 PM, Ramshankar wrote:
>
> Hi,
>
> Thanks for the testcase. I took a look at your patch and the testcase.
>
> I don't think injecting #DB directly the way you're suggesting is the
> right thing to do. The right fix, as far as I can see, would be to setup
> the pending debug exceptions VMCS field, that way VMX injects the #DB
> taking into account the priority of exceptions.
>
> The fix would be to simply remove the "if (fBlockSti || fBlockMovSS)" in
> hmR0VmxInjectPendingEvent() but I've not yet tested if it works. Once I
> test that it works, I'll do the needful. I need to verify that I'm not
> screwing up something else in the block-by-STI and block-by-MovSS scenarios.
>
> Thanks again for the patch!
>
>
> Oh and you'll have to call hmR0VmxSaveGuestRflags() before the "if
> (pMixedCtx->eflags.Bits.u1TF)" check in hmR0VmxInjectPendingEvent().
>
>
> Regards,
> Ram.
>
>
> On 11/12/2013 01:44 PM, Konrad Kuźmiński wrote:
>
> Hi,
>
> It looks like gmail silently failed to attach the file and didn't even
> notify me about this. I reattached my program in this message. It was
> compiled with MASM using attached script and tested under Windows XP SP3
> host and guest.
>
> Here's the bat file I used for compilation (I cannot attach such files):
> @REM Make sure this path is correct before attempting to build
> @set MASM_PATH=C:\masm32
> @set PATH=%PATH%;%MASM_PATH%\bin
>
> ml -c -coff -I%MASM_PATH%\include 10947_test.asm
> link /ENTRY:start /SUBSYSTEM:CONSOLE /LIBPATH:%MASM_PATH%\lib
> 10947_test.obj
>
> Regards,
> Konrad
>
>
> 2013/11/12 Ramshankar <ramshankar.venkataraman at oracle.com>
>
>> Hi,
>>
>> You said in the attachment, you have included a testcase, I can't find
>> any attachment in your original email. Could you please re-attach it?
>>
>> Regards,
>> Ram.
>>
>>
>> On 11/09/2013 05:01 AM, Ramshankar wrote:
>>
>>> 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
>>>>
>>>>
>>> _______________________________________________
>>> vbox-dev mailing list
>>> vbox-dev at virtualbox.org
>>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>>>
>>
>>
>
>
>
> _______________________________________________
> vbox-dev mailing listvbox-dev at virtualbox.orghttps://www.virtualbox.org/mailman/listinfo/vbox-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20131112/92082dc7/attachment.html>
More information about the vbox-dev
mailing list