[vbox-dev] [Patch] Fix for bug #10947
Ramshankar
ramshankar.venkataraman at oracle.com
Thu Nov 14 12:58:58 GMT 2013
I've fixed the bug in SVN. Should be part of the next maintenance
release of VirtualBox 4.3 branch.
Thanks & Regards,
Ram.
On 11/12/2013 09:06 PM, Konrad Kuźmiński wrote:
> 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
> <mailto: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
>>> <mailto: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 <mailto:vbox-dev at virtualbox.org>
>>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>>>
>>>
>>> _______________________________________________
>>> vbox-dev mailing list
>>> vbox-dev at virtualbox.org <mailto:vbox-dev at virtualbox.org>
>>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>>>
>>>
>>>
>>
>>
>>
>> _______________________________________________
>> vbox-dev mailing list
>> vbox-dev at virtualbox.org <mailto:vbox-dev at virtualbox.org>
>> https://www.virtualbox.org/mailman/listinfo/vbox-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20131114/5de64551/attachment.html>
More information about the vbox-dev
mailing list