| Message ID | 592E899A020000780015DF82@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 31/05/17 08:15, Jan Beulich wrote: > The flag is really only meant for those, both HVM and 32-bit PV tell > kernel from user mode based on CPL/RPL. Remove the all-question-marks > comment and let's be on the safe side here and also suppress clearing > for 32-bit PV (this isn't a fast path after all). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Wouldn't it just be safer to disallow starting a 64bit PV guest in user mode? No real kernel would do such a thing, and keeping the corner case around is bad from an attack-surface point of view. ~Andrew
>>> On 31.05.17 at 13:08, <andrew.cooper3@citrix.com> wrote: > On 31/05/17 08:15, Jan Beulich wrote: >> The flag is really only meant for those, both HVM and 32-bit PV tell >> kernel from user mode based on CPL/RPL. Remove the all-question-marks >> comment and let's be on the safe side here and also suppress clearing >> for 32-bit PV (this isn't a fast path after all). >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Wouldn't it just be safer to disallow starting a 64bit PV guest in user > mode? > > No real kernel would do such a thing, and keeping the corner case around > is bad from an attack-surface point of view. If it really was "starting a guest", I would probably agree. But we're talking about starting a vCPU, and I could see uses for this (not the least in XTF). After all the operation allows for enough state to be set up such that further initialization inside the guest may not be necessary. Jan
>>> On 31.05.17 at 13:54, wrote: >>>> On 31.05.17 at 13:08, <andrew.cooper3@citrix.com> wrote: > > On 31/05/17 08:15, Jan Beulich wrote: > >> The flag is really only meant for those, both HVM and 32-bit PV tell > >> kernel from user mode based on CPL/RPL. Remove the all-question-marks > >> comment and let's be on the safe side here and also suppress clearing > >> for 32-bit PV (this isn't a fast path after all). > >> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > > > Wouldn't it just be safer to disallow starting a 64bit PV guest in user > > mode? > > > > No real kernel would do such a thing, and keeping the corner case around > > is bad from an attack-surface point of view. > > If it really was "starting a guest", I would probably agree. But we're > talking about starting a vCPU, and I could see uses for this (not the > least in XTF). After all the operation allows for enough state to be > set up such that further initialization inside the guest may not be > necessary. Any opinion here, or change of opinion on the original patch? Jan
--- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -950,9 +950,15 @@ int arch_set_info_guest( v->fpu_initialised = !!(flags & VGCF_I387_VALID); - v->arch.flags &= ~TF_kernel_mode; - if ( (flags & VGCF_in_kernel) || is_hvm_domain(d)/*???*/ ) - v->arch.flags |= TF_kernel_mode; + v->arch.flags |= TF_kernel_mode; + if ( unlikely(!(flags & VGCF_in_kernel)) && + /* + * TF_kernel_mode is only allowed to be clear for 64-bit PV. See + * update_cr3(), sh_update_cr3(), and shadow_one_bit_disable() for + * why that is. + */ + !is_hvm_domain(d) && !is_pv_32bit_domain(d) ) + v->arch.flags &= ~TF_kernel_mode; v->arch.vgc_flags = flags;