. expanded grant manipulation interface to more direct

manipulation
. made fs allocate a grant for every ioctl, even if no data
  is being copied, in order to disambiguate concurrent ioctls
  on the same minor

include/minix/safecopies.h

@@ -71,6 +71,14 @@ _PROTOTYPE( int cpf_revoke, (cp_grant_id_t grant_id));
 _PROTOTYPE( int cpf_preallocate, (cp_grant_t *, int));
 _PROTOTYPE( int cpf_lookup, (cp_grant_id_t g, endpoint_t *ep, endpoint_t *ep2));
 
+_PROTOTYPE( int cpf_getgrants, (cp_grant_id_t *grant_ids, int n));
+_PROTOTYPE( int cpf_setgrant_direct, (cp_grant_id_t g, endpoint_t who,
+	vir_bytes addr, size_t size, int access));
+_PROTOTYPE( int cpf_setgrant_indirect, (cp_grant_id_t g, endpoint_t who_to,
+	endpoint_t who_from, cp_grant_id_t his_g));
+_PROTOTYPE( int cpf_setgrant_magic, (cp_grant_id_t g, endpoint_t who_to,
+	endpoint_t who_from, vir_bytes addr, size_t bytes, int access));
+
 /* Set a process' grant table location and size (in-kernel only). */
 #define _K_SET_GRANT_TABLE(rp, ptr, entries)	\
 	priv(rp)->s_grant_table= (ptr);		\

lib/syslib/safecopies.c

@@ -15,6 +15,28 @@
 #include <minix/com.h>
 #include <string.h>
 
+#define ACCESS_CHECK(a) { 			\
+	if((a) & ~(CPF_READ|CPF_WRITE)) {	\
+		errno = EINVAL;			\
+		return -1;			\
+	}					\
+   }
+
+#define GID_CHECK(gid) {					\
+	if(!GRANT_VALID(gid) || (gid) < 0 || (gid) >= ngrants) {\
+		errno = EINVAL;					\
+		return -1;					\
+	}							\
+   }
+
+#define GID_CHECK_USED(gid) {					\
+	GID_CHECK(gid);						\
+	if(!(grants[gid].cp_flags & CPF_USED)) {		\
+		errno = EINVAL;					\
+		return -1;					\
+	}							\
+   }
+
 PRIVATE cp_grant_t *grants = NULL;
 PRIVATE int ngrants = 0, dynamic = 1;
 
@@ -147,26 +169,21 @@ PUBLIC cp_grant_id_t
 cpf_grant_direct(endpoint_t who_to, vir_bytes addr, size_t bytes, int access)
 {
 	cp_grant_id_t g;
+	int r;
  
 	/* Get new slot to put new grant in. */
 	if((g = cpf_new_grantslot()) < 0)
 		return -1;
 
-	/* Don't let caller specify any other flags than access. */
-	if(access & ~(CPF_READ|CPF_WRITE)) {
-		errno = EINVAL;
-		return -1;
-	}
-
 	assert(GRANT_VALID(g));
 	assert(g >= 0);
 	assert(g < ngrants);
 	assert(!(grants[g].cp_flags & CPF_USED));
 
-	grants[g].cp_flags = CPF_USED | CPF_DIRECT | access;
+	if((r=cpf_setgrant_direct(g, who_to, addr, bytes, access)) < 0) {
-	grants[g].cp_u.cp_direct.cp_who_to = who_to;
+		cpf_revoke(g);
-	grants[g].cp_u.cp_direct.cp_start = addr;
+		return GRANT_INVALID;
-	grants[g].cp_u.cp_direct.cp_len = bytes;
+	}
 
 	return g;
 }
@@ -178,6 +195,7 @@ cpf_grant_indirect(endpoint_t who_to, endpoint_t who_from, cp_grant_id_t gr)
  * id 'gr'.
  */
 	cp_grant_id_t g;
+	int r;
 
 	/* Obtain new slot. */
 	if((g = cpf_new_grantslot()) < 0)
@@ -190,10 +208,10 @@ cpf_grant_indirect(endpoint_t who_to, endpoint_t who_from, cp_grant_id_t gr)
 	assert(!(grants[g].cp_flags & CPF_USED));
 
 	/* Fill in new slot data. */
-	grants[g].cp_flags = CPF_USED | CPF_INDIRECT;
+	if((r=cpf_setgrant_indirect(g, who_to, who_from, gr)) < 0) {
-	grants[g].cp_u.cp_indirect.cp_who_to = who_to;
+		cpf_revoke(g);
-	grants[g].cp_u.cp_indirect.cp_who_from = who_from;
+		return GRANT_INVALID;
-	grants[g].cp_u.cp_indirect.cp_grant = gr;
+	}
 
 	return g;
 }
@@ -204,6 +222,9 @@ cpf_grant_magic(endpoint_t who_to, endpoint_t who_from,
 {
 /* Grant process A access into process B. Not everyone can do this. */
 	cp_grant_id_t g;
+	int r;
+
+	ACCESS_CHECK(access);
 
 	/* Obtain new slot. */
 	if((g = cpf_new_grantslot()) < 0)
@@ -215,19 +236,12 @@ cpf_grant_magic(endpoint_t who_to, endpoint_t who_from,
 	assert(g < ngrants);
 	assert(!(grants[g].cp_flags & CPF_USED));
 
-	/* Don't let caller specify any other flags than access. */
+	if((r=cpf_setgrant_magic(g, who_to, who_from, addr,
-	if(access & ~(CPF_READ|CPF_WRITE)) {
+		bytes, access)) < 0) {
-		errno = EINVAL;
+		cpf_revoke(g);
 		return -1;
 	}
 
-	/* Fill in new slot data. */
-	grants[g].cp_flags = CPF_USED | CPF_MAGIC | access;
-	grants[g].cp_u.cp_magic.cp_who_to = who_to;
-	grants[g].cp_u.cp_magic.cp_who_from = who_from;
-	grants[g].cp_u.cp_magic.cp_start = addr;
-	grants[g].cp_u.cp_magic.cp_len = bytes;
-
 	return g;
 }
 
@@ -235,11 +249,7 @@ PUBLIC int
 cpf_revoke(cp_grant_id_t g)
 {
 /* Revoke previously granted access, identified by grant id. */
-	/* First check slot validity, and if it's in use currently. */
+	GID_CHECK_USED(g);
-	if(g < 0 || g >= ngrants || !(grants[g].cp_flags & CPF_USED)) {
-		errno = EINVAL;
-		return -1;
-	}
 
 	/* Make grant invalid by setting flags to 0, clearing CPF_USED.
 	 * This invalidates the grant.
@@ -253,11 +263,7 @@ PUBLIC int
 cpf_lookup(cp_grant_id_t g, endpoint_t *granter, endpoint_t *grantee)
 {
 	/* First check slot validity, and if it's in use currently. */
-	if(!GRANT_VALID(g) ||
+	GID_CHECK_USED(g);
-	   g < 0 || g >= ngrants || !(grants[g].cp_flags & CPF_USED)) {
-		errno = EINVAL;
-		return -1;
-	}
 
 	if(grants[g].cp_flags & CPF_DIRECT) {
 		if(granter) *granter = SELF;
@@ -270,3 +276,78 @@ cpf_lookup(cp_grant_id_t g, endpoint_t *granter, endpoint_t *grantee)
 	return 0;
 }
 
+PUBLIC int
+cpf_getgrants(grant_ids, n)
+cp_grant_id_t *grant_ids;
+int n;
+{
+	cp_grant_id_t g;
+	int i;
+
+	for(i = 0; i < n; i++) {
+	  if((grant_ids[i] = cpf_new_grantslot()) < 0)
+		break;
+	  cpf_revoke(grant_ids[i]);
+	}
+
+	/* return however many grants were assigned. */
+	return i;
+}
+
+PUBLIC int
+cpf_setgrant_direct(gid, who, addr, bytes, access)
+cp_grant_id_t gid;
+endpoint_t who;
+vir_bytes addr;
+size_t bytes;
+int access;
+{
+	GID_CHECK(gid);
+	ACCESS_CHECK(access);
+
+	grants[gid].cp_flags = access | CPF_DIRECT | CPF_USED;
+	grants[gid].cp_u.cp_direct.cp_who_to = who;
+	grants[gid].cp_u.cp_direct.cp_start = addr;
+	grants[gid].cp_u.cp_direct.cp_len = bytes;
+
+	return 0;
+}
+
+PUBLIC int
+cpf_setgrant_indirect(gid, who_to, who_from, his_gid)
+cp_grant_id_t gid;
+endpoint_t who_to, who_from;
+cp_grant_id_t his_gid;
+{
+	GID_CHECK(gid);
+
+	/* Fill in new slot data. */
+	grants[gid].cp_flags = CPF_USED | CPF_INDIRECT;
+	grants[gid].cp_u.cp_indirect.cp_who_to = who_to;
+	grants[gid].cp_u.cp_indirect.cp_who_from = who_from;
+	grants[gid].cp_u.cp_indirect.cp_grant = his_gid;
+
+	return 0;
+}
+
+PUBLIC int
+cpf_setgrant_magic(gid, who_to, who_from, addr, bytes, access)
+cp_grant_id_t gid;
+endpoint_t who_to, who_from;
+vir_bytes addr;
+size_t bytes;
+int access;
+{
+	GID_CHECK(gid);
+	ACCESS_CHECK(access);
+
+	/* Fill in new slot data. */
+	grants[gid].cp_flags = CPF_USED | CPF_MAGIC | access;
+	grants[gid].cp_u.cp_magic.cp_who_to = who_to;
+	grants[gid].cp_u.cp_magic.cp_who_from = who_from;
+	grants[gid].cp_u.cp_magic.cp_start = addr;
+	grants[gid].cp_u.cp_magic.cp_len = bytes;
+
+	return 0;
+}
+

servers/fs/device.c

@@ -178,14 +178,6 @@ off_t *pos;
 	int m, j;
 	iovec_t *v;
 
-	/* Is this device driver (identified by major number)
-	 * ready to accept *_S commands?
-	 */
-#if 0
-	if(major(dev) == 7)	/* major number of inet. */
-		return 0;	/* inet is not safe-capable. */
-#endif
-
 	/* Number of grants allocated in vector I/O. */
 	*vec_grants = 0;
 
@@ -239,13 +231,15 @@ off_t *pos;
 			if(_MINIX_IOCTL_IOR(m_in.REQUEST)) access |= CPF_WRITE;
 			if(_MINIX_IOCTL_IOW(m_in.REQUEST)) access |= CPF_READ;
 			size = _MINIX_IOCTL_SIZE(m_in.REQUEST);
-			if(access && size > 0) {
+
-				if((*gid=cpf_grant_magic(driver, *io_ept,
+			/* Do this even if no I/O happens with the ioctl, in
-					(vir_bytes) buf, size, access)) < 0) {
+			 * order to disambiguate requests with DEV_IOCTL_S.
-					panic(__FILE__,
+			 */
-					"cpf_grant_magic failed (ioctl)\n",
+			if((*gid=cpf_grant_magic(driver, *io_ept,
-					NO_NUM);
+				(vir_bytes) buf, size, access)) < 0) {
-				}
+				panic(__FILE__,
+				"cpf_grant_magic failed (ioctl)\n",
+				NO_NUM);
 			}
 	}