Re: [Xen-devel] The hypercall will fail and return EFAULT when the page becomes COW by forking process in linux

[Wangzhenguo <wangzhenguo@xxxxxxxxxx>]

> From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> 
> It would be good to write this in a comment next to each of the
> xc_{interface,evtchn,gnttab,gntshr}_open() prototypes in the header
> (assuming it applies to all of them, since they all make hypercalls I
> expect it does and in any case it is easy to relax this restriction in
> the future if not)
> 
> Otherwise:
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

I added a comment and your acked-by in patch 

The patch is

# HG changeset patch
# Parent a5dfd924fcdb173a154dad9f37073c1de1302065
libxc: Add VM_DONTCOPY flag of the VMA of the hypercall buffer, to avoid the 
hypercall buffer becoming COW on hypercall.

In multi-threads and multi-processes environment, e.g. the process has two 
threads, thread A 
may call hypercall, thread B may call fork() to create child process. After 
forking, all pages 
of the process including hypercall buffers are cow. The hypervisor calls 
copy_to_user in hypercall 
in thread A context, will cause a write protection and return EFAULT error.

Fix:
1. Before hypercall: use MADV_DONTFORK of madvise syscall to make the hypercall 
buffer 
   not to be copied to child process after fork. 
2. After hypercall: undo the effect of MADV_DONTFORK for the hypercall buffer 
by 
   using MADV_DOFORK of madvise syscall.
3. Use mmap/nunmap for memory alloc/free instead of malloc/free to bypass libc.

Note: 
Chlid process do not use xc interface handle which is opend by parent process, 
it should open 
a fresh handle if it wants to keep interacting with xc. Otherwise, it may cause 
segment fault 
to access hypercall buffer cache in the handle.

Signed-off-by: Zhenguo Wang <wangzhenguo@xxxxxxxxxx>
Signed-off-by: Xiaowei Yang <xiaowei.yang@xxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r a5dfd924fcdb tools/libxc/xc_linux_osdep.c
--- a/tools/libxc/xc_linux_osdep.c      Tue Aug 07 13:52:10 2012 +0800
+++ b/tools/libxc/xc_linux_osdep.c      Wed Aug 08 19:31:53 2012 +0800
@@ -93,22 +93,20 @@ static void *linux_privcmd_alloc_hyperca
     size_t size = npages * XC_PAGE_SIZE;
     void *p;
 
-    p = xc_memalign(xch, XC_PAGE_SIZE, size);
-    if (!p)
-        return NULL;
+    /* Address returned by mmap is page aligned. */
+    p = mmap(NULL, size, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_ANONYMOUS|MAP_LOCKED, -1, 0);
 
-    if ( mlock(p, size) < 0 )
-    {
-        free(p);
-        return NULL;
-    }
+    /* Do not copy the VMA to child process on fork. Avoid the page being COW 
on hypercall. */
+    madvise(ptr, npages * XC_PAGE_SIZE, MADV_DONTFORK);
     return p;
 }
 
 static void linux_privcmd_free_hypercall_buffer(xc_interface *xch, 
xc_osdep_handle h, void *ptr, int npages)
 {
-    munlock(ptr, npages * XC_PAGE_SIZE);
-    free(ptr);
+    /* Recover the VMA flags. Maybe it's not necessary */
+    madvise(ptr, npages * XC_PAGE_SIZE, MADV_DOFORK);
+    
+    munmap(ptr, npages * XC_PAGE_SIZE);
 }
 
 static int linux_privcmd_hypercall(xc_interface *xch, xc_osdep_handle h, 
privcmd_hypercall_t *hypercall)
diff -r a5dfd924fcdb tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h     Tue Aug 07 13:52:10 2012 +0800
+++ b/tools/libxc/xenctrl.h     Wed Aug 08 19:31:53 2012 +0800
@@ -131,8 +131,11 @@ typedef enum xc_error_code xc_error_code
 
 /**
  * This function opens a handle to the hypervisor interface.  This function can
- * be called multiple times within a single process.  Multiple processes can
- * have an open hypervisor interface at the same time.
+ * be called multiple times within a single process.  Multiple processes can 
not
+ * have an open hypervisor interface at the same time. Chlid process do not 
+ * use xc interface handle which is opend by parent process, it should open
+ * a fresh handle if it wants to keep interacting with xc. Otherwise, it may 
+ * cause segment fault to access hypercall buffer cache in the handle.
  *
  * Each call to this function should have a corresponding call to
  * xc_interface_close().
@@ -908,6 +911,11 @@ int xc_evtchn_status(xc_interface *xch, 
  * Return a handle to the event channel driver, or -1 on failure, in which case
  * errno will be set appropriately.
  *
+ * Note:
+ * Chlid process do not use xc evtchn handle which is opend by parent process, 
+ * it should open a fresh handle if it wants to keep interacting with xc. 
Otherwise, 
+ * it may cause segment fault to access hypercall buffer cache in the handle.
+ *
  * Before Xen pre-4.1 this function would sometimes report errors with perror.
  */
 xc_evtchn *xc_evtchn_open(xentoollog_logger *logger,
@@ -1339,9 +1347,12 @@ int xc_domain_subscribe_for_suspend(
 
 /*
  * These functions sometimes log messages as above, but not always.
- */
-
-/*
+ *
+ * Note:
+ * Chlid process do not use xc gnttab handle which is opend by parent process, 
+ * it should open a fresh handle if it wants to keep interacting with xc. 
Otherwise, 
+ * it may cause segment fault to access hypercall buffer cache in the handle.
+ *
  * Return an fd onto the grant table driver.  Logs errors.
  */
 xc_gnttab *xc_gnttab_open(xentoollog_logger *logger,
@@ -1458,6 +1469,12 @@ grant_entry_v2_t *xc_gnttab_map_table_v2
 
 /*
  * Return an fd onto the grant sharing driver.  Logs errors.
+ *
+ * Note:
+ * Chlid process do not use xc gntshr handle which is opend by parent process, 
+ * it should open a fresh handle if it wants to keep interacting with xc. 
Otherwise, 
+ * it may cause segment fault to access hypercall buffer cache in the handle.
+ *
  */
 xc_gntshr *xc_gntshr_open(xentoollog_logger *logger,
                          unsigned open_flags);
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel