Fix intermittent failures in lin_lin_copy and verify_grant.

Sometimes, the srcptr or dstptr in lin_lin_copy would have the RTS_VMINHIBIT flag set leading to an assertion error. In case this happens defer the kernel call instead of panicking. Verify_grant would somtimes page-fault when accessing an entry. Defer the kernel call when that happens.
Stichting-MINIX-Research-Foundation · Mar 10, 2019 · 663ee11 · 663ee11
1 parent dd0c109
commit 663ee11
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 16 deletions.
diff --git a/minix/kernel/arch/i386/do_sdevio.c b/minix/kernel/arch/i386/do_sdevio.c
@@ -13,6 +13,7 @@
 #include "kernel/system.h"
 #include <minix/devio.h>
 #include <minix/endpoint.h>
+#include "kernel/vm.h"
 
 #include "arch_proto.h"
 
@@ -67,12 +68,13 @@ int do_sdevio(struct proc * caller, message *m_ptr)
   /* Check for 'safe' variants. */
   if((m_ptr->m_lsys_krn_sys_sdevio.request & _DIO_SAFEMASK) == _DIO_SAFE) {
      /* Map grant address to physical address. */
-     if((r=verify_grant(proc_nr_e, caller->p_endpoint,
+     if((r=verify_grant(caller,proc_nr_e, caller->p_endpoint,
 		m_ptr->m_lsys_krn_sys_sdevio.vec_addr, count,
 		req_dir == _DIO_INPUT ? CPF_WRITE : CPF_READ,
 		m_ptr->m_lsys_krn_sys_sdevio.offset, &newoffset, &newep,
 		NULL)) != OK) {
 	if(r == ENOTREADY) return r;
+	if(r == VMSUSPEND) return r;
 	printf("do_sdevio: verify_grant failed\n");
 	return EPERM;
     }

diff --git a/minix/kernel/arch/i386/memory.c b/minix/kernel/arch/i386/memory.c
@@ -164,8 +164,16 @@ static int lin_lin_copy(struct proc *srcproc, vir_bytes srclinaddr,
 	if(dstproc) assert(!RTS_ISSET(dstproc, RTS_SLOT_FREE));
 	assert(!RTS_ISSET(get_cpulocal_var(ptproc), RTS_SLOT_FREE));
 	assert(get_cpulocal_var(ptproc)->p_seg.p_cr3_v);
-	if(srcproc) assert(!RTS_ISSET(srcproc, RTS_VMINHIBIT));
-	if(dstproc) assert(!RTS_ISSET(dstproc, RTS_VMINHIBIT));
+	if(srcproc&&RTS_ISSET(srcproc, RTS_VMINHIBIT)) {
+		/* If the src is marked with an unstable memory space, then
+		 * suspend as if a page fault occured. */
+		return EFAULT_SRC;
+	}
+	if(dstproc&&RTS_ISSET(dstproc, RTS_VMINHIBIT)) {
+		/* If the dst is marked with an unstable memory space, then
+		 * suspend as if a page fault occured. */
+		return EFAULT_DST;
+	}
 
 	while(bytes > 0) {
 		phys_bytes srcptr, dstptr;

diff --git a/minix/kernel/proto.h b/minix/kernel/proto.h
@@ -157,7 +157,7 @@ void hook_ipc_clear(struct proc *proc);
 
 /* system/do_safecopy.c */
 struct cp_sfinfo; /* external callers may only provide NULL */
-int verify_grant(endpoint_t, endpoint_t, cp_grant_id_t, vir_bytes, int,
+int verify_grant(struct proc*,endpoint_t, endpoint_t, cp_grant_id_t, vir_bytes, int,
 	vir_bytes, vir_bytes *, endpoint_t *, struct cp_sfinfo *);
 
 /* system/do_diagctl.c */

diff --git a/minix/kernel/system/do_safecopy.c b/minix/kernel/system/do_safecopy.c
@@ -39,6 +39,7 @@ struct cp_sfinfo {		/* information for handling soft faults */
  *				verify_grant				     *
  *===========================================================================*/
 int verify_grant(
+  struct proc *caller,		/* The caller */
   endpoint_t granter,		/* copyee */
   endpoint_t grantee,		/* copyer */
   cp_grant_id_t grant,		/* grant id */
@@ -118,12 +119,25 @@ int verify_grant(
 		 * has (presumably) set an invalid grant table entry by
 		 * returning EPERM, just like with an invalid grant id.
 		 */
-		if(data_copy(granter, priv(granter_proc)->s_grant_table +
-			sizeof(g) * grant_idx,
-			KERNEL, (vir_bytes) &g, sizeof(g)) != OK) {
-			printf(
-			"verify_grant: grant verify: data_copy failed\n");
-			return EPERM;
+		const vir_bytes entry_addr = priv(granter_proc)->s_grant_table+sizeof(g)*grant_idx;
+		const int copy_res =data_copy_vmcheck(caller,granter,entry_addr,
+			KERNEL, (vir_bytes) &g, sizeof(g));
+		if(copy_res!=OK) {
+                       /* The copy failed, it may be because we had a page
+                        * fault from the source. In this case, the caller has
+                        * been suspended already, but we need to propagate the
+                        * VMSUSPEND to the upper level (until kernel_call_finish
+                        * to make it happen. */
+                       if(copy_res==VMSUSPEND) {
+                               /* Propagate the VMSUSPEND. */
+                               return VMSUSPEND;
+                       } else {
+                               /* The reason is not a pagefault in the source
+                                * , in this case report the error. */
+                               panic(
+                               "verify_grant: grant verify: data_copy failed\n");
+                               return EPERM;
+                       }
 		}
 
 		/* Check validity: flags and sequence number. */
@@ -302,9 +316,10 @@ static int safecopy(
 	}
 
 	/* Verify permission exists. */
-	if((r=verify_grant(granter, grantee, grantid, bytes, access,
+	if((r=verify_grant(caller,granter, grantee, grantid, bytes, access,
 	    g_offset, &v_offset, &new_granter, &sfinfo)) != OK) {
 		if(r == ENOTREADY) return r;
+		if(r == VMSUSPEND) return r;
 			printf(
 		"grant %d verify to copy %d->%d by %d failed: err %d\n",
 				grantid, *src, *dst, grantee, r);

diff --git a/minix/kernel/system/do_safememset.c b/minix/kernel/system/do_safememset.c
@@ -13,6 +13,7 @@
 #include <minix/safecopies.h>
 
 #include "kernel/system.h"
+#include "kernel/vm.h"
 
 /*===========================================================================*
  *                              do_safememset                                *
@@ -45,9 +46,9 @@ int do_safememset(struct proc *caller, message *m_ptr) {
 	}
 
 	/* Verify permission exists, memset always requires CPF_WRITE */
-	r = verify_grant(dst_endpt, caller_endpt, grantid, len, CPF_WRITE,
+	r = verify_grant(caller,dst_endpt, caller_endpt, grantid, len, CPF_WRITE,
 			 g_offset, &v_offset, &new_granter, NULL);
-
+	if(r==VMSUSPEND) return r;
 	if (r != OK) {
 		printf("safememset: grant %d verify failed %d", grantid, r);
 		return r;

diff --git a/minix/kernel/system/do_umap_remote.c b/minix/kernel/system/do_umap_remote.c
@@ -11,6 +11,7 @@
  */
 
 #include "kernel/system.h"
+#include "kernel/vm.h"
 
 #include <minix/endpoint.h>
 
@@ -63,8 +64,10 @@ int do_umap_remote(struct proc * caller, message * m_ptr)
 	int new_proc_nr;
 	cp_grant_id_t grant = (cp_grant_id_t) offset;
 
-        if(verify_grant(targetpr->p_endpoint, grantee, grant, count,
-                0, 0, &newoffset, &newep, NULL) != OK) {
+	const int vres =verify_grant(caller,targetpr->p_endpoint, grantee, grant, count,
+		0, 0, &newoffset, &newep, NULL);
+	if(vres==VMSUSPEND) return vres;
+        if(vres!=OK) {
                 printf("SYSTEM: do_umap: verify_grant in %s, grant %d, bytes 0x%lx, failed, caller %s\n", targetpr->p_name, offset, count, caller->p_name);
 		proc_stacktrace(caller);
                 return EFAULT;

diff --git a/minix/kernel/system/do_vumap.c b/minix/kernel/system/do_vumap.c
@@ -77,7 +77,7 @@ int do_vumap(struct proc *caller, message *m_ptr)
 	size -= offset;
 
 	if (source != SELF) {
-		r = verify_grant(source, endpt, vvec[i].vv_grant, size, access,
+		r = verify_grant(caller,source, endpt, vvec[i].vv_grant, size, access,
 			offset, &vir_addr, &granter, NULL);
 		if (r != OK)
 			return r;