From: David van Moolenbroek <david@minix3.org>
Date: Sun, 11 Dec 2011 21:30:35 +0000 (+0100)
Subject: Add "expected size" parameter to getsysinfo()
X-Git-Tag: v3.2.0~185
X-Git-Url: http://zhaoyanbai.com/repos/?a=commitdiff_plain;h=6f374faca5c32553406a3cb4edb79fb0d9c89966;p=minix.git

Add "expected size" parameter to getsysinfo()

This patch provides basic protection against damage resulting from
differently compiled servers blindly copying tables to one another.
In every getsysinfo() call, the caller is provided with the expected
size of the requested data structure. The callee fails the call if
the expected size does not match the data structure's actual size.
---

diff --git a/common/include/minix/com.h b/common/include/minix/com.h
index d3d5d2809..678637673 100644
--- a/common/include/minix/com.h
+++ b/common/include/minix/com.h
@@ -849,6 +849,7 @@
 #define COMMON_GETSYSINFO	(COMMON_RQ_BASE+2)
 #	define SI_WHAT		m1_i1
 #	define SI_WHERE		m1_p1
+#	define SI_SIZE		m1_i2
 
 /* PM field names */
 /* BRK */
diff --git a/common/include/minix/sysinfo.h b/common/include/minix/sysinfo.h
index d2b602c7d..ddb8afbd7 100644
--- a/common/include/minix/sysinfo.h
+++ b/common/include/minix/sysinfo.h
@@ -5,14 +5,13 @@
 #include <minix/endpoint.h>
 #include <minix/type.h>
 
-_PROTOTYPE( int getsysinfo, (endpoint_t who, int what, void *where)       );
+_PROTOTYPE( int getsysinfo, (endpoint_t who, int what, void *where,
+	size_t size) );
 
 /* What system info to retrieve with sysgetinfo(). */
 #define SI_PROC_TAB	   2	/* copy of entire process table */
 #define SI_DMAP_TAB	   3	/* get device <-> driver mappings */
 #define SI_DATA_STORE	   5	/* get copy of data store mappings */
-#define SI_SUBSCRIPTION	   6	/* get copy of data store subscriptions */
-#define SI_LOADINFO	   7	/* get copy of load average structure */
 #define SI_CALL_STATS	   9	/* system call statistics */
 #define SI_PROCPUB_TAB	   11	/* copy of public entries of process table */
 
diff --git a/lib/libsys/getsysinfo.c b/lib/libsys/getsysinfo.c
index 575a90051..79eacb2fe 100644
--- a/lib/libsys/getsysinfo.c
+++ b/lib/libsys/getsysinfo.c
@@ -3,14 +3,17 @@
 #include <minix/sysinfo.h>
 #include <minix/com.h>
 
-PUBLIC int getsysinfo(who, what, where)
-endpoint_t who;			/* from whom to request info */
-int what;			/* what information is requested */
-void *where;			/* where to put it */
+PUBLIC int getsysinfo(
+  endpoint_t who,		/* from whom to request info */
+  int what,			/* what information is requested */
+  void *where,			/* where to put it */
+  size_t size 			/* how big it should be */
+)
 {
   message m;
   m.SI_WHAT = what;
   m.SI_WHERE = where;
+  m.SI_SIZE = size;
   if (_syscall(who, COMMON_GETSYSINFO, &m) < 0) return(-1);
   return(0);
 }
diff --git a/servers/avfs/misc.c b/servers/avfs/misc.c
index 89e8ac067..f05352619 100644
--- a/servers/avfs/misc.c
+++ b/servers/avfs/misc.c
@@ -70,7 +70,7 @@ PUBLIC int do_getsysinfo()
 
   if (!super_user) return(EPERM);
 
-  switch(m_in.info_what) {
+  switch(m_in.SI_WHAT) {
     case SI_PROC_TAB:
 	src_addr = (vir_bytes) fproc;
 	len = sizeof(struct fproc) * NR_PROCS;
@@ -89,7 +89,10 @@ PUBLIC int do_getsysinfo()
 	return(EINVAL);
   }
 
-  dst_addr = (vir_bytes) m_in.info_where;
+  if (len != m_in.SI_SIZE)
+	return(EINVAL);
+
+  dst_addr = (vir_bytes) m_in.SI_WHERE;
   return sys_datacopy(SELF, src_addr, who_e, dst_addr, len);
 }
 
diff --git a/servers/avfs/param.h b/servers/avfs/param.h
index ad9107625..ea05621bb 100644
--- a/servers/avfs/param.h
+++ b/servers/avfs/param.h
@@ -45,8 +45,6 @@
 #define whence	      m2_i2
 #define svrctl_req    m2_i1
 #define svrctl_argp   m2_p1
-#define info_what     m1_i1
-#define info_where    m1_p1
 #define md_label	m2_p1
 #define md_label_len	m2_l1
 #define md_major	m2_i1
diff --git a/servers/ds/store.c b/servers/ds/store.c
index 401dc15ad..1c8efa4b0 100644
--- a/servers/ds/store.c
+++ b/servers/ds/store.c
@@ -776,21 +776,20 @@ PUBLIC int do_getsysinfo(const message *m_ptr)
   size_t length;
   int s;
 
-  switch(m_ptr->m1_i1) {
+  switch(m_ptr->SI_WHAT) {
   case SI_DATA_STORE:
 	src_addr = (vir_bytes)ds_store;
 	length = sizeof(struct data_store) * NR_DS_KEYS;
 	break;
-  case SI_SUBSCRIPTION:
-	src_addr = (vir_bytes)ds_subs;
-	length = sizeof(struct subscription) * NR_DS_SUBS;
-	break;
   default:
   	return EINVAL;
   }
 
+  if (length != m_ptr->SI_SIZE)
+	return EINVAL;
+
   if (OK != (s=sys_datacopy(SELF, src_addr,
-		m_ptr->m_source, (vir_bytes)m_ptr->m1_p1, length))) {
+		m_ptr->m_source, (vir_bytes)m_ptr->SI_WHERE, length))) {
 	printf("DS: copy failed: %d\n", s);
 	return s;
   }
diff --git a/servers/is/dmp_ds.c b/servers/is/dmp_ds.c
index 38cc18335..513a93727 100644
--- a/servers/is/dmp_ds.c
+++ b/servers/is/dmp_ds.c
@@ -9,10 +9,10 @@ PUBLIC void data_store_dmp()
 {
   struct data_store *p;
   static int prev_i = 0;
-  int r, i, n = 0;
+  int i, n = 0;
 
-  if((r=getsysinfo(DS_PROC_NR, SI_DATA_STORE, ds_store)) != OK) {
-	printf("Couldn't talk to DS: %d.\n", r);
+  if (getsysinfo(DS_PROC_NR, SI_DATA_STORE, ds_store, sizeof(ds_store)) != OK) {
+	printf("Error obtaining table from DS. Perhaps recompile IS?\n");
 	return;
   }
 
diff --git a/servers/is/dmp_fs.c b/servers/is/dmp_fs.c
index ad5522ab6..92b7cd7a9 100644
--- a/servers/is/dmp_fs.c
+++ b/servers/is/dmp_fs.c
@@ -33,7 +33,10 @@ PUBLIC void fproc_dmp()
   int i, n=0;
   static int prev_i;
 
-  getsysinfo(VFS_PROC_NR, SI_PROC_TAB, fproc);
+  if (getsysinfo(VFS_PROC_NR, SI_PROC_TAB, fproc, sizeof(fproc)) != OK) {
+	printf("Error obtaining table from VFS. Perhaps recompile IS?\n");
+	return;
+  }
 
   printf("File System (FS) process table dump\n");
   printf("-nr- -pid- -tty- -umask- --uid-- --gid-- -ldr- -sus-rev-proc-\n");
@@ -107,7 +110,10 @@ PUBLIC void dtab_dmp()
 {
     int i;
 
-    getsysinfo(VFS_PROC_NR, SI_DMAP_TAB, dmap);
+    if (getsysinfo(VFS_PROC_NR, SI_DMAP_TAB, dmap, sizeof(dmap)) != OK) {
+        printf("Error obtaining table from VFS. Perhaps recompile IS?\n");
+        return;
+    }
     
     printf("File System (FS) device <-> driver mappings\n");
     printf("    Label     Major Driver ept Flags     Style   \n");
diff --git a/servers/is/dmp_pm.c b/servers/is/dmp_pm.c
index ab757bc69..921f6a31d 100644
--- a/servers/is/dmp_pm.c
+++ b/servers/is/dmp_pm.c
@@ -45,10 +45,12 @@ PUBLIC void mproc_dmp()
   int i, n=0;
   static int prev_i = 0;
 
-  printf("Process manager (PM) process table dump\n");
-
-  getsysinfo(PM_PROC_NR, SI_PROC_TAB, mproc);
+  if (getsysinfo(PM_PROC_NR, SI_PROC_TAB, mproc, sizeof(mproc)) != OK) {
+	printf("Error obtaining table from PM. Perhaps recompile IS?\n");
+	return;
+  }
 
+  printf("Process manager (PM) process table dump\n");
   printf("-process- -nr-pnr-tnr- --pid--ppid--pgrp- -uid--  -gid--  -nice- -flags-------\n");
   for (i=prev_i; i<NR_PROCS; i++) {
   	mp = &mproc[i];
@@ -77,11 +79,13 @@ PUBLIC void sigaction_dmp()
   static int prev_i = 0;
   clock_t uptime;
 
-  printf("Process manager (PM) signal action dump\n");
-
-  getsysinfo(PM_PROC_NR, SI_PROC_TAB, mproc);
+  if (getsysinfo(PM_PROC_NR, SI_PROC_TAB, mproc, sizeof(mproc)) != OK) {
+	printf("Error obtaining table from PM. Perhaps recompile IS?\n");
+	return;
+  }
   getuptime(&uptime);
 
+  printf("Process manager (PM) signal action dump\n");
   printf("-process- -nr- --ignore- --catch- --block- -pending- -alarm---\n");
   for (i=prev_i; i<NR_PROCS; i++) {
   	mp = &mproc[i];
diff --git a/servers/is/dmp_rs.c b/servers/is/dmp_rs.c
index dd8ec994c..ed6b9937a 100644
--- a/servers/is/dmp_rs.c
+++ b/servers/is/dmp_rs.c
@@ -29,8 +29,11 @@ PUBLIC void rproc_dmp()
   int i, n=0;
   static int prev_i=0;
 
-  getsysinfo(RS_PROC_NR, SI_PROCPUB_TAB, rprocpub);
-  getsysinfo(RS_PROC_NR, SI_PROC_TAB, rproc);
+  if (getsysinfo(RS_PROC_NR, SI_PROCPUB_TAB, rprocpub, sizeof(rprocpub)) != OK
+	|| getsysinfo(RS_PROC_NR, SI_PROC_TAB, rproc, sizeof(rproc)) != OK) {
+	printf("Error obtaining table from RS. Perhaps recompile IS?\n");
+	return;
+  }
 
   printf("Reincarnation Server (RS) system process table dump\n");
   printf("----label---- endpoint- -pid- flags- -dev- -T- alive_tm starts command\n");
diff --git a/servers/pm/main.c b/servers/pm/main.c
index 12b41daf0..96637e0f5 100644
--- a/servers/pm/main.c
+++ b/servers/pm/main.c
@@ -178,7 +178,7 @@ PRIVATE void sef_local_startup()
 /*===========================================================================*
  *		            sef_cb_init_fresh                                *
  *===========================================================================*/
-PRIVATE int sef_cb_init_fresh(int type, sef_init_info_t *info)
+PRIVATE int sef_cb_init_fresh(int UNUSED(type), sef_init_info_t *UNUSED(info))
 {
 /* Initialize the process manager. 
  * Memory use info is collected from the boot monitor, the kernel, and
diff --git a/servers/pm/misc.c b/servers/pm/misc.c
index a2347ec19..ea28e2d8f 100644
--- a/servers/pm/misc.c
+++ b/servers/pm/misc.c
@@ -155,9 +155,7 @@ PUBLIC int do_sysuname()
 PUBLIC int do_getsysinfo()
 {
   vir_bytes src_addr, dst_addr;
-  struct loadinfo loadinfo;
   size_t len;
-  int s;
 
   /* This call leaks important information. In the future, requests from
    * non-system processes should be denied.
@@ -170,16 +168,11 @@ PUBLIC int do_getsysinfo()
 	return EPERM;
   }
 
-  switch(m_in.info_what) {
+  switch(m_in.SI_WHAT) {
   case SI_PROC_TAB:			/* copy entire process table */
         src_addr = (vir_bytes) mproc;
         len = sizeof(struct mproc) * NR_PROCS;
         break;
-  case SI_LOADINFO:			/* loadinfo is obtained via PM */
-        sys_getloadinfo(&loadinfo);
-        src_addr = (vir_bytes) &loadinfo;
-        len = sizeof(struct loadinfo);
-        break;
 #if ENABLE_SYSCALL_STATS
   case SI_CALL_STATS:
   	src_addr = (vir_bytes) calls_stats;
@@ -190,10 +183,11 @@ PUBLIC int do_getsysinfo()
   	return(EINVAL);
   }
 
-  dst_addr = (vir_bytes) m_in.info_where;
-  if (OK != (s=sys_datacopy(SELF, src_addr, who_e, dst_addr, len)))
-  	return(s);
-  return(OK);
+  if (len != m_in.SI_SIZE)
+	return(EINVAL);
+
+  dst_addr = (vir_bytes) m_in.SI_WHERE;
+  return sys_datacopy(SELF, src_addr, who_e, dst_addr, len);
 }
 
 /*===========================================================================*
diff --git a/servers/pm/param.h b/servers/pm/param.h
index 83024fa7f..4c2863306 100644
--- a/servers/pm/param.h
+++ b/servers/pm/param.h
@@ -26,8 +26,6 @@
 #define sig_set		m2_l1
 #define sig_how		m2_i1
 #define sig_context	m2_p1
-#define info_what	m1_i1
-#define info_where	m1_p1
 #define reboot_flag	m1_i1
 #define reboot_code	m1_p1
 #define reboot_strlen	m1_i2
diff --git a/servers/procfs/tree.c b/servers/procfs/tree.c
index 404fb6fb1..2d03b7b0f 100644
--- a/servers/procfs/tree.c
+++ b/servers/procfs/tree.c
@@ -77,47 +77,78 @@ PRIVATE int dir_is_pid(struct inode *node)
 		get_inode_index(node) != NO_INDEX);
 }
 
-PRIVATE int mproc_ok(struct mproc *tab, int slots)
+/*===========================================================================*
+ *				update_proc_table			     *
+ *===========================================================================*/
+PRIVATE int update_proc_table(void)
 {
-	int i;
+	/* Get the process table from the kernel.
+	 * Check the magic number in the table entries.
+	 */
+	int r, slot;
 
-	/* sanity check of mproc */
+	if ((r = sys_getproctab(proc)) != OK) return r;
+
+	for (slot = 0; slot < NR_PROCS + NR_TASKS; slot++) {
+		if (proc[slot].p_magic != PMAGIC) {
+			printf("PROCFS: system version mismatch!\n");
 
-	for(i = 0; i < slots; i++) {
-		if(tab[i].mp_magic != MP_MAGIC) {
-			printf("procfs: mproc table magic number mismatch\n");
-			return 0;
+			return EINVAL;
 		}
 	}
 
-	return 1;
+	return OK;
 }
 
 /*===========================================================================*
- *				update_tables				     *
+ *				update_mproc_table			     *
  *===========================================================================*/
-PRIVATE int update_tables(void)
+PRIVATE int update_mproc_table(void)
 {
-	/* Get the process tables from the kernel, PM, and VFS.
-	 * Check the magic number in the kernel table entries.
+	/* Get the process table from PM.
+	 * Check the magic number in the table entries.
 	 */
 	int r, slot;
 
-	if ((r = sys_getproctab(proc)) != OK) return r;
+	r = getsysinfo(PM_PROC_NR, SI_PROC_TAB, mproc, sizeof(mproc));
+	if (r != OK) return r;
 
-	for (slot = 0; slot < NR_PROCS + NR_TASKS; slot++) {
-		if (proc[slot].p_magic != PMAGIC) {
-			printf("PROCFS: system version mismatch!\n");
+	for (slot = 0; slot < NR_PROCS; slot++) {
+		if (mproc[slot].mp_magic != MP_MAGIC) {
+			printf("PROCFS: PM version mismatch!\n");
 
 			return EINVAL;
 		}
 	}
 
-	if ((r = getsysinfo(PM_PROC_NR, SI_PROC_TAB, mproc)) != OK) return r;
+	return OK;
+}
+
+/*===========================================================================*
+ *				update_fproc_table			     *
+ *===========================================================================*/
+PRIVATE int update_fproc_table(void)
+{
+	/* Get the process table from VFS.
+	 */
+
+	return getsysinfo(VFS_PROC_NR, SI_PROC_TAB, fproc, sizeof(fproc));
+}
+
+/*===========================================================================*
+ *				update_tables				     *
+ *===========================================================================*/
+PRIVATE int update_tables(void)
+{
+	/* Get the process tables from the kernel, PM, and VFS.
+	 */
+	int r;
+
+	if ((r = update_proc_table()) != OK) return r;
 
-	if(!mproc_ok(mproc, NR_PROCS)) return EINVAL;
+	if ((r = update_mproc_table()) != OK) return r;
 
-	if ((r = getsysinfo(VFS_PROC_NR, SI_PROC_TAB, fproc)) != OK) return r;
+	if ((r = update_fproc_table()) != OK) return r;
 
 	return OK;
 }
diff --git a/servers/rs/request.c b/servers/rs/request.c
index f664f357d..6fd5d74cb 100755
--- a/servers/rs/request.c
+++ b/servers/rs/request.c
@@ -879,7 +879,7 @@ message *m_ptr;
   if((s = check_call_permission(m_ptr->m_source, 0, NULL)) != OK)
       return s;
 
-  switch(m_ptr->m1_i1) {
+  switch(m_ptr->SI_WHAT) {
   case SI_PROC_TAB:
   	src_addr = (vir_bytes) rproc;
   	len = sizeof(struct rproc) * NR_SYS_PROCS;
@@ -892,11 +892,12 @@ message *m_ptr;
   	return(EINVAL);
   }
 
+  if (len != m_ptr->SI_SIZE)
+	return(EINVAL);
+
   dst_proc = m_ptr->m_source;
-  dst_addr = (vir_bytes) m_ptr->m1_p1;
-  if (OK != (s=sys_datacopy(SELF, src_addr, dst_proc, dst_addr, len)))
-  	return(s);
-  return(OK);
+  dst_addr = (vir_bytes) m_ptr->SI_WHERE;
+  return sys_datacopy(SELF, src_addr, dst_proc, dst_addr, len);
 }
 
 /*===========================================================================*
diff --git a/servers/vfs/misc.c b/servers/vfs/misc.c
index e7be635c7..569b32202 100644
--- a/servers/vfs/misc.c
+++ b/servers/vfs/misc.c
@@ -65,7 +65,6 @@ PUBLIC int do_getsysinfo()
 {
   vir_bytes src_addr, dst_addr;
   size_t len;
-  int s;
 
   /* Only su may call do_getsysinfo. This call may leak information (and is not
    * stable enough to be part of the API/ABI). In the future, requests from
@@ -74,7 +73,7 @@ PUBLIC int do_getsysinfo()
 
   if (!super_user) return(EPERM);
 
-  switch(m_in.info_what) {
+  switch(m_in.SI_WHAT) {
   case SI_PROC_TAB:
   	src_addr = (vir_bytes) fproc;
   	len = sizeof(struct fproc) * NR_PROCS;
@@ -93,10 +92,11 @@ PUBLIC int do_getsysinfo()
   	return(EINVAL);
   }
 
-  dst_addr = (vir_bytes) m_in.info_where;
-  if (OK != (s = sys_datacopy(SELF, src_addr, who_e, dst_addr, len))) return(s);
-  return(OK);
+  if (len != m_in.SI_SIZE)
+	return(EINVAL);
 
+  dst_addr = (vir_bytes) m_in.SI_WHERE;
+  return sys_datacopy(SELF, src_addr, who_e, dst_addr, len);
 }
 
 /*===========================================================================*
diff --git a/servers/vfs/param.h b/servers/vfs/param.h
index e5be4a275..77a27f830 100644
--- a/servers/vfs/param.h
+++ b/servers/vfs/param.h
@@ -42,8 +42,6 @@
 #define whence	      m2_i2
 #define svrctl_req    m2_i1
 #define svrctl_argp   m2_p1
-#define info_what     m1_i1
-#define info_where    m1_p1
 #define md_label	m2_p1
 #define md_label_len	m2_l1
 #define md_major	m2_i1