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]
> Sent: Wednesday, August 08, 2012 8:06 PM
> To: Wangzhenguo
> Cc: Yangxiaowei; Yechuan; Hongtao; xen-devel@xxxxxxxxxxxxx; Ian Jackson
> Subject: Re: [Xen-devel] The hypercall will fail and return EFAULT when
> the page becomes COW by forking process in linux
> 
> > Note:
> > Chlid process do not use xc interface handle which is opend by parent
> process, it should open
> 
> "Child" and "opened" (you made these typos pretty consistently
> throughout ;-))
> 
> Please can you try and keep the commit message to 75-80 characters wide.

Hi, Ian, thanks your reply. There is a new patch as follow

# 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. It will cause a write protection and return EFAULT error if hypervisor
calls copy_to_user in hypercall in thread A context,

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:
Child processes must not use the opened xc_{interface,evtchn,gnttab,gntshr}
handle that inherits from parents. They should reopen the handle if they want
to interact with xc. Otherwise, it may cause segment fault to access hypercall
buffer caches of 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      Thu Aug 09 14:42:29 2012 +0800
@@ -93,22 +93,21 @@ 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     Thu Aug 09 14:42:29 2012 +0800
@@ -134,6 +134,12 @@ typedef enum xc_error_code xc_error_code
  * be called multiple times within a single process.  Multiple processes can
  * have an open hypervisor interface at the same time.
  *
+ * Note:
+ * Child processes must not use the opened xc interface handle that inherits
+ * from parents. They should reopen the handle if they want to interact with
+ * xc. Otherwise, it may cause segment fault to access hypercall buffer caches
+ * of the handle.
+ *
  * Each call to this function should have a corresponding call to
  * xc_interface_close().
  *
@@ -908,6 +914,12 @@ 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:
+ * Child processes must not use the opened xc evtchn handle that inherits from
+ * parents. They should reopen the handle if they want to interact with xc.
+ * Otherwise, it may cause segment fault to access hypercall buffer caches of
+ * 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 +1351,13 @@ int xc_domain_subscribe_for_suspend(
 
 /*
  * These functions sometimes log messages as above, but not always.
- */
-
-/*
+ *
+ * Note:
+ * Child processes must not use the opened xc gnttab handle that inherits from
+ * parents. They should reopen the handle if they want to interact with xc.
+ * Otherwise, it may cause segment fault to access hypercall buffer caches of
+ * the handle.
+ *
  * Return an fd onto the grant table driver.  Logs errors.
  */
 xc_gnttab *xc_gnttab_open(xentoollog_logger *logger,
@@ -1458,6 +1474,13 @@ grant_entry_v2_t *xc_gnttab_map_table_v2
 
 /*
  * Return an fd onto the grant sharing driver.  Logs errors.
+ *
+ * Note:
+ * Child processes must not use the opened xc gntshr handle that inherits from
+ * parents. They should reopen the handle if they want to interact with xc.
+ * Otherwise, it may cause segment fault to access hypercall buffer caches of
+ * 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