Re: [Xen-devel] [PATCH for-4.12 V3] x86/altp2m: fix HVMOP_altp2m_set_domain_state race

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.02.19 at 11:11, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 2/11/19 6:59 PM, Jan Beulich wrote:
>> Plus I can't see p2m_switch_vcpu_altp2m_by_id() called for
>> any HVMOP_altp2m_* at all. One of the actual callers is guarded
>> by altp2m_active(), but the other isn't.
> 
> Actually I see that both places are guarded by altp2m_active().
> 
> In p2m.c we have:
> 
> 2312 void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> 2313 {
> 2314     if ( altp2m_active(v->domain) )
> 2315         p2m_switch_vcpu_altp2m_by_id(v, idx);
> 2316 }
> 
> and in vmx.c:
> 
> 2225 static int vmx_vcpu_emulate_vmfunc(const struct cpu_user_regs *regs)
> 2226 {
> 2227     int rc = X86EMUL_EXCEPTION;
> 2228     struct vcpu *curr = current;
> 2229
> 2230     if ( !cpu_has_vmx_vmfunc && altp2m_active(curr->domain) &&
> 2231          regs->eax == 0 &&
> 2232          p2m_switch_vcpu_altp2m_by_id(curr, regs->ecx) )
> 2233         rc = X86EMUL_OKAY;
> 2234
> 2235     return rc;
> 2236 }
> 
> here there's an "&& altp2m_active(curr->domain)" in the if().

Oh, so I must have overlooked one of the two, sorry.

> So I suppose in our scenario all that's needed it a similar check here:
> 
> 4636     case HVMOP_altp2m_switch_p2m:
> 4637         rc = p2m_switch_domain_altp2m_by_id(d, a.u.view.view);
> 4638         break;
> 
> for the other function, p2m_switch_domain_altp2m_by_id().
> 
> Unless I'm missing something.

Perhaps. Question is whether outside of your scenario similar checks
are missing elsewhere.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel