Xen devel - Mar 2008 - [PATCH]Fix the bug of guest os installation failure and win2k boot failure

Hi, Keir,

      This patch is to fix the problem of Linux guest installation failure and
Windows 2000 boot failure.

      In the early code, we use vmx_vmexit_handler() -> vmx_io_instruction()
function to emulate I/O instructions. But now, we use vmx_vmexit_handler() ->
handle_mmio -> hvm_emulate_one() -> x86_emulate() to emulate I/O
instructions. Also nowadays, the realmode emulation code walks through the path:
vmx_realmode() -> realmode_emulate_one() -> hvm_emulate_one() ->
x86_emulate().

      The I/O handle code in x86_emulate() checks the cpl and iopl value, and if
cpl > iopl, it will generate a GP fault. This causes Linux guest installation
failure and Windows 2000 boot failure. I think this check code may be not
reasonable for two aspects:

      1. If x86_emulate() is called from vmexit or from realmode emulation, I
think this line of code is not needed, because:

            a). In I/O emulation, the cpu has already checked the cpl, iopl, and
also the I/O bitmap before vmx_vmexit_handler() is called,
            b). For realmode, we shouldn't check the cpl and iopl, because
any I/O operation is permitted in realmode.

      2. If x86_emulate() is called from multi.c, which emulates up to four
instructions when dealing with PAE guest page tables. In this condition, the
check is needed, but it is not correct, it should follow the code as follows,
which is stated in the Intel SDM:

　If (cpl <= iopl)
　    Do I/O operation;
　Else {
　    If (I/O permission bit for the port == 0)
　        Do I/O operation;
　    Else 
　        Generate GP fault;
　} 

      Now this patch remove the cpl and iopl check in I/O handler code in
x86_emulate() function. And it checks the four instructions which would be
emulated by multi.c, if any of them is IN/INS/OUT/OUTS, or REP IN/INS/OUT/OUTS,
we will break that four-instruction emulation, and let the I/O instruction walk
through the path of vmx_vmexit_handler() -> handle_mmio ->
hvm_emulate_one() -> x86_emulate().

      Another way to solve this issue could be that, we put the entire io
permission check in x86_emulate(), and use a flag to indicate whether we should
do the check. If x86_emulate() is called by vmexit or realmode emulation, we
skip this check; if it is called by multi.c, then we do the io permission check.
But it may be a bit complex for hypervisor to read guest process’s TSS and find
and check its io bitmap.

BTW: Why the existence code doesn't check the LOCK prefix (which should
cause #UD injected to guest)
      
      Welcome for your comment, thanks! 

Signed-off-by: Xu Dongxiao <dongxiao.xu@intel.com>

Best Regards,
--Dongxiao



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Dongxiao,

Thanks for tracking this one down this far! I''m not fully convinced by
your
analysis however, for the following reasons:

 1. In real mode we must surely have CPL==SS.DPL==0, otherwise the write of
CR0.PE would have been disallowed. Hence all I/O port operations must surely
succeed in our emulated real mode.

 2. I don''t understand your changes to the shadow emulation path.
Firstly,
the shadow emulator does not provide callback functions for I/O-port
operations, so they can never be emulated. Secondly, even if we do fall into
one of the generate_exception_if(!mode_iopl(), EXC_GP) statements, the
shadow emulator does not provide an inject_hw_exception() callback and hence
no exception will be generated and instead the emulation will (correctly)
fail.

This makes sense, since the nasty guest-installation problems have only
appeared (as far as I know) since the old I/O vmexit path was changed to use
x86_emulate(). Before that the 4-instruction shadow emulator and also the
real-mode emulator were working pretty much fine!

So... with regard to what is wrong with the I/O vmexit path. I agree that
the CPL-IOPL check is redundant, but it *should* work! Are we simply falling
down because we are not also checking the TSS bitmap?

Removing the IOPL checks from x86_emulate() may be the right thing to do,
but I would like to really understand the underlying root-cause problem
first.

 -- Keir

On 17/3/08 08:08, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> Hi, Keir,
> 
>       This patch is to fix the problem of Linux guest installation failure
and
> Windows 2000 boot failure.
> 
> ????? In the early code, we use vmx_vmexit_handler() ->
vmx_io_instruction()
> function to emulate I/O instructions. But now, we use vmx_vmexit_handler()
->
> handle_mmio -> hvm_emulate_one() -> x86_emulate() to emulate I/O
instructions.
> Also nowadays, the realmode emulation code walks through the path:
> vmx_realmode() -> realmode_emulate_one() -> hvm_emulate_one() ->
> x86_emulate().
> 
> ????? The I/O handle code in x86_emulate() checks the cpl and iopl value,
and
> if cpl > iopl, it will generate a GP fault. This causes Linux guest
> installation failure and Windows 2000 boot failure. I think this check code
> may be not reasonable for two aspects:
> 
> ????? 1. If x86_emulate() is called from vmexit or from realmode emulation,
I
> think this line of code is not needed, because:
> 
> ??????????? a). In I/O emulation, the cpu has already checked the cpl,
iopl,
> and also the I/O bitmap before vmx_vmexit_handler() is called,
> ??????????? b). For realmode, we shouldn''t check the cpl and iopl,
because any
> I/O operation is permitted in realmode.
> 
> ????? 2. If x86_emulate() is called from multi.c, which emulates up to four
> instructions when dealing with PAE guest page tables. In this condition,
the
> check is needed, but it is not correct, it should follow the code as
follows,
> which is stated in the Intel SDM:
> 
> 　If (cpl <= iopl)
> 　??? Do I/O operation;
> 　Else {
> 　??? If (I/O permission bit for the port == 0)
> 　??????? Do I/O operation;
> 　??? Else 
> 　??????? Generate GP fault;
> 　} 
> 
> ????? Now this patch remove the cpl and iopl check in I/O handler code in
> x86_emulate() function. And it checks the four instructions which would be
> emulated by multi.c, if any of them is IN/INS/OUT/OUTS, or REP
> IN/INS/OUT/OUTS, we will break that four-instruction emulation, and let the
> I/O instruction walk through the path of vmx_vmexit_handler() ->
handle_mmio
> -> hvm_emulate_one() -> x86_emulate().
> 
>       Another way to solve this issue could be that, we put the entire io
> permission check in x86_emulate(), and use a flag to indicate whether we
> should do the check. If x86_emulate() is called by vmexit or realmode
> emulation, we skip this check; if it is called by multi.c, then we do the
io
> permission check. But it may be a bit complex for hypervisor to read guest
> process’s TSS and find and check its io bitmap.
> 
> BTW: Why the existence code doesn''t check the LOCK prefix (which
should cause
> #UD injected to guest)
>       
>       Welcome for your comment, thanks!
> 
> Signed-off-by: Xu Dongxiao <dongxiao.xu@intel.com>
> 
> Best Regards,
> --Dongxiao
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, Keir, 

    Thanks for your response.

    Realmode emulation code path for I/O is fine. I just think that realmode I/O
doesn''t need that line of check code.

    For vmexit code path: When I debug this issue, I found that for some I/O
vmexits which happened when /sbin/loader is executed, the cpl is 3, the iopl is
0, so when "generate_exception_if(!mode_iopl(), EXC_GP)" checks cpl
and iopl relationship, it push out a GP fault and makes the guest installation
fail. Actually before the code enters vmx_vmexit_handler(), the processor has
already checked the I/O permission. So here I think that line of check code is
not needed.

    Also we haven''t found any bug caused by the 4-instruction emulation
till now. Adding the change in the shadow code path is because: There may be I/O
instructions among the 4 instructions in theory. In this case, I think a full
check of cpl, iopl, and the I/O bitmap is needed. So we may either add the I/O
permission check in software, or break the 4-instruction emulate and let
processor do the I/O permission check, then emulate it by
vmx_vmexit_handler()->handle_mmio() code path. Here the patch uses the second
way.

    There may be some points that I haven''t considered. Thanks for
comments!

Best Regards,
-- Dongxiao
    



-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] 
Sent: 2008年3月17日 16:32
To: Xu, Dongxiao; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH]Fix the bug of guest os installation failure and
win2k boot failure

Hi Dongxiao,

Thanks for tracking this one down this far! I''m not fully convinced by
your
analysis however, for the following reasons:

 1. In real mode we must surely have CPL==SS.DPL==0, otherwise the write of
CR0.PE would have been disallowed. Hence all I/O port operations must surely
succeed in our emulated real mode.

 2. I don''t understand your changes to the shadow emulation path.
Firstly,
the shadow emulator does not provide callback functions for I/O-port
operations, so they can never be emulated. Secondly, even if we do fall into
one of the generate_exception_if(!mode_iopl(), EXC_GP) statements, the
shadow emulator does not provide an inject_hw_exception() callback and hence
no exception will be generated and instead the emulation will (correctly)
fail.

This makes sense, since the nasty guest-installation problems have only
appeared (as far as I know) since the old I/O vmexit path was changed to use
x86_emulate(). Before that the 4-instruction shadow emulator and also the
real-mode emulator were working pretty much fine!

So... with regard to what is wrong with the I/O vmexit path. I agree that
the CPL-IOPL check is redundant, but it *should* work! Are we simply falling
down because we are not also checking the TSS bitmap?

Removing the IOPL checks from x86_emulate() may be the right thing to do,
but I would like to really understand the underlying root-cause problem
first.

 -- Keir

On 17/3/08 08:08, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> Hi, Keir,
> 
>       This patch is to fix the problem of Linux guest installation failure
and
> Windows 2000 boot failure.
> 
> ????? In the early code, we use vmx_vmexit_handler() ->
vmx_io_instruction()
> function to emulate I/O instructions. But now, we use vmx_vmexit_handler()
->
> handle_mmio -> hvm_emulate_one() -> x86_emulate() to emulate I/O
instructions.
> Also nowadays, the realmode emulation code walks through the path:
> vmx_realmode() -> realmode_emulate_one() -> hvm_emulate_one() ->
> x86_emulate().
> 
> ????? The I/O handle code in x86_emulate() checks the cpl and iopl value,
and
> if cpl > iopl, it will generate a GP fault. This causes Linux guest
> installation failure and Windows 2000 boot failure. I think this check code
> may be not reasonable for two aspects:
> 
> ????? 1. If x86_emulate() is called from vmexit or from realmode emulation,
I
> think this line of code is not needed, because:
> 
> ??????????? a). In I/O emulation, the cpu has already checked the cpl,
iopl,
> and also the I/O bitmap before vmx_vmexit_handler() is called,
> ??????????? b). For realmode, we shouldn''t check the cpl and iopl,
because any
> I/O operation is permitted in realmode.
> 
> ????? 2. If x86_emulate() is called from multi.c, which emulates up to four
> instructions when dealing with PAE guest page tables. In this condition,
the
> check is needed, but it is not correct, it should follow the code as
follows,
> which is stated in the Intel SDM:
> 
> 　If (cpl <= iopl)
> 　??? Do I/O operation;
> 　Else {
> 　??? If (I/O permission bit for the port == 0)
> 　??????? Do I/O operation;
> 　??? Else 
> 　??????? Generate GP fault;
> 　} 
> 
> ????? Now this patch remove the cpl and iopl check in I/O handler code in
> x86_emulate() function. And it checks the four instructions which would be
> emulated by multi.c, if any of them is IN/INS/OUT/OUTS, or REP
> IN/INS/OUT/OUTS, we will break that four-instruction emulation, and let the
> I/O instruction walk through the path of vmx_vmexit_handler() ->
handle_mmio
> -> hvm_emulate_one() -> x86_emulate().
> 
>       Another way to solve this issue could be that, we put the entire io
> permission check in x86_emulate(), and use a flag to indicate whether we
> should do the check. If x86_emulate() is called by vmexit or realmode
> emulation, we skip this check; if it is called by multi.c, then we do the
io
> permission check. But it may be a bit complex for hypervisor to read guest
> process’s TSS and find and check its io bitmap.
> 
> BTW: Why the existence code doesn''t check the LOCK prefix (which
should cause
> #UD injected to guest)
>       
>       Welcome for your comment, thanks!
> 
> Signed-off-by: Xu Dongxiao <dongxiao.xu@intel.com>
> 
> Best Regards,
> --Dongxiao
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 17/3/08 10:06, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
>     Realmode emulation code path for I/O is fine. I just think that
realmode
> I/O doesn''t need that line of check code.
Okay, understood and I agree.
>     For vmexit code path: When I debug this issue, I found that for some
I/O
> vmexits which happened when /sbin/loader is executed, the cpl is 3, the
iopl
> is 0, so when "generate_exception_if(!mode_iopl(), EXC_GP)"
checks cpl and
> iopl relationship, it push out a GP fault and makes the guest installation
> fail. Actually before the code enters vmx_vmexit_handler(), the processor
has
> already checked the I/O permission. So here I think that line of check code
is
> not needed.
Ah, and so we assume that the I/O access is actually permitted by the TSS
I/O bitmap? Seems reasonable, and so we could add that check or just remove
the CPL-IOPL check. I agree again.
>     Also we haven''t found any bug caused by the 4-instruction
emulation till
> now. Adding the change in the shadow code path is because: There may be I/O
> instructions among the 4 instructions in theory. In this case, I think a
full
> check of cpl, iopl, and the I/O bitmap is needed. So we may either add the
I/O
> permission check in software, or break the 4-instruction emulate and let
> processor do the I/O permission check, then emulate it by
> vmx_vmexit_handler()->handle_mmio() code path. Here the patch uses the
second
> way. 
I think you misunderstand. The shadow emulator *never* emulates I/O port
accesses or exception deliveries. Those callback functions are simply not
implemented and are left as NULL. Hence we will fail the IOPL check, but we
will also fail to deliver EXC_GP, and so we will simply return
X86EMUL_UNHANDLEABLE, which is the right thing to do. So, no bug here and
your changes to the shadow emulator are not required.

I will try out a version of your patch which is basically just your
x86_emulate.c changes, and see how that works out for me.

 Thanks!
 Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser wrote:
>>     For vmexit code path: When I debug this issue, I found that for
>> some I/O vmexits which happened when /sbin/loader is executed, the
>> cpl is 3, the iopl is 0, so when
>> "generate_exception_if(!mode_iopl(), EXC_GP)" checks cpl and
iopl
>> relationship, it push out a GP fault and makes the guest
>> installation fail. Actually before the code enters
>> vmx_vmexit_handler(), the processor has already checked the I/O
>> permission. So here I think that line of check code is not needed. 
> 
> Ah, and so we assume that the I/O access is actually permitted by the
> TSS I/O bitmap? 
When a VMexit with reason IO_INSTRUCTION occurs, VMM can tell that: in
guest OS, 
the execution has enough privilege to access the port ( (an actually
virtual port, from VMM''s 
perspective), because if ( vCPL > vIOPL || vTR.io_bitmap[port]==1), CPU
will raise #GP to guest directly without VM-exit.
> Seems reasonable, and so we could add that check or
> just remove the CPL-IOPL check. I agree again.
> 
>>     Also we haven''t found any bug caused by the 4-instruction
>> emulation till now. Adding the change in the shadow code path is
>> because: There may be I/O instructions among the 4 instructions in
>> theory. In this case, I think a full check of cpl, iopl, and the I/O
>> bitmap is needed. So we may either add the I/O permission check in
>> software, or break the 4-instruction emulate and let processor do
>> the I/O permission check, then emulate it by
>> vmx_vmexit_handler()->handle_mmio() code path. Here the patch uses
>> the second way. 
> 
> I think you misunderstand. The shadow emulator *never* emulates I/O
> port accesses or exception deliveries. Those callback functions are
> simply not implemented and are left as NULL. 
Those callback functions -- what are they? -- do you mean the following?
static struct x86_emulate_ops hvm_emulate_ops = {
    ....
    .read_io       = hvmemul_read_io,
    .write_io      = hvmemul_write_io,
...
};

>Hence we will fail the
> IOPL check, but we will also fail to deliver EXC_GP, and so we will
> simply return X86EMUL_UNHANDLEABLE, which is the right thing to do.
> So, no bug here and your changes to the shadow emulator are not
> required. 
> 
> I will try out a version of your patch which is basically just your
> x86_emulate.c changes, and see how that works out for me.
> 
>  Keir

Thanks
-- Dexuan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 17/3/08 11:16, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>> I think you misunderstand. The shadow emulator *never* emulates I/O
>> port accesses or exception deliveries. Those callback functions are
>> simply not implemented and are left as NULL.
> Those callback functions -- what are they? -- do you mean the following?
> static struct x86_emulate_ops hvm_emulate_ops = {
>     ....
>     .read_io       = hvmemul_read_io,
>     .write_io      = hvmemul_write_io,
> ...
> };
Yes indeed. Also, crucially, .inject_hw_exception. Without that
x86_emulate() is unable to inject any exception into the guest, and will
instead return X86EMUL_UNHANDLEABLE.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, Keir, 
    Now I understand what you mean. read_io, write_io, inject_hw_exception
callbacks are not defined within the multi.c. So I/O instructions will not be
emulated by it. Thanks for your comments. And the new patch is attached.

Best regards,
-- Dongxiao

-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] 
Sent: 2008年3月17日 19:21
To: Cui, Dexuan; Xu, Dongxiao; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH]Fix the bug of guest os installationfailure and
win2k boot failure

On 17/3/08 11:16, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>> I think you misunderstand. The shadow emulator *never* emulates I/O
>> port accesses or exception deliveries. Those callback functions are
>> simply not implemented and are left as NULL.
> Those callback functions -- what are they? -- do you mean the following?
> static struct x86_emulate_ops hvm_emulate_ops = {
>     ....
>     .read_io       = hvmemul_read_io,
>     .write_io      = hvmemul_write_io,
> ...
> };
Yes indeed. Also, crucially, .inject_hw_exception. Without that
x86_emulate() is unable to inject any exception into the guest, and will
instead return X86EMUL_UNHANDLEABLE.

 -- Keir




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

We''re on the same page now, except that I realised there is a TOCTTOU
race
introduced by relying on the processor''s permission check while
re-fetching
the instruction from scratch in the hypervisor. This allows, in theory, a
multi-threaded process to rewrite the I/O-port accessing instruction after
the processor has fetched the instruction, and validated the access, but
before Xen re-fetches the instruction for emulation. Possibly we do not care
too much about this, since the process must already have some
I/O-port-access permissions, but equally I don''t expect we fall into
the
TSS-bitmap check all that often, it''s not that hard to implement, and
then
we are definitely safe.

 -- Keir

On 18/3/08 01:49, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> Hi, Keir, 
>     Now I understand what you mean. read_io, write_io, inject_hw_exception
> callbacks are not defined within the multi.c. So I/O instructions will not
be
> emulated by it. Thanks for your comments. And the new patch is attached.
> 
> Best regards,
> -- Dongxiao
> 
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: 2008年3月17日 19:21
> To: Cui, Dexuan; Xu, Dongxiao; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH]Fix the bug of guest os installationfailure
> and win2k boot failure
> 
> On 17/3/08 11:16, "Cui, Dexuan" <dexuan.cui@intel.com>
wrote:
> 
>>> I think you misunderstand. The shadow emulator *never* emulates I/O
>>> port accesses or exception deliveries. Those callback functions are
>>> simply not implemented and are left as NULL.
>> Those callback functions -- what are they? -- do you mean the
following?
>> static struct x86_emulate_ops hvm_emulate_ops = {
>>     ....
>>     .read_io       = hvmemul_read_io,
>>     .write_io      = hvmemul_write_io,
>> ...
>> };
> 
> Yes indeed. Also, crucially, .inject_hw_exception. Without that
> x86_emulate() is unable to inject any exception into the guest, and will
> instead return X86EMUL_UNHANDLEABLE.
> 
>  -- Keir
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, Keir, 
    Do you mean that in a multi-thread process, one thread issues an I/O
operation, and in the time slot that just after the processor has fetched the
instruction, validated the access, but before Xen re-fetches the instruction for
emulation, another thread steals that I/O instruction and replace it with a new
one? Maybe we can regard it as a kind of attack...
    This could be happen in theory, but I think other instruction emulation may
also have this problem. In your last sentence, do you mean that we still need to
do an entire I/O permission check (including CPL, IOPL, and TSS I/O bitmap) in
x86_emulate() for safety consideration? Thanks! :-)

Best regards,
-- Dongxiao


-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] 
Sent: 2008年3月18日 16:46
To: Xu, Dongxiao; Cui, Dexuan; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH]Fix the bug of guest os installationfailure and
win2k boot failure

We''re on the same page now, except that I realised there is a TOCTTOU
race
introduced by relying on the processor''s permission check while
re-fetching
the instruction from scratch in the hypervisor. This allows, in theory, a
multi-threaded process to rewrite the I/O-port accessing instruction after
the processor has fetched the instruction, and validated the access, but
before Xen re-fetches the instruction for emulation. Possibly we do not care
too much about this, since the process must already have some
I/O-port-access permissions, but equally I don''t expect we fall into
the
TSS-bitmap check all that often, it''s not that hard to implement, and
then
we are definitely safe.

 -- Keir

On 18/3/08 01:49, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> Hi, Keir, 
>     Now I understand what you mean. read_io, write_io, inject_hw_exception
> callbacks are not defined within the multi.c. So I/O instructions will not
be
> emulated by it. Thanks for your comments. And the new patch is attached.
> 
> Best regards,
> -- Dongxiao
> 
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: 2008年3月17日 19:21
> To: Cui, Dexuan; Xu, Dongxiao; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH]Fix the bug of guest os installationfailure
> and win2k boot failure
> 
> On 17/3/08 11:16, "Cui, Dexuan" <dexuan.cui@intel.com>
wrote:
> 
>>> I think you misunderstand. The shadow emulator *never* emulates I/O
>>> port accesses or exception deliveries. Those callback functions are
>>> simply not implemented and are left as NULL.
>> Those callback functions -- what are they? -- do you mean the
following?
>> static struct x86_emulate_ops hvm_emulate_ops = {
>>     ....
>>     .read_io       = hvmemul_read_io,
>>     .write_io      = hvmemul_write_io,
>> ...
>> };
> 
> Yes indeed. Also, crucially, .inject_hw_exception. Without that
> x86_emulate() is unable to inject any exception into the guest, and will
> instead return X86EMUL_UNHANDLEABLE.
> 
>  -- Keir
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 18/3/08 09:35, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
>     Do you mean that in a multi-thread process, one thread issues an I/O
> operation, and in the time slot that just after the processor has fetched
the
> instruction, validated the access, but before Xen re-fetches the
instruction
> for emulation, another thread steals that I/O instruction and replace it
with
> a new one? Maybe we can regard it as a kind of attack...
We could regard it as that, since that is what it would be. :-)
>     This could be happen in theory, but I think other instruction emulation
> may also have this problem.
Which other instruction emulations? Can you give an example?
> In your last sentence, do you mean that we still
> need to do an entire I/O permission check (including CPL, IOPL, and TSS I/O
> bitmap) in x86_emulate() for safety consideration? Thanks! :-)
Yes. Like I said: the CPL-IOPL check is very cheap, the TSS bitmap check is
a little more expensive but probably relatively rare. And in any case the
I/O port access latency is largely dominated by the VMEXIT/VMENTRY times.
Also the devices we emulate are mostly managed by mmio.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, Keir, 
    I think it is a common problem because there is always a time slot for all
instruction emulation. In the time slot, an attacker could replace the old
instruction with a new one. Just for example, if one thread issues a "push
reg" operation, and during the time slot, another thread can replace it
with a "pop reg" operation. Because there is no mechanism for us to
check whether the instruction has been changed during that time slot. This may
cause the guest OS doesn''t work well. So, I think this kind of issue
may not only happen with I/O emulations. :-)
    
Best regards,
-- Dongxiao

-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] 
Sent: 2008年3月18日 18:03
To: Xu, Dongxiao; Cui, Dexuan; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH]Fix the bug of guest os installationfailure and
win2k boot failure

On 18/3/08 09:35, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
>     Do you mean that in a multi-thread process, one thread issues an I/O
> operation, and in the time slot that just after the processor has fetched
the
> instruction, validated the access, but before Xen re-fetches the
instruction
> for emulation, another thread steals that I/O instruction and replace it
with
> a new one? Maybe we can regard it as a kind of attack...
We could regard it as that, since that is what it would be. :-)
>     This could be happen in theory, but I think other instruction emulation
> may also have this problem.
Which other instruction emulations? Can you give an example?
> In your last sentence, do you mean that we still
> need to do an entire I/O permission check (including CPL, IOPL, and TSS I/O
> bitmap) in x86_emulate() for safety consideration? Thanks! :-)
Yes. Like I said: the CPL-IOPL check is very cheap, the TSS bitmap check is
a little more expensive but probably relatively rare. And in any case the
I/O port access latency is largely dominated by the VMEXIT/VMENTRY times.
Also the devices we emulate are mostly managed by mmio.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

That example is not a security hole, though. The worst the process can do is
shoot itself in the foot. The time slot exists for execution by the real
processor too, does it not?

 -- Keir

On 18/3/08 10:31, "Xu, Dongxiao" <dongxiao.xu@intel.com> wrote:
> Hi, Keir, 
>     I think it is a common problem because there is always a time slot for
all
> instruction emulation. In the time slot, an attacker could replace the old
> instruction with a new one. Just for example, if one thread issues a
"push
> reg" operation, and during the time slot, another thread can replace
it with a
> "pop reg" operation. Because there is no mechanism for us to
check whether the
> instruction has been changed during that time slot. This may cause the
guest
> OS doesn''t work well. So, I think this kind of issue may not only
happen with
> I/O emulations. :-)
>     
> Best regards,
> -- Dongxiao
> 
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: 2008年3月18日 18:03
> To: Xu, Dongxiao; Cui, Dexuan; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH]Fix the bug of guest os installationfailure
> and win2k boot failure
> 
> On 18/3/08 09:35, "Xu, Dongxiao" <dongxiao.xu@intel.com>
wrote:
> 
>>     Do you mean that in a multi-thread process, one thread issues an
I/O
>> operation, and in the time slot that just after the processor has
fetched the
>> instruction, validated the access, but before Xen re-fetches the
instruction
>> for emulation, another thread steals that I/O instruction and replace
it with
>> a new one? Maybe we can regard it as a kind of attack...
> 
> We could regard it as that, since that is what it would be. :-)
> 
>>     This could be happen in theory, but I think other instruction
emulation
>> may also have this problem.
> 
> Which other instruction emulations? Can you give an example?
> 
>> In your last sentence, do you mean that we still
>> need to do an entire I/O permission check (including CPL, IOPL, and TSS
I/O
>> bitmap) in x86_emulate() for safety consideration? Thanks! :-)
> 
> Yes. Like I said: the CPL-IOPL check is very cheap, the TSS bitmap check is
> a little more expensive but probably relatively rare. And in any case the
> I/O port access latency is largely dominated by the VMEXIT/VMENTRY times.
> Also the devices we emulate are mostly managed by mmio.
> 
>  -- Keir
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 18/3/08 10:02, "Keir Fraser" <keir.fraser@eu.citrix.com>
wrote:
>> In your last sentence, do you mean that we still
>> need to do an entire I/O permission check (including CPL, IOPL, and TSS
I/O
>> bitmap) in x86_emulate() for safety consideration? Thanks! :-)
> 
> Yes. Like I said: the CPL-IOPL check is very cheap, the TSS bitmap check is
a
> little more expensive but probably relatively rare. And in any case the I/O
> port access latency is largely dominated by the VMEXIT/VMENTRY times. Also
the
> devices we emulate are mostly managed by mmio.
I''ll cook up a patch for this myself, by the way.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel