On 04/07/18 11:18, Wei Liu wrote:
> On Tue, Jul 03, 2018 at 09:55:26PM +0100, Andrew Cooper wrote:
>> From: Sergey Dyasli <sergey.dya...@citrix.com>
>>
>> This hypercall allows the toolstack to present one combined CPUID and MSR
>> policy for a domain, which can be audited in one go by Xen, which is
>> necessary
>> for correctness of the auditing.
>>
>> A stub x86_policies_are_compatible() function is introduced, although at
>> present it will always fail the hypercall.
>>
>> The hypercall ABI allows for update of individual CPUID or MSR entries, so
>> begins by duplicating the existing policy (for which a helper is introduced),
>> merging the toolstack data, then checking compatibility of the result.
>>
>> The system PV/HVM max policy is used for the compatiblity check.
>>
>> Signed-off-by: Sergey Dyasli <sergey.dya...@citrix.com>
>> Signed-off-by: Roger Pau Monné <roger....@citrix.com>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>> ---
>> CC: Jan Beulich <jbeul...@suse.com>
>> CC: Ian Jackson <ian.jack...@eu.citrix.com>
>> CC: Wei Liu <wei.l...@citrix.com>
>> CC: Roger Pau Monné <roger....@citrix.com>
>> CC: Sergey Dyasli <sergey.dya...@citrix.com>
>> CC: Daniel De Graaf <dgde...@tycho.nsa.gov>
>>
>> One awkard corner case is re-deserialising of the vcpu msrs. The correct fix
>> would be to allocate a buffer, copy the MSRs list, then deserialise from
>> that,
>> but trips the bounds checks in the copy_from_guest() helpers. The compat
>> XLAT
>> are would work, but would require that we allocate it even for 64bit PV
>> guests.
> I'm not sure I follow this. The issue isn't obvious from looking at the
> code.
>
>> ---
>> + /* Merge the (now audited) vCPU MSRs into every other msr_vcpu_policy.
>> */
>> + for ( ; v; v = v->next_in_list )
>> + {
>> + /* XXX - Figure out how to avoid a TOCTOU race here. XLAT area? */
> What is the TOCTOU race here?
In the lines you snipped...
>
>> + if ( (ret = x86_msr_copy_from_buffer(
>> + NULL, v->arch.msr, xdpc->msr_policy, xdpc->nr_msrs,
>> NULL)) )
>
xdpc->msr_policy is a pointer into toolstack userspace, which we re-read.
The problem is that we can't copy this up front to a hypervisor buffer
(which would avoid the TOCTOU race), without tripping the access_ok()
check in copy_from_guest().
However, Jan and Sergey (IRL) have suggested an alternative approach
which will work for now.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
