[PATCH] xen: Disable REP MOVSD optimization if introspection is active

[PATCH] xen: Disable REP MOVSD optimization if introspection is active

[Razvan Cojocaru]

REP MOVSD emulation is optimized to write several pages at once
if possible. While this is a very good thing in general, it
can interfere with a memory introspection application's ability
to detect suspect behaviour, since it supresses potentially
telling future page faults. This patch disables the optimization,
gated on introspection being active for the domain.

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
 xen/arch/x86/hvm/emulate.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index c0f47d2..1848a33 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -885,6 +885,7 @@ static int hvmemul_rep_movs(
     p2m_type_t sp2mt, dp2mt;
     int rc, df = !!(ctxt->regs->eflags & X86_EFLAGS_DF);
     char *buf;
+    struct domain *currd = current->domain;
 
     rc = hvmemul_virtual_to_linear(
         src_seg, src_offset, bytes_per_rep, reps, hvm_access_read,
@@ -928,6 +929,10 @@ static int hvmemul_rep_movs(
         return hvmemul_do_mmio(
             dgpa, reps, bytes_per_rep, sgpa, IOREQ_WRITE, df, NULL);
 
+    /* Fall back to slow emulation if introspection is enabled. */
+    if ( currd->arch.hvm_domain.introspection_enabled )
+        return X86EMUL_UNHANDLEABLE;
+
     /* RAM-to-RAM copy: emulate as equivalent of memmove(dgpa, sgpa, bytes). */
     bytes = *reps * bytes_per_rep;
 
-- 
1.7.9.5

Re: [PATCH] xen: Disable REP MOVSD optimization if introspection is active

[Andrew Cooper]

On 27/10/14 10:37, Razvan Cojocaru wrote:
> REP MOVSD emulation is optimized to write several pages at once
> if possible. While this is a very good thing in general, it
> can interfere with a memory introspection application's ability
> to detect suspect behaviour, since it supresses potentially
> telling future page faults. This patch disables the optimization,
> gated on introspection being active for the domain.

An individual rep_movs() call should only optimise to within the current
page, then break back out to the emulator when passing the page boundary.

How does this suppress pagefaults?

~Andrew

Re: [PATCH] xen: Disable REP MOVSD optimization if introspection is active

[Razvan Cojocaru]

On 10/27/2014 12:50 PM, Andrew Cooper wrote:
> On 27/10/14 10:37, Razvan Cojocaru wrote:
>> REP MOVSD emulation is optimized to write several pages at once
>> if possible. While this is a very good thing in general, it
>> can interfere with a memory introspection application's ability
>> to detect suspect behaviour, since it supresses potentially
>> telling future page faults. This patch disables the optimization,
>> gated on introspection being active for the domain.
> 
> An individual rep_movs() call should only optimise to within the current
> page, then break back out to the emulator when passing the page boundary.
> 
> How does this suppress pagefaults?

You're right, sorry for the confusion. Indeed, although we do need (and
have tested) the patch, its description is misleading (and wrong, as
you've rigtly noticed).

Rep_movs() does indeed optimise within the current page, however the
introspection application looks at the EPT mem_event parameters in order
to decide what to do, and the problem is that for the optimized cases
only one mem_event is being sent (for the first MOV) with the initial
gpa and gla. Then no more events are being sent for the rest of the
operation in that page, which is confusing for a client application
expecting to be called for each actual write to a protected page.

If you feel that the patch is acceptable, I'll correct the description
when submitting V2.


Thanks,
Razvan

Re: [PATCH] xen: Disable REP MOVSD optimization if introspection is active

[Andrew Cooper]

On 27/10/14 11:09, Razvan Cojocaru wrote:
> On 10/27/2014 12:50 PM, Andrew Cooper wrote:
>> On 27/10/14 10:37, Razvan Cojocaru wrote:
>>> REP MOVSD emulation is optimized to write several pages at once
>>> if possible. While this is a very good thing in general, it
>>> can interfere with a memory introspection application's ability
>>> to detect suspect behaviour, since it supresses potentially
>>> telling future page faults. This patch disables the optimization,
>>> gated on introspection being active for the domain.
>> An individual rep_movs() call should only optimise to within the current
>> page, then break back out to the emulator when passing the page boundary.
>>
>> How does this suppress pagefaults?
> You're right, sorry for the confusion. Indeed, although we do need (and
> have tested) the patch, its description is misleading (and wrong, as
> you've rigtly noticed).
>
> Rep_movs() does indeed optimise within the current page, however the
> introspection application looks at the EPT mem_event parameters in order
> to decide what to do, and the problem is that for the optimized cases
> only one mem_event is being sent (for the first MOV) with the initial
> gpa and gla. Then no more events are being sent for the rest of the
> operation in that page, which is confusing for a client application
> expecting to be called for each actual write to a protected page.
>
> If you feel that the patch is acceptable, I'll correct the description
> when submitting V2.

I feel that there is a reasonable argument to be made for disabling
optimisations like this in the case that you want mem_events for each step.

~Andrew

Re: [PATCH] xen: Disable REP MOVSD optimization if introspection is active

[Jan Beulich]

>>> On 27.10.14 at 12:09, <rcojocaru@bitdefender.com> wrote:
> If you feel that the patch is acceptable, I'll correct the description
> when submitting V2.

I think it's not unreasonable conceptually, but I somewhat doubt the
place you put the change is the right one. For one it only covers
"rep movs", but not "rep ins" or "rep outs", and then even non-rep
memory access may be broken up (due to the 8 byte limit of the
channel to qemu). For me (lacking sufficient mem-event knowledge)
it is quite hard to tell whether your change perhaps should be near
the actual generation of the mem-event (simply because I can't -
without spending more time than I have for this - right now see
where exactly that event gets generated). Perhaps
hvmemul_virtual_to_linear() should be made force *reps to 1 in
that case instead of making various functions return failure?

Jan

Re: [PATCH] xen: Disable REP MOVSD optimization if introspection is active

[Razvan Cojocaru]

On 10/27/2014 03:01 PM, Jan Beulich wrote:
>>>> On 27.10.14 at 12:09, <rcojocaru@bitdefender.com> wrote:
>> If you feel that the patch is acceptable, I'll correct the description
>> when submitting V2.
> 
> I think it's not unreasonable conceptually, but I somewhat doubt the
> place you put the change is the right one. For one it only covers
> "rep movs", but not "rep ins" or "rep outs", and then even non-rep
> memory access may be broken up (due to the 8 byte limit of the
> channel to qemu). For me (lacking sufficient mem-event knowledge)
> it is quite hard to tell whether your change perhaps should be near
> the actual generation of the mem-event (simply because I can't -
> without spending more time than I have for this - right now see
> where exactly that event gets generated). Perhaps
> hvmemul_virtual_to_linear() should be made force *reps to 1 in
> that case instead of making various functions return failure?

Thank you, that's certainly a more elegant way to handle it than my
hack. I'll test it with reps set to 1 in hvmemul_virtual_to_linear(),
and if all goes well and there are no objections to this way of handling
it, will submit a new patch.


Thanks,
Razvan