Xen devel - Feb 2006 - [PATCH] direct_remap_pfn_range vm_flags fix

direct_remap_pfn_range() does not properly mark vma with VM_PFNMAP.
This triggers improper reference counting on what rmap thought was
a normal page, and a subsequent BUG() such as:

Eeek! page_mapcount(page) went negative! (-1)
  page->flags = 414
  page->count = 1
  page->mapping = 00000000
------------[ cut here ]------------
kernel BUG at
/home/chrisw/hg/xen/xen-unstable/linux-2.6.16-rc2-xen0/mm/rmap.c:555!

Signed-off-by: Chris Wright <chrisw@sous-sol.org>
---

diff -r 859c8d66b203 linux-2.6-xen-sparse/arch/i386/mm/ioremap-xen.c
--- a/linux-2.6-xen-sparse/arch/i386/mm/ioremap-xen.c	Tue Feb  7 20:46:13 2006
+++ b/linux-2.6-xen-sparse/arch/i386/mm/ioremap-xen.c	Tue Feb  7 23:46:21 2006
@@ -120,7 +120,7 @@
 			   domid_t  domid)
 {
 	/* Same as remap_pfn_range(). */
-	vma->vm_flags |= VM_IO | VM_RESERVED;
+	vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP;
 
 	return __direct_remap_pfn_range(
 		vma->vm_mm, address, mfn, size, prot, domid);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 8 Feb 2006, at 08:28, Chris Wright wrote:
> direct_remap_pfn_range() does not properly mark vma with VM_PFNMAP.
> This triggers improper reference counting on what rmap thought was
> a normal page, and a subsequent BUG() such as:
It isn''t really proper for direct_remap_pfn_range() to set VM_PFNMAP. 
Properly that function should actually be called remap_mfn_range(). 
vm_pgoff is an MFN, and what is mapped is a contiguous sequence of 
MFNs.

So the PFNMAP checks in vm_normal_page() do not work, since pte_pfn() 
will not return a contiguous sequence of PFNs starting from vm_pgoff -- 
the MFNs are contiguous, not the PFNs. What saves you currently is the 
cow_mapping() check in vm_normal_page(). If we created private mappings 
of any guest-local memory via direct_remap_pfn_range(), we would still 
crash.

Possible proper fixes:
  1. Don''t map local memory via that interface -- only xenstored does 
this, when mapping dom0''s xenbus page. This could be changed.
  2. Add a _PAGE_DIRECTMAP flag, just for Xen, that indicates 
unrefcounted PTEs.

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

* Keir Fraser (Keir.Fraser@cl.cam.ac.uk) wrote:
> Possible proper fixes:
>  1. Don''t map local memory via that interface -- only xenstored
does
> this, when mapping dom0''s xenbus page. This could be changed.
This sounds preferable.
>  2. Add a _PAGE_DIRECTMAP flag, just for Xen, that indicates 
> unrefcounted PTEs.
Unless you have other users of such a flag?

thanks,
-chris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 8 Feb 2006, at 18:16, Chris Wright wrote:
> * Keir Fraser (Keir.Fraser@cl.cam.ac.uk) wrote:
>> Possible proper fixes:
>>  1. Don''t map local memory via that interface -- only
xenstored does
>> this, when mapping dom0''s xenbus page. This could be changed.
>
> This sounds preferable.
>
>>  2. Add a _PAGE_DIRECTMAP flag, just for Xen, that indicates
>> unrefcounted PTEs.
>
> Unless you have other users of such a flag?
We decided that option (1) was more sane, and that''s now checked in as 
c/s 8783. We''re not proud of using /dev/kmem to map dom0''s
xenstore
page, but that was the only way we could find to use remap_pfn_range() 
and so use a VM_PFNMAP vma for that mapping, as we require. We could 
have made an mmap()able xsd_page device file, but it''s not clear that 
is any more tasteful.

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel