Re: [Xen-devel] [PATCH v2 3/4] x86/vmx: Fix security issue when a guest balloons out the #VE info page

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/02/2019 12:24, Jan Beulich wrote:
>>>> On 21.02.19 at 21:18, <andrew.cooper3@xxxxxxxxxx> wrote:
>> The logic in altp2m_vcpu_{en,dis}able_ve() and vmx_vcpu_update_vmfunc_ve() is
>> dangerous.  After #VE has been set up, the guest can balloon out and free the
>> nominated GFN, after which the processor may write to it.  Also, the unlocked
>> GFN query means the MFN is stale by the time it is used.  Alternatively, a
>> guest can race two disable calls to cause one VMCS to still reference the
>> nominated GFN after the tracking information was dropped.
>>
>> Rework the logic from scratch to make it safe.
>>
>> Hold an extra page reference on the underlying frame, to account for the
>> VMCS's reference.  This means that if the GFN gets ballooned out, it isn't
>> freed back to Xen until #VE is disabled, and the VMCS no longer refers to the
>> page.
>>
>> A consequence of this is that altp2m_vcpu_disable_ve() needs to be called
>> during the domain_kill() path, to drop the reference for domains which shut
>> down with #VE still enabled.
>>
>> For domains using altp2m, we expect a single enable call and no disable for
>> the remaining lifetime of the domain.  However, to avoid problems with
>> concurrent calls, use cmpxchg() to locklessly maintain safety.
>>
>> This doesn't have an XSA because altp2m is not yet a security-supported
>> feature.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Release-acked-by: Juergen Gross <jgross@xxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> I would appreciate though if you could add half a sentence clarifying
> that and why you decided against obtaining a writable type-ref.
>
> Also - I take it that altp2m and migration don't work together?
> Otherwise wouldn't you need to mark the page dirty in more
> cases?

Noone ever considers migration, and it definitely doesn't work as a
self-contained action.  For one, nothing preserves the already
established mem_access / alternatives information.

For general logdirty, you will recall that Razvan spent a long time
fixing that in Xen 4.12.

The #VE-info page itself can in principle be tracked with Page
Modification Logging, but PML isn't available because EPT A/D is
mutually exclusive with introspection in current processors.  Enabling
EPT A/D causes all guest pagetable accesses to be treated as writes even
when they aren't setting guest A/D bits, meaning that if you try and
write protect a pagetable, you livelock taking EPT_VIOLATIONs when
trying to perform a TLB Fill.

However, what happens in practice is that you can't safely migrate VM's
with an attached vmevent ring as part of the p2m (as the guest kernel
can interfere), and you might notice that there is ongoing effort to use
resource mapping instead.  IIRC Introspection detaches in the post-pause
phase of migrate, and (by design) is capable of re-initialising all
protections from first principles at the destination (so individual
introspection agents on each server don't need to communicate state).

So overall, migration does work at least in the altp2m_external case,
but only due to a fair amount of heavy lifting in the toolstack

~Andrew

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