* [PATCH 0/2] KVM: SVM: convert get_user_pages() --> pin_user_pages(), bug fixes @ 2020-05-26 6:22 John Hubbard 2020-05-26 6:22 ` [PATCH 1/2] KVM: SVM: fix svn_pin_memory()'s use of get_user_pages_fast() John Hubbard 2020-05-26 6:22 ` [PATCH 2/2] KVM: SVM: convert get_user_pages() --> pin_user_pages() John Hubbard 0 siblings, 2 replies; 5+ messages in thread From: John Hubbard @ 2020-05-26 6:22 UTC (permalink / raw) To: LKML Cc: Souptick Joarder, John Hubbard, Ingo Molnar, Borislav Petkov, Thomas Gleixner, Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, H . Peter Anvin, x86, kvm Hi, This is just for the SEV (Secure Encrypted Virtualization) part of KVM. It converts the get_user_pages_fast() call, after fixing a couple of small bugs in the vicinity. Note that I have only compile-tested these two patches, so any run-time testing coverage would be greatly appreciated. Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <sean.j.christopherson@intel.com> Cc: Vitaly Kuznetsov <vkuznets@redhat.com> Cc: Wanpeng Li <wanpengli@tencent.com> Cc: Jim Mattson <jmattson@google.com> Cc: Joerg Roedel <joro@8bytes.org> Cc: H. Peter Anvin <hpa@zytor.com> Cc: x86@kernel.org Cc: kvm@vger.kernel.org John Hubbard (2): KVM: SVM: fix svn_pin_memory()'s use of get_user_pages_fast() KVM: SVM: convert get_user_pages() --> pin_user_pages() arch/x86/kvm/svm/sev.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) base-commit: 9cb1fd0efd195590b828b9b865421ad345a4a145 -- 2.26.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] KVM: SVM: fix svn_pin_memory()'s use of get_user_pages_fast() 2020-05-26 6:22 [PATCH 0/2] KVM: SVM: convert get_user_pages() --> pin_user_pages(), bug fixes John Hubbard @ 2020-05-26 6:22 ` John Hubbard 2020-05-26 7:33 ` Vitaly Kuznetsov 2020-05-26 6:22 ` [PATCH 2/2] KVM: SVM: convert get_user_pages() --> pin_user_pages() John Hubbard 1 sibling, 1 reply; 5+ messages in thread From: John Hubbard @ 2020-05-26 6:22 UTC (permalink / raw) To: LKML Cc: Souptick Joarder, John Hubbard, Ingo Molnar, Borislav Petkov, Thomas Gleixner, Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, H . Peter Anvin, x86, kvm There are two problems in svn_pin_memory(): 1) The return value of get_user_pages_fast() is stored in an unsigned long, although the declared return value is of type int. This will not cause any symptoms, but it is misleading. Fix this by changing the type of npinned to "int". 2) The number of pages passed into get_user_pages_fast() is stored in an unsigned long, even though get_user_pages_fast() accepts an int. This means that it is possible to silently overflow the number of pages. Fix this by adding a WARN_ON_ONCE() and an early error return. The npages variable is left as an unsigned long for convenience in checking for overflow. Fixes: 89c505809052 ("KVM: SVM: Add support for KVM_SEV_LAUNCH_UPDATE_DATA command") Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <sean.j.christopherson@intel.com> Cc: Vitaly Kuznetsov <vkuznets@redhat.com> Cc: Wanpeng Li <wanpengli@tencent.com> Cc: Jim Mattson <jmattson@google.com> Cc: Joerg Roedel <joro@8bytes.org> Cc: H. Peter Anvin <hpa@zytor.com> Cc: x86@kernel.org Cc: kvm@vger.kernel.org Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- arch/x86/kvm/svm/sev.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 89f7f3aebd31..9693db1af57c 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -313,7 +313,8 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, int write) { struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; - unsigned long npages, npinned, size; + unsigned long npages, size; + int npinned; unsigned long locked, lock_limit; struct page **pages; unsigned long first, last; @@ -333,6 +334,9 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, return NULL; } + if (WARN_ON_ONCE(npages > INT_MAX)) + return NULL; + /* Avoid using vmalloc for smaller buffers. */ size = npages * sizeof(struct page *); if (size > PAGE_SIZE) -- 2.26.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] KVM: SVM: fix svn_pin_memory()'s use of get_user_pages_fast() 2020-05-26 6:22 ` [PATCH 1/2] KVM: SVM: fix svn_pin_memory()'s use of get_user_pages_fast() John Hubbard @ 2020-05-26 7:33 ` Vitaly Kuznetsov 2020-06-30 3:10 ` John Hubbard 0 siblings, 1 reply; 5+ messages in thread From: Vitaly Kuznetsov @ 2020-05-26 7:33 UTC (permalink / raw) To: John Hubbard, LKML Cc: Souptick Joarder, John Hubbard, Ingo Molnar, Borislav Petkov, Thomas Gleixner, Paolo Bonzini, Sean Christopherson, Wanpeng Li, Jim Mattson, Joerg Roedel, H . Peter Anvin, x86, kvm John Hubbard <jhubbard@nvidia.com> writes: > There are two problems in svn_pin_memory(): > > 1) The return value of get_user_pages_fast() is stored in an > unsigned long, although the declared return value is of type int. > This will not cause any symptoms, but it is misleading. > Fix this by changing the type of npinned to "int". > > 2) The number of pages passed into get_user_pages_fast() is stored > in an unsigned long, even though get_user_pages_fast() accepts an > int. This means that it is possible to silently overflow the number > of pages. > > Fix this by adding a WARN_ON_ONCE() and an early error return. The > npages variable is left as an unsigned long for convenience in > checking for overflow. > > Fixes: 89c505809052 ("KVM: SVM: Add support for KVM_SEV_LAUNCH_UPDATE_DATA command") > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Cc: Wanpeng Li <wanpengli@tencent.com> > Cc: Jim Mattson <jmattson@google.com> > Cc: Joerg Roedel <joro@8bytes.org> > Cc: H. Peter Anvin <hpa@zytor.com> > Cc: x86@kernel.org > Cc: kvm@vger.kernel.org > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > arch/x86/kvm/svm/sev.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 89f7f3aebd31..9693db1af57c 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -313,7 +313,8 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, > int write) > { > struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > - unsigned long npages, npinned, size; > + unsigned long npages, size; > + int npinned; > unsigned long locked, lock_limit; > struct page **pages; > unsigned long first, last; > @@ -333,6 +334,9 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, > return NULL; > } > > + if (WARN_ON_ONCE(npages > INT_MAX)) > + return NULL; > + I bit unrelated to this patch, but callers of sev_pin_memory() treat NULL differently: sev_launch_secret()/svm_register_enc_region() return -ENOMEM sev_dbg_crypt() returns -EFAULT Should we switch to ERR_PTR() to preserve the error? > /* Avoid using vmalloc for smaller buffers. */ > size = npages * sizeof(struct page *); > if (size > PAGE_SIZE) Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> -- Vitaly ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] KVM: SVM: fix svn_pin_memory()'s use of get_user_pages_fast() 2020-05-26 7:33 ` Vitaly Kuznetsov @ 2020-06-30 3:10 ` John Hubbard 0 siblings, 0 replies; 5+ messages in thread From: John Hubbard @ 2020-06-30 3:10 UTC (permalink / raw) To: Vitaly Kuznetsov, LKML Cc: Souptick Joarder, Ingo Molnar, Borislav Petkov, Thomas Gleixner, Paolo Bonzini, Sean Christopherson, Wanpeng Li, Jim Mattson, Joerg Roedel, H . Peter Anvin, x86, kvm On 2020-05-26 00:33, Vitaly Kuznetsov wrote: ... > I bit unrelated to this patch, but callers of sev_pin_memory() treat > NULL differently: > > sev_launch_secret()/svm_register_enc_region() return -ENOMEM > sev_dbg_crypt() returns -EFAULT > > Should we switch to ERR_PTR() to preserve the error? > >> /* Avoid using vmalloc for smaller buffers. */ >> size = npages * sizeof(struct page *); >> if (size > PAGE_SIZE) > > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > Thanks for the review, Vitaly. If anyone is able to do any run-time testing of this patch and also patch 2/2, then I think a maintainer would be more willing to pick them up. Any testing help there is greatly appreciated! thanks, -- John Hubbard NVIDIA ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] KVM: SVM: convert get_user_pages() --> pin_user_pages() 2020-05-26 6:22 [PATCH 0/2] KVM: SVM: convert get_user_pages() --> pin_user_pages(), bug fixes John Hubbard 2020-05-26 6:22 ` [PATCH 1/2] KVM: SVM: fix svn_pin_memory()'s use of get_user_pages_fast() John Hubbard @ 2020-05-26 6:22 ` John Hubbard 1 sibling, 0 replies; 5+ messages in thread From: John Hubbard @ 2020-05-26 6:22 UTC (permalink / raw) To: LKML Cc: Souptick Joarder, John Hubbard, Ingo Molnar, Borislav Petkov, Thomas Gleixner, Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, H . Peter Anvin, x86, kvm This code was using get_user_pages*(), in a "Case 2" scenario (DMA/RDMA), using the categorization from [1]. That means that it's time to convert the get_user_pages*() + put_page() calls to pin_user_pages*() + unpin_user_pages() calls. There is some helpful background in [2]: basically, this is a small part of fixing a long-standing disconnect between pinning pages, and file systems' use of those pages. [1] Documentation/core-api/pin_user_pages.rst [2] "Explicit pinning of user-space pages": https://lwn.net/Articles/807108/ Cc: Ingo Molnar <mingo@redhat.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <sean.j.christopherson@intel.com> Cc: Vitaly Kuznetsov <vkuznets@redhat.com> Cc: Wanpeng Li <wanpengli@tencent.com> Cc: Jim Mattson <jmattson@google.com> Cc: Joerg Roedel <joro@8bytes.org> Cc: H. Peter Anvin <hpa@zytor.com> Cc: x86@kernel.org Cc: kvm@vger.kernel.org Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- arch/x86/kvm/svm/sev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 9693db1af57c..a83f2e73bcbb 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -349,7 +349,7 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, return NULL; /* Pin the user virtual address. */ - npinned = get_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : 0, pages); + npinned = pin_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : 0, pages); if (npinned != npages) { pr_err("SEV: Failure locking %lu pages.\n", npages); goto err; @@ -362,7 +362,7 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, err: if (npinned > 0) - release_pages(pages, npinned); + unpin_user_pages(pages, npinned); kvfree(pages); return NULL; @@ -373,7 +373,7 @@ static void sev_unpin_memory(struct kvm *kvm, struct page **pages, { struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; - release_pages(pages, npages); + unpin_user_pages(pages, npages); kvfree(pages); sev->pages_locked -= npages; } -- 2.26.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-30 3:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-26 6:22 [PATCH 0/2] KVM: SVM: convert get_user_pages() --> pin_user_pages(), bug fixes John Hubbard 2020-05-26 6:22 ` [PATCH 1/2] KVM: SVM: fix svn_pin_memory()'s use of get_user_pages_fast() John Hubbard 2020-05-26 7:33 ` Vitaly Kuznetsov 2020-06-30 3:10 ` John Hubbard 2020-05-26 6:22 ` [PATCH 2/2] KVM: SVM: convert get_user_pages() --> pin_user_pages() John Hubbard
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.