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 <[email protected]> >> >> 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 <[email protected]> >> Signed-off-by: Roger Pau Monné <[email protected]> >> Signed-off-by: Andrew Cooper <[email protected]> >> --- >> CC: Jan Beulich <[email protected]> >> CC: Ian Jackson <[email protected]> >> CC: Wei Liu <[email protected]> >> CC: Roger Pau Monné <[email protected]> >> CC: Sergey Dyasli <[email protected]> >> CC: Daniel De Graaf <[email protected]> >> >> 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 [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
