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.
This commit is contained in:
Justinien Bouron 2019-03-10 05:22:08 -07:00
parent dd0c1095a1
commit 663ee11368
7 changed files with 45 additions and 16 deletions

View File

@ -13,6 +13,7 @@
#include "kernel/system.h" #include "kernel/system.h"
#include <minix/devio.h> #include <minix/devio.h>
#include <minix/endpoint.h> #include <minix/endpoint.h>
#include "kernel/vm.h"
#include "arch_proto.h" #include "arch_proto.h"
@ -67,12 +68,13 @@ int do_sdevio(struct proc * caller, message *m_ptr)
/* Check for 'safe' variants. */ /* Check for 'safe' variants. */
if((m_ptr->m_lsys_krn_sys_sdevio.request & _DIO_SAFEMASK) == _DIO_SAFE) { if((m_ptr->m_lsys_krn_sys_sdevio.request & _DIO_SAFEMASK) == _DIO_SAFE) {
/* Map grant address to physical address. */ /* 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, m_ptr->m_lsys_krn_sys_sdevio.vec_addr, count,
req_dir == _DIO_INPUT ? CPF_WRITE : CPF_READ, req_dir == _DIO_INPUT ? CPF_WRITE : CPF_READ,
m_ptr->m_lsys_krn_sys_sdevio.offset, &newoffset, &newep, m_ptr->m_lsys_krn_sys_sdevio.offset, &newoffset, &newep,
NULL)) != OK) { NULL)) != OK) {
if(r == ENOTREADY) return r; if(r == ENOTREADY) return r;
if(r == VMSUSPEND) return r;
printf("do_sdevio: verify_grant failed\n"); printf("do_sdevio: verify_grant failed\n");
return EPERM; return EPERM;
} }

View File

@ -164,8 +164,16 @@ static int lin_lin_copy(struct proc *srcproc, vir_bytes srclinaddr,
if(dstproc) assert(!RTS_ISSET(dstproc, RTS_SLOT_FREE)); if(dstproc) assert(!RTS_ISSET(dstproc, RTS_SLOT_FREE));
assert(!RTS_ISSET(get_cpulocal_var(ptproc), RTS_SLOT_FREE)); assert(!RTS_ISSET(get_cpulocal_var(ptproc), RTS_SLOT_FREE));
assert(get_cpulocal_var(ptproc)->p_seg.p_cr3_v); assert(get_cpulocal_var(ptproc)->p_seg.p_cr3_v);
if(srcproc) assert(!RTS_ISSET(srcproc, RTS_VMINHIBIT)); if(srcproc&&RTS_ISSET(srcproc, RTS_VMINHIBIT)) {
if(dstproc) assert(!RTS_ISSET(dstproc, 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) { while(bytes > 0) {
phys_bytes srcptr, dstptr; phys_bytes srcptr, dstptr;

View File

@ -157,7 +157,7 @@ void hook_ipc_clear(struct proc *proc);
/* system/do_safecopy.c */ /* system/do_safecopy.c */
struct cp_sfinfo; /* external callers may only provide NULL */ 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 *); vir_bytes, vir_bytes *, endpoint_t *, struct cp_sfinfo *);
/* system/do_diagctl.c */ /* system/do_diagctl.c */

View File

@ -39,6 +39,7 @@ struct cp_sfinfo { /* information for handling soft faults */
* verify_grant * * verify_grant *
*===========================================================================*/ *===========================================================================*/
int verify_grant( int verify_grant(
struct proc *caller, /* The caller */
endpoint_t granter, /* copyee */ endpoint_t granter, /* copyee */
endpoint_t grantee, /* copyer */ endpoint_t grantee, /* copyer */
cp_grant_id_t grant, /* grant id */ cp_grant_id_t grant, /* grant id */
@ -118,12 +119,25 @@ int verify_grant(
* has (presumably) set an invalid grant table entry by * has (presumably) set an invalid grant table entry by
* returning EPERM, just like with an invalid grant id. * returning EPERM, just like with an invalid grant id.
*/ */
if(data_copy(granter, priv(granter_proc)->s_grant_table + const vir_bytes entry_addr = priv(granter_proc)->s_grant_table+sizeof(g)*grant_idx;
sizeof(g) * grant_idx, const int copy_res =data_copy_vmcheck(caller,granter,entry_addr,
KERNEL, (vir_bytes) &g, sizeof(g)) != OK) { KERNEL, (vir_bytes) &g, sizeof(g));
printf( if(copy_res!=OK) {
"verify_grant: grant verify: data_copy failed\n"); /* The copy failed, it may be because we had a page
return EPERM; * 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. */ /* Check validity: flags and sequence number. */
@ -302,9 +316,10 @@ static int safecopy(
} }
/* Verify permission exists. */ /* 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) { g_offset, &v_offset, &new_granter, &sfinfo)) != OK) {
if(r == ENOTREADY) return r; if(r == ENOTREADY) return r;
if(r == VMSUSPEND) return r;
printf( printf(
"grant %d verify to copy %d->%d by %d failed: err %d\n", "grant %d verify to copy %d->%d by %d failed: err %d\n",
grantid, *src, *dst, grantee, r); grantid, *src, *dst, grantee, r);

View File

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

View File

@ -11,6 +11,7 @@
*/ */
#include "kernel/system.h" #include "kernel/system.h"
#include "kernel/vm.h"
#include <minix/endpoint.h> #include <minix/endpoint.h>
@ -63,8 +64,10 @@ int do_umap_remote(struct proc * caller, message * m_ptr)
int new_proc_nr; int new_proc_nr;
cp_grant_id_t grant = (cp_grant_id_t) offset; cp_grant_id_t grant = (cp_grant_id_t) offset;
if(verify_grant(targetpr->p_endpoint, grantee, grant, count, const int vres =verify_grant(caller,targetpr->p_endpoint, grantee, grant, count,
0, 0, &newoffset, &newep, NULL) != OK) { 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); 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); proc_stacktrace(caller);
return EFAULT; return EFAULT;

View File

@ -77,7 +77,7 @@ int do_vumap(struct proc *caller, message *m_ptr)
size -= offset; size -= offset;
if (source != SELF) { 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); offset, &vir_addr, &granter, NULL);
if (r != OK) if (r != OK)
return r; return r;