From 0eabb93c0c1d18ee661e6ed51798f14d2dfd3f92 Mon Sep 17 00:00:00 2001 From: David van Moolenbroek Date: Fri, 19 Dec 2014 13:40:10 +0000 Subject: [PATCH] procfs: retrieve both RS tables from RS at once Previously, procfs would retrieve the rproc and rprocpub tables from RS in two separate calls. This allowed for a race condition where the tables could change in between the calls, resulting in a panic in procfs under certain circumstances. RS now implements a new method for getsysinfo that allows the retrieval of both tables at once. Change-Id: I5ec22d25898361270c90e805a43fc6d76ad9e29d --- minix/fs/procfs/service.c | 35 +++++++++++++++++++++-------------- minix/include/minix/sysinfo.h | 1 + minix/servers/rs/request.c | 21 +++++++++++++++++---- 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/minix/fs/procfs/service.c b/minix/fs/procfs/service.c index 852332a48..576472105 100644 --- a/minix/fs/procfs/service.c +++ b/minix/fs/procfs/service.c @@ -19,8 +19,11 @@ struct policies { enum policy supported; }; -static struct rprocpub rprocpub[NR_SYS_PROCS]; -static struct rproc rproc[NR_SYS_PROCS]; +static struct { + struct rproc proc[NR_SYS_PROCS]; + struct rprocpub pub[NR_SYS_PROCS]; +} rproc; + static struct policies policies[NR_SYS_PROCS]; static struct inode *service_node; @@ -132,9 +135,9 @@ service_get_policies(struct policies * pol, index_t slot) }; /* Find the related policy, based on the file name of the service. */ - ref_label = strrchr(rprocpub[slot].proc_name, '/'); + ref_label = strrchr(rproc.pub[slot].proc_name, '/'); if (NULL == ref_label) - ref_label = rprocpub[slot].proc_name; + ref_label = rproc.pub[slot].proc_name; memset(pol[slot].formatted, 0, MAX_POL_FORMAT_SZ); for(pos = 0; pos < (sizeof(def_pol) / sizeof(def_pol[0])); pos++) { @@ -161,11 +164,15 @@ service_update(void) struct inode *node; struct inode_stat stat; index_t slot; + static int warned = FALSE; + int r; - /* There is not much we can do if either of these calls fails. */ - (void)getsysinfo(RS_PROC_NR, SI_PROCPUB_TAB, rprocpub, - sizeof(rprocpub)); - (void)getsysinfo(RS_PROC_NR, SI_PROC_TAB, rproc, sizeof(rproc)); + /* There is not much we can do if this call fails. */ + r = getsysinfo(RS_PROC_NR, SI_PROCALL_TAB, &rproc, sizeof(rproc)); + if (r != OK && !warned) { + printf("PROCFS: unable to obtain RS tables (%d)\n", r); + warned = TRUE; + } /* * As with PIDs, we make two passes. Delete first, then add. This @@ -180,8 +187,8 @@ service_update(void) * If the slot is no longer in use, or the label name does not * match, the node must be deleted. */ - if (!(rproc[slot].r_flags & RS_IN_USE) || - strcmp(get_inode_name(node), rprocpub[slot].label)) + if (!(rproc.proc[slot].r_flags & RS_IN_USE) || + strcmp(get_inode_name(node), rproc.pub[slot].label)) delete_inode(node); } @@ -191,11 +198,11 @@ service_update(void) stat.gid = SUPER_USER; for (slot = 0; slot < NR_SYS_PROCS; slot++) { - if (!(rproc[slot].r_flags & RS_IN_USE) || + if (!(rproc.proc[slot].r_flags & RS_IN_USE) || get_inode_by_index(service_node, slot) != NULL) continue; - node = add_inode(service_node, rprocpub[slot].label, slot, + node = add_inode(service_node, rproc.pub[slot].label, slot, &stat, (index_t)0, (cbdata_t)slot); if (node == NULL) @@ -277,8 +284,8 @@ service_read(struct inode * node) return; slot = get_inode_index(node); - rpub = &rprocpub[slot]; - rp = &rproc[slot]; + rpub = &rproc.pub[slot]; + rp = &rproc.proc[slot]; /* TODO: add a large number of other fields! */ buf_printf("filename: %s\n", rpub->proc_name); diff --git a/minix/include/minix/sysinfo.h b/minix/include/minix/sysinfo.h index e63bb006e..1e4e2bbf9 100644 --- a/minix/include/minix/sysinfo.h +++ b/minix/include/minix/sysinfo.h @@ -13,6 +13,7 @@ int getsysinfo(endpoint_t who, int what, void *where, size_t size); #define SI_DATA_STORE 5 /* get copy of data store mappings */ #define SI_CALL_STATS 9 /* system call statistics */ #define SI_PROCPUB_TAB 11 /* copy of public entries of process table */ +#define SI_PROCALL_TAB 12 /* copy of both private and public entries */ #endif diff --git a/minix/servers/rs/request.c b/minix/servers/rs/request.c index c951d9454..39a9981b9 100755 --- a/minix/servers/rs/request.c +++ b/minix/servers/rs/request.c @@ -867,18 +867,33 @@ message *m_ptr; { vir_bytes src_addr, dst_addr; int dst_proc; - size_t len; + size_t size, len; int s; /* Check if the call can be allowed. */ if((s = check_call_permission(m_ptr->m_source, 0, NULL)) != OK) return s; + dst_proc = m_ptr->m_source; + dst_addr = m_ptr->m_lsys_getsysinfo.where; + size = m_ptr->m_lsys_getsysinfo.size; + switch(m_ptr->m_lsys_getsysinfo.what) { case SI_PROC_TAB: src_addr = (vir_bytes) rproc; len = sizeof(struct rproc) * NR_SYS_PROCS; break; + case SI_PROCALL_TAB: + /* Copy out both tables, one after the other. */ + src_addr = (vir_bytes) rproc; + len = sizeof(struct rproc) * NR_SYS_PROCS; + if (len > size) + return EINVAL; + if ((s = sys_datacopy(SELF, src_addr, dst_proc, dst_addr, len)) != OK) + return s; + dst_addr += len; + size -= len; + /* FALLTHROUGH */ case SI_PROCPUB_TAB: src_addr = (vir_bytes) rprocpub; len = sizeof(struct rprocpub) * NR_SYS_PROCS; @@ -887,11 +902,9 @@ message *m_ptr; return(EINVAL); } - if (len != m_ptr->m_lsys_getsysinfo.size) + if (len != size) return(EINVAL); - dst_proc = m_ptr->m_source; - dst_addr = m_ptr->m_lsys_getsysinfo.where; return sys_datacopy(SELF, src_addr, dst_proc, dst_addr, len); }