[PATCH] gfn_to_page() acquires mmap_sem twice

[PATCH] gfn_to_page() acquires mmap_sem twice

[Anthony Liguori]

KVM's nopage handler calls gfn_to_page() which acquires the mmap_sem when
calling out to get_user_pages().  nopage handlers are already invoked with the
mmap_sem held though.  Introduce a __gfn_to_page() for use by the nopage
handler which requires the lock to already be held.

This was noticed by tglx.

Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 9a3d663..5ce0754 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -1028,7 +1028,10 @@ int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn)
 }
 EXPORT_SYMBOL_GPL(kvm_is_visible_gfn);
 
-struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
+/*
+ * Requires current->mm->mmap_sem to be held
+ */
+static struct page *__gfn_to_page(struct kvm *kvm, gfn_t gfn)
 {
 	struct kvm_memory_slot *slot;
 	struct page *page[1];
@@ -1043,12 +1046,10 @@ struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
 		return bad_page;
 	}
 
-	down_read(&current->mm->mmap_sem);
 	npages = get_user_pages(current, current->mm,
 				slot->userspace_addr
 				+ (gfn - slot->base_gfn) * PAGE_SIZE, 1,
 				1, 0, page, NULL);
-	up_read(&current->mm->mmap_sem);
 	if (npages != 1) {
 		get_page(bad_page);
 		return bad_page;
@@ -1056,6 +1057,18 @@ struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
 
 	return page[0];
 }
+
+struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
+{
+	struct page *page;
+
+	down_read(&current->mm->mmap_sem);
+	page = __gfn_to_page(kvm, gfn);
+	up_read(&current->mm->mmap_sem);
+
+	return page;
+}
+
 EXPORT_SYMBOL_GPL(gfn_to_page);
 
 void kvm_release_page(struct page *page)
@@ -3130,7 +3143,8 @@ static struct page *kvm_vm_nopage(struct vm_area_struct *vma,
 	pgoff = ((address - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
 	if (!kvm_is_visible_gfn(kvm, pgoff))
 		return NOPAGE_SIGBUS;
-	page = gfn_to_page(kvm, pgoff);
+	/* current->mm->mmap_sem is already held so call lockless version */
+	page = __gfn_to_page(kvm, pgoff);
 	if (is_error_page(page)) {
 		kvm_release_page(page);
 		return NOPAGE_SIGBUS;

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH] gfn_to_page() acquires mmap_sem twice

[Izik Eidus]

Anthony Liguori wrote:
> KVM's nopage handler calls gfn_to_page() which acquires the mmap_sem when
> calling out to get_user_pages().  nopage handlers are already invoked with the
> mmap_sem held though.  Introduce a __gfn_to_page() for use by the nopage
> handler which requires the lock to already be held.
>
> This was noticed by tglx.
>
> Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>
>   
look good, thanks

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH] gfn_to_page() acquires mmap_sem twice

[Avi Kivity]

Anthony Liguori wrote:
> KVM's nopage handler calls gfn_to_page() which acquires the mmap_sem when
> calling out to get_user_pages().  nopage handlers are already invoked with the
> mmap_sem held though.  Introduce a __gfn_to_page() for use by the nopage
> handler which requires the lock to already be held.
>
> This was noticed by tglx.
>   

Applied, thanks.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/