* [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY @ 2025-06-12 4:49 Paolo Bonzini 2025-06-12 5:29 ` Xiaoyao Li 2025-06-13 3:00 ` Yan Zhao 0 siblings, 2 replies; 7+ messages in thread From: Paolo Bonzini @ 2025-06-12 4:49 UTC (permalink / raw) To: linux-kernel, kvm; +Cc: seanjc, xiaoyao.li, yan.y.zhao Only let userspace pass the same addresses that were used in KVM_SET_USER_MEMORY_REGION (or KVM_SET_USER_MEMORY_REGION2); gpas in the the upper half of the address space are an implementation detail of TDX and KVM. Extracted from a patch by Sean Christopherson <seanjc@google.com>. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/kvm/mmu/mmu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index a4040578b537..4e06e2e89a8f 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4903,6 +4903,9 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, if (!vcpu->kvm->arch.pre_fault_allowed) return -EOPNOTSUPP; + if (kvm_is_gfn_alias(vcpu->kvm, gpa_to_gfn(range->gpa))) + return -EINVAL; + /* * reload is efficient when called repeatedly, so we can do it on * every iteration. -- 2.43.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY 2025-06-12 4:49 [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY Paolo Bonzini @ 2025-06-12 5:29 ` Xiaoyao Li 2025-06-12 5:34 ` Paolo Bonzini 2025-06-13 3:00 ` Yan Zhao 1 sibling, 1 reply; 7+ messages in thread From: Xiaoyao Li @ 2025-06-12 5:29 UTC (permalink / raw) To: Paolo Bonzini, linux-kernel, kvm; +Cc: seanjc, yan.y.zhao On 6/12/2025 12:49 PM, Paolo Bonzini wrote: > Only let userspace pass the same addresses that were used in KVM_SET_USER_MEMORY_REGION > (or KVM_SET_USER_MEMORY_REGION2); gpas in the the upper half of the address space > are an implementation detail of TDX and KVM. > > Extracted from a patch by Sean Christopherson <seanjc@google.com>. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/kvm/mmu/mmu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index a4040578b537..4e06e2e89a8f 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -4903,6 +4903,9 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > if (!vcpu->kvm->arch.pre_fault_allowed) > return -EOPNOTSUPP; > > + if (kvm_is_gfn_alias(vcpu->kvm, gpa_to_gfn(range->gpa))) > + return -EINVAL; Do we need to worry about the case (range->gpa + range->size) becomes alias? > /* > * reload is efficient when called repeatedly, so we can do it on > * every iteration. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY 2025-06-12 5:29 ` Xiaoyao Li @ 2025-06-12 5:34 ` Paolo Bonzini 2025-06-12 5:41 ` Xiaoyao Li 0 siblings, 1 reply; 7+ messages in thread From: Paolo Bonzini @ 2025-06-12 5:34 UTC (permalink / raw) To: Xiaoyao Li; +Cc: linux-kernel, kvm, seanjc, yan.y.zhao On Thu, Jun 12, 2025 at 7:29 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote: > > On 6/12/2025 12:49 PM, Paolo Bonzini wrote: > > Only let userspace pass the same addresses that were used in KVM_SET_USER_MEMORY_REGION > > (or KVM_SET_USER_MEMORY_REGION2); gpas in the the upper half of the address space > > are an implementation detail of TDX and KVM. > > > > Extracted from a patch by Sean Christopherson <seanjc@google.com>. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > arch/x86/kvm/mmu/mmu.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > index a4040578b537..4e06e2e89a8f 100644 > > --- a/arch/x86/kvm/mmu/mmu.c > > +++ b/arch/x86/kvm/mmu/mmu.c > > @@ -4903,6 +4903,9 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > > if (!vcpu->kvm->arch.pre_fault_allowed) > > return -EOPNOTSUPP; > > > > + if (kvm_is_gfn_alias(vcpu->kvm, gpa_to_gfn(range->gpa))) > > + return -EINVAL; > > Do we need to worry about the case (range->gpa + range->size) becomes alias? No, because the function only processes a single page and everything in the non-aliased part of the address space *can* be prefaulted. KVM's generic kvm_vcpu_pre_fault_memory() call will see the EINVAL on a later invocation and will stop processing the part of the request that is has the shared/direct bit set. Paolo > > > /* > > * reload is efficient when called repeatedly, so we can do it on > > * every iteration. > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY 2025-06-12 5:34 ` Paolo Bonzini @ 2025-06-12 5:41 ` Xiaoyao Li 0 siblings, 0 replies; 7+ messages in thread From: Xiaoyao Li @ 2025-06-12 5:41 UTC (permalink / raw) To: Paolo Bonzini; +Cc: linux-kernel, kvm, seanjc, yan.y.zhao On 6/12/2025 1:34 PM, Paolo Bonzini wrote: > On Thu, Jun 12, 2025 at 7:29 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote: >> >> On 6/12/2025 12:49 PM, Paolo Bonzini wrote: >>> Only let userspace pass the same addresses that were used in KVM_SET_USER_MEMORY_REGION >>> (or KVM_SET_USER_MEMORY_REGION2); gpas in the the upper half of the address space >>> are an implementation detail of TDX and KVM. >>> >>> Extracted from a patch by Sean Christopherson <seanjc@google.com>. >>> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> --- >>> arch/x86/kvm/mmu/mmu.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c >>> index a4040578b537..4e06e2e89a8f 100644 >>> --- a/arch/x86/kvm/mmu/mmu.c >>> +++ b/arch/x86/kvm/mmu/mmu.c >>> @@ -4903,6 +4903,9 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, >>> if (!vcpu->kvm->arch.pre_fault_allowed) >>> return -EOPNOTSUPP; >>> >>> + if (kvm_is_gfn_alias(vcpu->kvm, gpa_to_gfn(range->gpa))) >>> + return -EINVAL; >> >> Do we need to worry about the case (range->gpa + range->size) becomes alias? > > No, because the function only processes a single page and everything > in the non-aliased part of the address space *can* be prefaulted. > > KVM's generic kvm_vcpu_pre_fault_memory() call will see the EINVAL on > a later invocation and will stop processing the part of the request > that is has the shared/direct bit set. reasonable.. Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com> > Paolo > >> >>> /* >>> * reload is efficient when called repeatedly, so we can do it on >>> * every iteration. >> > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY 2025-06-12 4:49 [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY Paolo Bonzini 2025-06-12 5:29 ` Xiaoyao Li @ 2025-06-13 3:00 ` Yan Zhao 2025-06-13 20:16 ` Sean Christopherson 1 sibling, 1 reply; 7+ messages in thread From: Yan Zhao @ 2025-06-13 3:00 UTC (permalink / raw) To: Paolo Bonzini; +Cc: linux-kernel, kvm, seanjc, xiaoyao.li On Thu, Jun 12, 2025 at 12:49:43AM -0400, Paolo Bonzini wrote: > Only let userspace pass the same addresses that were used in KVM_SET_USER_MEMORY_REGION > (or KVM_SET_USER_MEMORY_REGION2); gpas in the the upper half of the address space > are an implementation detail of TDX and KVM. > > Extracted from a patch by Sean Christopherson <seanjc@google.com>. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/kvm/mmu/mmu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index a4040578b537..4e06e2e89a8f 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -4903,6 +4903,9 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > if (!vcpu->kvm->arch.pre_fault_allowed) > return -EOPNOTSUPP; > > + if (kvm_is_gfn_alias(vcpu->kvm, gpa_to_gfn(range->gpa))) > + return -EINVAL; > + Do we need the same check in kvm_vm_ioctl_set_mem_attributes()? > /* > * reload is efficient when called repeatedly, so we can do it on > * every iteration. > -- > 2.43.5 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY 2025-06-13 3:00 ` Yan Zhao @ 2025-06-13 20:16 ` Sean Christopherson 2025-06-16 2:04 ` Yan Zhao 0 siblings, 1 reply; 7+ messages in thread From: Sean Christopherson @ 2025-06-13 20:16 UTC (permalink / raw) To: Yan Zhao; +Cc: Paolo Bonzini, linux-kernel, kvm, xiaoyao.li On Fri, Jun 13, 2025, Yan Zhao wrote: > On Thu, Jun 12, 2025 at 12:49:43AM -0400, Paolo Bonzini wrote: > > Only let userspace pass the same addresses that were used in KVM_SET_USER_MEMORY_REGION > > (or KVM_SET_USER_MEMORY_REGION2); gpas in the the upper half of the address space > > are an implementation detail of TDX and KVM. > > > > Extracted from a patch by Sean Christopherson <seanjc@google.com>. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > arch/x86/kvm/mmu/mmu.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > index a4040578b537..4e06e2e89a8f 100644 > > --- a/arch/x86/kvm/mmu/mmu.c > > +++ b/arch/x86/kvm/mmu/mmu.c > > @@ -4903,6 +4903,9 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > > if (!vcpu->kvm->arch.pre_fault_allowed) > > return -EOPNOTSUPP; > > > > + if (kvm_is_gfn_alias(vcpu->kvm, gpa_to_gfn(range->gpa))) > > + return -EINVAL; > > + > Do we need the same check in kvm_vm_ioctl_set_mem_attributes()? Yeah, we probably should disallow aliases there. It should be benign? Because memslots disallow aliases, and so the aliased gfn entries in the xarray will never actually be consumed. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY 2025-06-13 20:16 ` Sean Christopherson @ 2025-06-16 2:04 ` Yan Zhao 0 siblings, 0 replies; 7+ messages in thread From: Yan Zhao @ 2025-06-16 2:04 UTC (permalink / raw) To: Sean Christopherson; +Cc: Paolo Bonzini, linux-kernel, kvm, xiaoyao.li On Fri, Jun 13, 2025 at 01:16:38PM -0700, Sean Christopherson wrote: > On Fri, Jun 13, 2025, Yan Zhao wrote: > > On Thu, Jun 12, 2025 at 12:49:43AM -0400, Paolo Bonzini wrote: > > > Only let userspace pass the same addresses that were used in KVM_SET_USER_MEMORY_REGION > > > (or KVM_SET_USER_MEMORY_REGION2); gpas in the the upper half of the address space > > > are an implementation detail of TDX and KVM. > > > > > > Extracted from a patch by Sean Christopherson <seanjc@google.com>. > > > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > > --- > > > arch/x86/kvm/mmu/mmu.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > > index a4040578b537..4e06e2e89a8f 100644 > > > --- a/arch/x86/kvm/mmu/mmu.c > > > +++ b/arch/x86/kvm/mmu/mmu.c > > > @@ -4903,6 +4903,9 @@ long kvm_arch_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > > > if (!vcpu->kvm->arch.pre_fault_allowed) > > > return -EOPNOTSUPP; > > > > > > + if (kvm_is_gfn_alias(vcpu->kvm, gpa_to_gfn(range->gpa))) > > > + return -EINVAL; > > > + > > Do we need the same check in kvm_vm_ioctl_set_mem_attributes()? > > Yeah, we probably should disallow aliases there. It should be benign? Because > memslots disallow aliases, and so the aliased gfn entries in the xarray will never > actually be consumed. Yes, it's benign after this patch. Userspace may find that setting attribute for an aliased gfn has no effect. Though there's a "kvm_mem_is_private(vcpu->kvm, gpa_to_gfn(cr2_or_gpa)" in kvm_mmu_page_fault(), it's only for KVM_X86_SW_PROTECTED_VM. So it's benign, just odd :) ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-06-16 2:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-12 4:49 [PATCH] KVM: x86/mmu: Reject direct bits in gpa passed to KVM_PRE_FAULT_MEMORY Paolo Bonzini 2025-06-12 5:29 ` Xiaoyao Li 2025-06-12 5:34 ` Paolo Bonzini 2025-06-12 5:41 ` Xiaoyao Li 2025-06-13 3:00 ` Yan Zhao 2025-06-13 20:16 ` Sean Christopherson 2025-06-16 2:04 ` Yan Zhao
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.