| Message ID | 1460118913-5321-1-git-send-email-rcojocaru@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/04/16 13:35, Razvan Cojocaru wrote: > It is meaningless (and potentially dangerous - see hvmemul_virtual_to_linear()) > to set mem_access_emulate_each_rep before xc_monitor_enable() (which allocates > vcpu->arch.vm_event) has been called, so return an error from the > XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case. > > Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> > > --- > Changes since V1: > - Fixed the if() condition. > - Introduced an rc return variable to simplify the code. > --- > xen/include/asm-x86/monitor.h | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h > index 0954b59..a66760d 100644 > --- a/xen/include/asm-x86/monitor.h > +++ b/xen/include/asm-x86/monitor.h > @@ -32,19 +32,29 @@ > static inline > int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop) > { > + int rc = 0; > + > switch ( mop->op ) > { > case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP: > domain_pause(d); > - d->arch.mem_access_emulate_each_rep = !!mop->event; > + /* > + * Enabling mem_access_emulate_each_rep without a vm_event subscriber > + * is meaningless. > + */ > + if ( d->vcpu && d->vcpu[0]->arch.vm_event ) Sorry - I forgot to mention this before, but this check is slightly buggy. You want if ( d->max_vcpus && d->vcpu[0] && d->vcpu[0]->arch.vm_event ) d->max_vcpus being non-zero guarentees that d->vcpu has been allocated, but you still need to check that d->vcpu[0] has been allocated before following the pointer. With that fixed, Reviewed-by: Andrew Cooper <andrew.cooper3@citirx.com> > + d->arch.mem_access_emulate_each_rep = !!mop->event; > + else > + rc = -EINVAL; > + > domain_unpause(d); > break; > > default: > - return -EOPNOTSUPP; > + rc = -EOPNOTSUPP; > } > > - return 0; > + return rc; > } > > int arch_monitor_domctl_event(struct domain *d,
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h index 0954b59..a66760d 100644 --- a/xen/include/asm-x86/monitor.h +++ b/xen/include/asm-x86/monitor.h @@ -32,19 +32,29 @@ static inline int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop) { + int rc = 0; + switch ( mop->op ) { case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP: domain_pause(d); - d->arch.mem_access_emulate_each_rep = !!mop->event; + /* + * Enabling mem_access_emulate_each_rep without a vm_event subscriber + * is meaningless. + */ + if ( d->vcpu && d->vcpu[0]->arch.vm_event ) + d->arch.mem_access_emulate_each_rep = !!mop->event; + else + rc = -EINVAL; + domain_unpause(d); break; default: - return -EOPNOTSUPP; + rc = -EOPNOTSUPP; } - return 0; + return rc; } int arch_monitor_domctl_event(struct domain *d,
It is meaningless (and potentially dangerous - see hvmemul_virtual_to_linear()) to set mem_access_emulate_each_rep before xc_monitor_enable() (which allocates vcpu->arch.vm_event) has been called, so return an error from the XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case. Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> --- Changes since V1: - Fixed the if() condition. - Introduced an rc return variable to simplify the code. --- xen/include/asm-x86/monitor.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)