From 4d272e5a970c4e7d6700cabd52e337e37404b70e Mon Sep 17 00:00:00 2001 From: David van Moolenbroek Date: Tue, 24 Nov 2015 23:01:22 +0000 Subject: [PATCH] IPC server: NetBSD sync, general improvements - switch to the NetBSD identifier system; it is not only better, but also required for porting NetBSD ipcs(1) and ipcrm(1); however, it requires that slots not be moved, and that results in some changes; - synchronize some other things with NetBSD: where keys are kept, as well as various non-permission mode flags; - fix semctl(2) vararg retrieval and message field type; - use SUSPEND instead of weird reply exceptions in the call table; - fix several memory leaks and at least one missing permission check; - improve the atomicity of semop(2) by a small amount, even though its atomicity is still broken at a fundamental level; - use the new cheaper way to retrieve the current time; - resolve all level-5 LLVM warnings. Change-Id: I0c47aacde478b23bb77d628384aeab855a22fdbf --- minix/include/minix/ipc.h | 2 +- minix/lib/libc/sys/sem.c | 22 ++- minix/servers/ipc/Makefile | 2 + minix/servers/ipc/inc.h | 12 +- minix/servers/ipc/main.c | 70 +++----- minix/servers/ipc/sem.c | 254 ++++++++++++++-------------- minix/servers/ipc/shm.c | 264 ++++++++++++++++++------------ minix/usr.bin/trace/service/ipc.c | 2 +- sys/sys/sem.h | 2 + sys/sys/shm.h | 8 +- 10 files changed, 353 insertions(+), 285 deletions(-) diff --git a/minix/include/minix/ipc.h b/minix/include/minix/ipc.h index 0934614c7..b4c18b7e0 100644 --- a/minix/include/minix/ipc.h +++ b/minix/include/minix/ipc.h @@ -365,7 +365,7 @@ typedef struct { int id; int num; int cmd; - int opt; + vir_bytes opt; int ret; uint8_t padding[36]; } mess_lc_ipc_semctl; diff --git a/minix/lib/libc/sys/sem.c b/minix/lib/libc/sys/sem.c index 7d78e22aa..958b11355 100644 --- a/minix/lib/libc/sys/sem.c +++ b/minix/lib/libc/sys/sem.c @@ -1,4 +1,3 @@ -#define __USE_MISC #define _SYSTEM 1 #define _MINIX_SYSTEM 1 @@ -64,10 +63,23 @@ int semctl(int semid, int semnum, int cmd, ...) m.m_lc_ipc_semctl.num = semnum; m.m_lc_ipc_semctl.cmd = cmd; va_start(ap, cmd); - if (cmd == IPC_STAT || cmd == IPC_SET || cmd == IPC_INFO || - cmd == SEM_INFO || cmd == SEM_STAT || cmd == GETALL || - cmd == SETALL || cmd == SETVAL) - m.m_lc_ipc_semctl.opt = (long) va_arg(ap, long); + switch (cmd) { + case SETVAL: + m.m_lc_ipc_semctl.opt = (vir_bytes)va_arg(ap, int); + break; + case IPC_STAT: + case IPC_SET: + case IPC_INFO: + case SEM_INFO: + case SEM_STAT: + case GETALL: + case SETALL: + m.m_lc_ipc_semctl.opt = (vir_bytes)va_arg(ap, void *); + break; + default: + m.m_lc_ipc_semctl.opt = 0; + break; + } va_end(ap); r = _syscall(ipc_pt, IPC_SEMCTL, &m); diff --git a/minix/servers/ipc/Makefile b/minix/servers/ipc/Makefile index e26053c50..ba2888fd5 100644 --- a/minix/servers/ipc/Makefile +++ b/minix/servers/ipc/Makefile @@ -9,4 +9,6 @@ FILES=ipc.conf FILESNAME=ipc FILESDIR= /etc/system.conf.d +WARNS?= 5 + .include diff --git a/minix/servers/ipc/inc.h b/minix/servers/ipc/inc.h index 833232aae..1a41048c0 100644 --- a/minix/servers/ipc/inc.h +++ b/minix/servers/ipc/inc.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -27,6 +28,15 @@ #include #include #include +#include + +/* + * On NetBSD, these macros are only defined when _KERNEL is set. However, + * since ipcs(1) uses IXSEQ_TO_IPCID, NetBSD cannot change these macros without + * breaking the userland API. Thus, having a copy of them here is not risky. + */ +#define IPCID_TO_IX(id) ((id) & 0xffff) +#define IPCID_TO_SEQ(id) (((id) >> 16) & 0xffff) int do_shmget(message *); int do_shmat(message *); @@ -41,6 +51,4 @@ int is_sem_nil(void); int is_shm_nil(void); void sem_process_vm_notify(void); -EXTERN int identifier; EXTERN endpoint_t who_e; -EXTERN int call_type; diff --git a/minix/servers/ipc/main.c b/minix/servers/ipc/main.c index 3e143158a..46d902ab7 100644 --- a/minix/servers/ipc/main.c +++ b/minix/servers/ipc/main.c @@ -1,25 +1,19 @@ #include "inc.h" -int identifier = 0x1234; endpoint_t who_e; -int call_type; - -static struct { - int type; - int (*func)(message *); - int reply; /* whether the reply action is passed through */ -} ipc_calls[] = { - { IPC_SHMGET, do_shmget, 0 }, - { IPC_SHMAT, do_shmat, 0 }, - { IPC_SHMDT, do_shmdt, 0 }, - { IPC_SHMCTL, do_shmctl, 0 }, - { IPC_SEMGET, do_semget, 0 }, - { IPC_SEMCTL, do_semctl, 0 }, - { IPC_SEMOP, do_semop, 1 }, +static unsigned int call_type; + +#define CALL(n) [((n) - IPC_BASE)] +static int (* const call_vec[])(message *) = { + CALL(IPC_SHMGET) = do_shmget, + CALL(IPC_SHMAT) = do_shmat, + CALL(IPC_SHMDT) = do_shmdt, + CALL(IPC_SHMCTL) = do_shmctl, + CALL(IPC_SEMGET) = do_semget, + CALL(IPC_SEMCTL) = do_semctl, + CALL(IPC_SEMOP) = do_semop, }; -#define SIZE(a) (sizeof(a)/sizeof(a[0])) - static int verbose = 0; /* SEF functions and variables. */ @@ -30,15 +24,14 @@ static void sef_cb_signal_handler(int signo); int main(int argc, char *argv[]) { message m; + unsigned int ipc_number; + int r; /* SEF local startup. */ env_setargs(argc, argv); sef_local_startup(); while (TRUE) { - int r; - int ipc_number; - if ((r = sef_receive(ANY, &m)) != OK) printf("sef_receive failed %d.\n", r); who_e = m.m_source; @@ -61,22 +54,11 @@ int main(int argc, char *argv[]) continue; } - /* - * The ipc number in the table can be obtained - * with a simple equation because the values of - * IPC system calls are consecutive and begin - * at ( IPC_BASE + 1 ) - */ - - ipc_number = call_type - (IPC_BASE + 1); + ipc_number = (unsigned int)(call_type - IPC_BASE); /* dispatch message */ - if (ipc_number >= 0 && ipc_number < SIZE(ipc_calls)) { - int result; - - if (ipc_calls[ipc_number].type != call_type) - panic("IPC: call table order mismatch"); - + if (ipc_number < __arraycount(call_vec) && + call_vec[ipc_number] != NULL) { /* If any process does an IPC call, * we have to know about it exiting. * Tell VM to watch it for us. @@ -85,19 +67,18 @@ int main(int argc, char *argv[]) printf("IPC: watch failed on %d\n", m.m_source); } - result = ipc_calls[ipc_number].func(&m); + r = call_vec[ipc_number](&m); /* * The handler of the IPC call did not * post a reply. */ - if (!ipc_calls[ipc_number].reply) { - - m.m_type = result; + if (r != SUSPEND) { + m.m_type = r; - if(verbose && result != OK) + if(verbose && r != OK) printf("IPC: error for %d: %d\n", - call_type, result); + call_type, r); if ((r = ipc_sendnb(who_e, &m)) != OK) printf("IPC send error %d.\n", r); @@ -117,7 +98,7 @@ int main(int argc, char *argv[]) /*===========================================================================* * sef_local_startup * *===========================================================================*/ -static void sef_local_startup() +static void sef_local_startup(void) { /* Register init callbacks. */ sef_setcb_init_fresh(sef_cb_init_fresh); @@ -152,7 +133,8 @@ static void sef_cb_signal_handler(int signo) if (signo != SIGTERM) return; /* Checkout if there are still IPC keys. Inform the user in that case. */ - if (!is_sem_nil() || !is_shm_nil()) - printf("IPC: exit with un-clean states.\n"); -} + if (is_sem_nil() && is_shm_nil()) + sef_exit(0); + printf("IPC: exit with un-clean states.\n"); +} diff --git a/minix/servers/ipc/sem.c b/minix/servers/ipc/sem.c index e5cf7706e..cabf50726 100644 --- a/minix/servers/ipc/sem.c +++ b/minix/servers/ipc/sem.c @@ -1,7 +1,3 @@ -#define __USE_MISC - -#include - #include "inc.h" struct waiting { @@ -19,33 +15,43 @@ struct semaphore { }; struct sem_struct { - key_t key; - int id; struct semid_ds semid_ds; struct semaphore sems[SEMMSL]; }; static struct sem_struct sem_list[SEMMNI]; -static int sem_list_nr = 0; +static unsigned int sem_list_nr = 0; /* highest in-use slot number plus one */ static struct sem_struct *sem_find_key(key_t key) { - int i; + unsigned int i; + if (key == IPC_PRIVATE) return NULL; - for (i = 0; i < sem_list_nr; i++) - if (sem_list[i].key == key) - return sem_list+i; + for (i = 0; i < sem_list_nr; i++) { + if (!(sem_list[i].semid_ds.sem_perm.mode & SEM_ALLOC)) + continue; + if (sem_list[i].semid_ds.sem_perm._key == key) + return &sem_list[i]; + } return NULL; } static struct sem_struct *sem_find_id(int id) { - int i; - for (i = 0; i < sem_list_nr; i++) - if (sem_list[i].id == id) - return sem_list+i; - return NULL; + struct sem_struct *sem; + unsigned int i; + + i = IPCID_TO_IX(id); + if (i >= sem_list_nr) + return NULL; + + sem = &sem_list[i]; + if (!(sem->semid_ds.sem_perm.mode & SEM_ALLOC)) + return NULL; + if (sem->semid_ds.sem_perm._seq != IPCID_TO_SEQ(id)) + return NULL; + return sem; } /*===========================================================================* @@ -53,9 +59,10 @@ static struct sem_struct *sem_find_id(int id) *===========================================================================*/ int do_semget(message *m) { - key_t key; - int nsems, flag, id; struct sem_struct *sem; + unsigned int i, seq; + key_t key; + int nsems, flag; key = m->m_lc_ipc_semget.key; nsems = m->m_lc_ipc_semget.nr; @@ -68,33 +75,41 @@ int do_semget(message *m) return EACCES; if (nsems > sem->semid_ds.sem_nsems) return EINVAL; - id = sem->id; + i = sem - sem_list; } else { if (!(flag & IPC_CREAT)) return ENOENT; if (nsems < 0 || nsems >= SEMMSL) return EINVAL; - if (sem_list_nr == SEMMNI) + + /* Find a free entry. */ + for (i = 0; i < __arraycount(sem_list); i++) + if (!(sem_list[i].semid_ds.sem_perm.mode & SEM_ALLOC)) + break; + if (i == __arraycount(sem_list)) return ENOSPC; - /* create a new semaphore set */ - sem = &sem_list[sem_list_nr]; + /* Initialize the entry. */ + sem = &sem_list[i]; + seq = sem->semid_ds.sem_perm._seq; memset(sem, 0, sizeof(struct sem_struct)); + sem->semid_ds.sem_perm._key = key; sem->semid_ds.sem_perm.cuid = sem->semid_ds.sem_perm.uid = getnuid(who_e); sem->semid_ds.sem_perm.cgid = sem->semid_ds.sem_perm.gid = getngid(who_e); - sem->semid_ds.sem_perm.mode = flag & 0777; + sem->semid_ds.sem_perm.mode = SEM_ALLOC | (flag & ACCESSPERMS); + sem->semid_ds.sem_perm._seq = (seq + 1) & 0x7fff; sem->semid_ds.sem_nsems = nsems; sem->semid_ds.sem_otime = 0; - sem->semid_ds.sem_ctime = time(NULL); - sem->id = id = identifier++; - sem->key = key; + sem->semid_ds.sem_ctime = clock_time(NULL); - sem_list_nr++; + assert(i <= sem_list_nr); + if (i == sem_list_nr) + sem_list_nr++; } - m->m_lc_ipc_semget.retid = id; + m->m_lc_ipc_semget.retid = IXSEQ_TO_IPCID(i, sem->semid_ds.sem_perm); return OK; } @@ -119,22 +134,29 @@ static void remove_semaphore(struct sem_struct *sem) free(sem->sems[i].nlist); } - for (i = 0; i < sem_list_nr; i++) { - if (&sem_list[i] == sem) - break; - } + /* Mark the entry as free. */ + sem->semid_ds.sem_perm.mode &= ~SEM_ALLOC; - if (i < sem_list_nr && --sem_list_nr != i) - sem_list[i] = sem_list[sem_list_nr]; + /* + * This may have been the last in-use slot in the list. Ensure that + * sem_list_nr again equals the highest in-use slot number plus one. + */ + while (sem_list_nr > 0 && + !(sem_list[sem_list_nr - 1].semid_ds.sem_perm.mode & SEM_ALLOC)) + sem_list_nr--; } #if 0 static void show_semaphore(void) { - int i, j, k; + unsigned int i; + int j, k, nr; for (i = 0; i < sem_list_nr; i++) { - int nr = sem_list[i].semid_ds.sem_nsems; + if (!(sem_list[i].semid_ds.sem_perm.mode & SEM_ALLOC)) + continue; + + nr = sem_list[i].semid_ds.sem_nsems; printf("===== [%d] =====\n", i); for (j = 0; j < nr; j++) { @@ -166,13 +188,16 @@ static void show_semaphore(void) static void remove_process(endpoint_t pt) { - int i; + struct sem_struct *sem; + unsigned int i; + int j, nr; for (i = 0; i < sem_list_nr; i++) { - struct sem_struct *sem = &sem_list[i]; - int nr = sem->semid_ds.sem_nsems; - int j; + sem = &sem_list[i]; + if (!(sem->semid_ds.sem_perm.mode & SEM_ALLOC)) + continue; + nr = sem->semid_ds.sem_nsems; for (j = 0; j < nr; j++) { struct semaphore *semaphore = &sem->sems[j]; int k; @@ -268,10 +293,13 @@ static void update_one_semaphore(struct sem_struct *sem, int is_remove) static void update_semaphores(void) { - int i; + unsigned int i; - for (i = 0; i < sem_list_nr; i++) - update_one_semaphore(sem_list+i, 0 /* not remove */); + for (i = 0; i < sem_list_nr; i++) { + if (!(sem_list[i].semid_ds.sem_perm.mode & SEM_ALLOC)) + continue; + update_one_semaphore(&sem_list[i], FALSE /*is_remove*/); + } } /*===========================================================================* @@ -279,10 +307,10 @@ static void update_semaphores(void) *===========================================================================*/ int do_semctl(message *m) { - int r, i; - long opt = 0; + unsigned int i; + vir_bytes opt; uid_t uid; - int id, num, cmd, val; + int r, id, num, cmd, val; unsigned short *buf; struct semid_ds *ds, tmp_ds; struct sem_struct *sem; @@ -291,11 +319,7 @@ int do_semctl(message *m) id = m->m_lc_ipc_semctl.id; num = m->m_lc_ipc_semctl.num; cmd = m->m_lc_ipc_semctl.cmd; - - if (cmd == IPC_STAT || cmd == IPC_SET || cmd == IPC_INFO || - cmd == SEM_INFO || cmd == SEM_STAT || cmd == GETALL || - cmd == SETALL || cmd == SETVAL) - opt = m->m_lc_ipc_semctl.opt; + opt = m->m_lc_ipc_semctl.opt; switch (cmd) { case IPC_INFO: @@ -303,13 +327,16 @@ int do_semctl(message *m) sem = NULL; break; case SEM_STAT: - if (id < 0 || id >= sem_list_nr) + if (id < 0 || (unsigned int)id >= sem_list_nr) return EINVAL; sem = &sem_list[id]; + if (!(sem->semid_ds.sem_perm.mode & SEM_ALLOC)) + return EINVAL; break; default: if (!(sem = sem_find_id(id))) return EINVAL; + break; } /* IPC_SET and IPC_RMID as its own permission check */ @@ -325,7 +352,9 @@ int do_semctl(message *m) if ((r = sys_datacopy(SELF, (vir_bytes)&sem->semid_ds, who_e, (vir_bytes)opt, sizeof(sem->semid_ds))) != OK) return r; - m->m_lc_ipc_semctl.ret = sem->id; + if (cmd == SEM_STAT) + m->m_lc_ipc_semctl.ret = + IXSEQ_TO_IPCID(id, sem->semid_ds.sem_perm); break; case IPC_SET: uid = getnuid(who_e); @@ -334,15 +363,15 @@ int do_semctl(message *m) uid != 0) return EPERM; ds = (struct semid_ds *) opt; - r = sys_datacopy(who_e, (vir_bytes) ds, - SELF, (vir_bytes) &tmp_ds, sizeof(struct semid_ds)); - if (r != OK) - return EINVAL; + if ((r = sys_datacopy(who_e, (vir_bytes)ds, SELF, + (vir_bytes)&tmp_ds, sizeof(struct semid_ds))) != OK) + return r; sem->semid_ds.sem_perm.uid = tmp_ds.sem_perm.uid; sem->semid_ds.sem_perm.gid = tmp_ds.sem_perm.gid; - sem->semid_ds.sem_perm.mode &= ~0777; - sem->semid_ds.sem_perm.mode |= tmp_ds.sem_perm.mode & 0666; - sem->semid_ds.sem_ctime = time(NULL); + sem->semid_ds.sem_perm.mode &= ~ACCESSPERMS; + sem->semid_ds.sem_perm.mode |= + tmp_ds.sem_perm.mode & ACCESSPERMS; + sem->semid_ds.sem_ctime = clock_time(NULL); break; case IPC_RMID: uid = getnuid(who_e); @@ -353,7 +382,7 @@ int do_semctl(message *m) /* awaken all processes block in semop * and remove the semaphore set. */ - update_one_semaphore(sem, 1); + update_one_semaphore(sem, TRUE /*is_remove*/); break; case IPC_INFO: case SEM_INFO: @@ -395,16 +424,15 @@ int do_semctl(message *m) break; case GETALL: buf = malloc(sizeof(unsigned short) * sem->semid_ds.sem_nsems); - if (!buf) + if (buf == NULL) return ENOMEM; for (i = 0; i < sem->semid_ds.sem_nsems; i++) buf[i] = sem->sems[i].semval; - r = sys_datacopy(SELF, (vir_bytes) buf, - who_e, (vir_bytes) opt, - sizeof(unsigned short) * sem->semid_ds.sem_nsems); + r = sys_datacopy(SELF, (vir_bytes)buf, who_e, (vir_bytes)opt, + sizeof(unsigned short) * sem->semid_ds.sem_nsems); + free(buf); if (r != OK) return EINVAL; - free(buf); break; case GETNCNT: if (num < 0 || num >= sem->semid_ds.sem_nsems) @@ -428,13 +456,14 @@ int do_semctl(message *m) break; case SETALL: buf = malloc(sizeof(unsigned short) * sem->semid_ds.sem_nsems); - if (!buf) + if (buf == NULL) return ENOMEM; - r = sys_datacopy(who_e, (vir_bytes) opt, - SELF, (vir_bytes) buf, - sizeof(unsigned short) * sem->semid_ds.sem_nsems); - if (r != OK) + r = sys_datacopy(who_e, (vir_bytes)opt, SELF, (vir_bytes)buf, + sizeof(unsigned short) * sem->semid_ds.sem_nsems); + if (r != OK) { + free(buf); return EINVAL; + } #ifdef DEBUG_SEM printf("SEMCTL: SETALL: opt: %lu\n", (vir_bytes) opt); for (i = 0; i < sem->semid_ds.sem_nsems; i++) @@ -465,7 +494,7 @@ int do_semctl(message *m) #ifdef DEBUG_SEM printf("SEMCTL: SETVAL: %d %d\n", num, val); #endif - sem->semid_ds.sem_ctime = time(NULL); + sem->semid_ds.sem_ctime = clock_time(NULL); /* awaken if possible */ update_semaphores(); break; @@ -481,7 +510,8 @@ int do_semctl(message *m) *===========================================================================*/ int do_semop(message *m) { - int id, i, j, r; + unsigned int i, j, mask; + int id, r; struct sembuf *sops; unsigned int nsops; struct sem_struct *sem; @@ -490,34 +520,23 @@ int do_semop(message *m) id = m->m_lc_ipc_semop.id; nsops = m->m_lc_ipc_semop.size; - r = EINVAL; if (!(sem = sem_find_id(id))) - goto out; + return EINVAL; if (nsops <= 0) - goto out; - - r = E2BIG; + return EINVAL; if (nsops > SEMOPM) - goto out; - - /* check for read permission */ - r = EACCES; - if (!check_perm(&sem->semid_ds.sem_perm, who_e, 0444)) - goto out; + return E2BIG; /* get the array from user application */ - r = ENOMEM; sops = malloc(sizeof(struct sembuf) * nsops); if (!sops) - goto out_free; + return ENOMEM; r = sys_datacopy(who_e, (vir_bytes) m->m_lc_ipc_semop.ops, SELF, (vir_bytes) sops, sizeof(struct sembuf) * nsops); - if (r != OK) { - r = EINVAL; + if (r != OK) goto out_free; - } #ifdef DEBUG_SEM for (i = 0; i < nsops; i++) @@ -530,6 +549,18 @@ int do_semop(message *m) if (sops[i].sem_num >= sem->semid_ds.sem_nsems) goto out_free; + /* check for permissions */ + r = EACCES; + mask = 0; + for (i = 0; i < nsops; i++) { + if (sops[i].sem_op != 0) + mask |= 0222; /* check for write permission */ + else + mask |= 0444; /* check for read permission */ + } + if (mask && !check_perm(&sem->semid_ds.sem_perm, who_e, mask)) + goto out_free; + /* check for duplicate number */ r = EINVAL; for (i = 0; i < nsops; i++) @@ -537,7 +568,7 @@ int do_semop(message *m) if (sops[i].sem_num == sops[j].sem_num) goto out_free; - /* check for EAGAIN error */ + /* check for nonblocking operations */ r = EAGAIN; for (i = 0; i < nsops; i++) { int op_n, val; @@ -550,8 +581,8 @@ int do_semop(message *m) (op_n < 0 && -op_n > val))) goto out_free; - } + /* there will be no errors left, so we can go ahead */ for (i = 0; i < nsops; i++) { struct semaphore *s; @@ -563,20 +594,16 @@ int do_semop(message *m) s->sempid = getnpid(who_e); if (op_n > 0) { - /* check for alter permission */ - r = EACCES; - if (!check_perm(&sem->semid_ds.sem_perm, who_e, 0222)) - goto out_free; s->semval += sops[i].sem_op; } else if (!op_n) { if (s->semval) { /* put the process asleep */ s->semzcnt++; - s->zlist = realloc(s->zlist, sizeof(struct waiting) * s->semzcnt); - if (!s->zlist) { - printf("IPC: zero waiting list lost...\n"); - break; - } + s->zlist = realloc(s->zlist, + sizeof(struct waiting) * s->semzcnt); + /* continuing if NULL would lead to disaster */ + if (s->zlist == NULL) + panic("out of memory"); s->zlist[s->semzcnt-1].who = who_e; s->zlist[s->semzcnt-1].val = op_n; @@ -586,20 +613,16 @@ int do_semop(message *m) no_reply++; } } else { - /* check for alter permission */ - r = EACCES; - if (!check_perm(&sem->semid_ds.sem_perm, who_e, 0222)) - goto out_free; if (s->semval >= -op_n) s->semval += op_n; else { /* put the process asleep */ s->semncnt++; - s->nlist = realloc(s->nlist, sizeof(struct waiting) * s->semncnt); - if (!s->nlist) { - printf("IPC: increase waiting list lost...\n"); - break; - } + s->nlist = realloc(s->nlist, + sizeof(struct waiting) * s->semncnt); + /* continuing if NULL would lead to disaster */ + if (s->nlist == NULL) + panic("out of memory"); s->nlist[s->semncnt-1].who = who_e; s->nlist[s->semncnt-1].val = -op_n; @@ -608,23 +631,14 @@ int do_semop(message *m) } } - r = OK; + r = no_reply ? SUSPEND : OK; out_free: free(sops); -out: - /* if we reach here by errors - * or with no errors but we should reply back. - */ - if (r != OK || !no_reply) { - m->m_type = r; - - ipc_sendnb(who_e, m); - } /* awaken process if possible */ update_semaphores(); - return 0; + return r; } /*===========================================================================* diff --git a/minix/servers/ipc/shm.c b/minix/servers/ipc/shm.c index 3ce9568db..edd351938 100644 --- a/minix/servers/ipc/shm.c +++ b/minix/servers/ipc/shm.c @@ -1,35 +1,46 @@ #include "inc.h" -#define MAX_SHM_NR 1024 +/* Private shm_perm.mode flags, synchronized with NetBSD kernel values */ +#define SHM_ALLOC 0x0800 /* slot is in use (SHMSEG_ALLOCATED) */ struct shm_struct { - key_t key; - int id; struct shmid_ds shmid_ds; vir_bytes page; - int vm_id; + phys_bytes vm_id; }; -static struct shm_struct shm_list[MAX_SHM_NR]; -static int shm_list_nr = 0; +static struct shm_struct shm_list[SHMMNI]; +static unsigned int shm_list_nr = 0; /* highest in-use slot number plus one */ static struct shm_struct *shm_find_key(key_t key) { - int i; + unsigned int i; + if (key == IPC_PRIVATE) return NULL; - for (i = 0; i < shm_list_nr; i++) - if (shm_list[i].key == key) - return shm_list+i; + for (i = 0; i < shm_list_nr; i++) { + if (!(shm_list[i].shmid_ds.shm_perm.mode & SHM_ALLOC)) + continue; + if (shm_list[i].shmid_ds.shm_perm._key == key) + return &shm_list[i]; + } return NULL; } static struct shm_struct *shm_find_id(int id) { - int i; - for (i = 0; i < shm_list_nr; i++) - if (shm_list[i].id == id) - return shm_list+i; - return NULL; + struct shm_struct *shm; + unsigned int i; + + i = IPCID_TO_IX(id); + if (i >= shm_list_nr) + return NULL; + + shm = &shm_list[i]; + if (!(shm->shmid_ds.shm_perm.mode & SHM_ALLOC)) + return NULL; + if (shm->shmid_ds.shm_perm._seq != IPCID_TO_SEQ(id)) + return NULL; + return shm; } /*===========================================================================* @@ -38,9 +49,11 @@ static struct shm_struct *shm_find_id(int id) int do_shmget(message *m) { struct shm_struct *shm; - long key, size, old_size; + unsigned int i, seq; + key_t key; + size_t size, old_size; int flag; - int id; + void *page; key = m->m_lc_ipc_shmget.key; old_size = size = m->m_lc_ipc_shmget.size; @@ -53,7 +66,7 @@ int do_shmget(message *m) return EEXIST; if (size && shm->shmid_ds.shm_segsz < size) return EINVAL; - id = shm->id; + i = shm - shm_list; } else { /* no key found */ if (!(flag & IPC_CREAT)) return ENOENT; @@ -65,39 +78,51 @@ int do_shmget(message *m) if (size <= 0) return EINVAL; - if (shm_list_nr == MAX_SHM_NR) - return ENOMEM; - /* TODO: shmmni should be changed... */ - if (identifier == SHMMNI) + /* Find a free entry. */ + for (i = 0; i < __arraycount(shm_list); i++) + if (!(shm_list[i].shmid_ds.shm_perm.mode & SHM_ALLOC)) + break; + if (i == __arraycount(shm_list)) return ENOSPC; - shm = &shm_list[shm_list_nr]; - memset(shm, 0, sizeof(struct shm_struct)); - shm->page = (vir_bytes) mmap(0, size, - PROT_READ|PROT_WRITE, MAP_ANON, -1, 0); - if (shm->page == (vir_bytes) MAP_FAILED) + + /* + * Allocate memory to share. For now, we store the page + * reference as a numerical value so as to avoid issues with + * live update. TODO: a proper solution. + */ + page = mmap(0, size, PROT_READ | PROT_WRITE, MAP_ANON, -1, 0); + if (page == MAP_FAILED) return ENOMEM; - shm->vm_id = vm_getphys(sef_self(), (void *) shm->page); - memset((void *)shm->page, 0, size); + memset(page, 0, size); + /* Initialize the entry. */ + shm = &shm_list[i]; + seq = shm->shmid_ds.shm_perm._seq; + memset(shm, 0, sizeof(struct shm_struct)); + + shm->shmid_ds.shm_perm._key = key; shm->shmid_ds.shm_perm.cuid = shm->shmid_ds.shm_perm.uid = getnuid(who_e); shm->shmid_ds.shm_perm.cgid = shm->shmid_ds.shm_perm.gid = getngid(who_e); - shm->shmid_ds.shm_perm.mode = flag & 0777; + shm->shmid_ds.shm_perm.mode = SHM_ALLOC | (flag & ACCESSPERMS); + shm->shmid_ds.shm_perm._seq = (seq + 1) & 0x7fff; shm->shmid_ds.shm_segsz = old_size; shm->shmid_ds.shm_atime = 0; shm->shmid_ds.shm_dtime = 0; - shm->shmid_ds.shm_ctime = time(NULL); + shm->shmid_ds.shm_ctime = clock_time(NULL); shm->shmid_ds.shm_cpid = getnpid(who_e); shm->shmid_ds.shm_lpid = 0; shm->shmid_ds.shm_nattch = 0; - shm->id = id = identifier++; - shm->key = key; + shm->page = (vir_bytes)page; + shm->vm_id = vm_getphys(sef_self(), page); - shm_list_nr++; + assert(i <= shm_list_nr); + if (i == shm_list_nr) + shm_list_nr++; } - m->m_lc_ipc_shmget.retid = id; + m->m_lc_ipc_shmget.retid = IXSEQ_TO_IPCID(i, shm->shmid_ds.shm_perm); return OK; } @@ -133,11 +158,11 @@ int do_shmat(message *m) return EACCES; ret = vm_remap(who_e, sef_self(), (void *)addr, (void *)shm->page, - shm->shmid_ds.shm_segsz); + shm->shmid_ds.shm_segsz); if (ret == MAP_FAILED) return ENOMEM; - shm->shmid_ds.shm_atime = time(NULL); + shm->shmid_ds.shm_atime = clock_time(NULL); shm->shmid_ds.shm_lpid = getnpid(who_e); /* nattach is updated lazily */ @@ -150,10 +175,13 @@ int do_shmat(message *m) *===========================================================================*/ void update_refcount_and_destroy(void) { - int i, j; + size_t size; + u8_t rc; + unsigned int i; - for (i = 0, j = 0; i < shm_list_nr; i++) { - u8_t rc; + for (i = 0; i < shm_list_nr; i++) { + if (!(shm_list[i].shmid_ds.shm_perm.mode & SHM_ALLOC)) + continue; rc = vm_getrefcount(sef_self(), (void *) shm_list[i].page); if (rc == (u8_t) -1) { @@ -162,19 +190,24 @@ void update_refcount_and_destroy(void) } shm_list[i].shmid_ds.shm_nattch = rc - 1; - if (shm_list[i].shmid_ds.shm_nattch || - !(shm_list[i].shmid_ds.shm_perm.mode & SHM_DEST)) { - if (i != j) - shm_list[j] = shm_list[i]; - j++; - } else { - int size = shm_list[i].shmid_ds.shm_segsz; + if (shm_list[i].shmid_ds.shm_nattch == 0 && + (shm_list[i].shmid_ds.shm_perm.mode & SHM_DEST)) { + size = shm_list[i].shmid_ds.shm_segsz; if (size % PAGE_SIZE) size += PAGE_SIZE - size % PAGE_SIZE; munmap((void *)shm_list[i].page, size); + /* Mark the entry as free. */ + shm_list[i].shmid_ds.shm_perm.mode &= ~SHM_ALLOC; } } - shm_list_nr = j; + + /* + * Now that we may have removed an arbitrary set of slots, ensure that + * shm_list_nr again equals the highest in-use slot number plus one. + */ + while (shm_list_nr > 0 && + !(shm_list[shm_list_nr - 1].shmid_ds.shm_perm.mode & SHM_ALLOC)) + shm_list_nr--; } /*===========================================================================* @@ -182,9 +215,10 @@ void update_refcount_and_destroy(void) *===========================================================================*/ int do_shmdt(message *m) { + struct shm_struct *shm; vir_bytes addr; phys_bytes vm_id; - int i; + unsigned int i; addr = (vir_bytes) m->m_lc_ipc_shmdt.addr; @@ -192,10 +226,13 @@ int do_shmdt(message *m) return EINVAL; for (i = 0; i < shm_list_nr; i++) { - if (shm_list[i].vm_id == vm_id) { - struct shm_struct *shm = &shm_list[i]; + shm = &shm_list[i]; - shm->shmid_ds.shm_atime = time(NULL); + if (!(shm->shmid_ds.shm_perm.mode & SHM_ALLOC)) + continue; + + if (shm->vm_id == vm_id) { + shm->shmid_ds.shm_atime = clock_time(NULL); shm->shmid_ds.shm_lpid = getnpid(who_e); /* nattch is updated lazily */ @@ -217,36 +254,56 @@ int do_shmdt(message *m) *===========================================================================*/ int do_shmctl(message *m) { - int id = m->m_lc_ipc_shmctl.id; - int cmd = m->m_lc_ipc_shmctl.cmd; - struct shmid_ds *ds = (struct shmid_ds *)m->m_lc_ipc_shmctl.buf; + struct shmid_ds *ds; struct shmid_ds tmp_ds; - struct shm_struct *shm = NULL; + struct shm_struct *shm; struct shminfo sinfo; struct shm_info s_info; + unsigned int i; uid_t uid; - int r, i; + int r, id, cmd; - if (cmd == IPC_STAT) + id = m->m_lc_ipc_shmctl.id; + cmd = m->m_lc_ipc_shmctl.cmd; + ds = (struct shmid_ds *)m->m_lc_ipc_shmctl.buf; + + /* + * For stat calls, sure that all information is up-to-date. Since this + * may free the slot, do this before mapping from ID to slot below. + */ + if (cmd == IPC_STAT || cmd == SHM_STAT) update_refcount_and_destroy(); - if ((cmd == IPC_STAT || - cmd == IPC_SET || - cmd == IPC_RMID) && - !(shm = shm_find_id(id))) - return EINVAL; + switch (cmd) { + case IPC_INFO: + case SHM_INFO: + shm = NULL; + break; + case SHM_STAT: + if (id < 0 || (unsigned int)id >= shm_list_nr) + return EINVAL; + shm = &shm_list[id]; + if (!(shm->shmid_ds.shm_perm.mode & SHM_ALLOC)) + return EINVAL; + break; + default: + if (!(shm = shm_find_id(id))) + return EINVAL; + break; + } switch (cmd) { case IPC_STAT: - if (!ds) - return EFAULT; - /* check whether it has read permission */ + case SHM_STAT: + /* Check whether the caller has read permission. */ if (!check_perm(&shm->shmid_ds.shm_perm, who_e, 0444)) return EACCES; - r = sys_datacopy(SELF, (vir_bytes)&shm->shmid_ds, - who_e, (vir_bytes)ds, sizeof(struct shmid_ds)); - if (r != OK) - return EFAULT; + if ((r = sys_datacopy(SELF, (vir_bytes)&shm->shmid_ds, who_e, + (vir_bytes)ds, sizeof(struct shmid_ds))) != OK) + return r; + if (cmd == SHM_STAT) + m->m_lc_ipc_shmctl.ret = + IXSEQ_TO_IPCID(id, shm->shmid_ds.shm_perm); break; case IPC_SET: uid = getnuid(who_e); @@ -254,15 +311,15 @@ int do_shmctl(message *m) uid != shm->shmid_ds.shm_perm.uid && uid != 0) return EPERM; - r = sys_datacopy(who_e, (vir_bytes)ds, - SELF, (vir_bytes)&tmp_ds, sizeof(struct shmid_ds)); - if (r != OK) - return EFAULT; + if ((r = sys_datacopy(who_e, (vir_bytes)ds, SELF, + (vir_bytes)&tmp_ds, sizeof(struct shmid_ds))) != OK) + return r; shm->shmid_ds.shm_perm.uid = tmp_ds.shm_perm.uid; shm->shmid_ds.shm_perm.gid = tmp_ds.shm_perm.gid; - shm->shmid_ds.shm_perm.mode &= ~0777; - shm->shmid_ds.shm_perm.mode |= tmp_ds.shm_perm.mode & 0666; - shm->shmid_ds.shm_ctime = time(NULL); + shm->shmid_ds.shm_perm.mode &= ~ACCESSPERMS; + shm->shmid_ds.shm_perm.mode |= + tmp_ds.shm_perm.mode & ACCESSPERMS; + shm->shmid_ds.shm_ctime = clock_time(NULL); break; case IPC_RMID: uid = getnuid(who_e); @@ -275,24 +332,22 @@ int do_shmctl(message *m) update_refcount_and_destroy(); break; case IPC_INFO: - if (!ds) - return EFAULT; + memset(&sinfo, 0, sizeof(sinfo)); sinfo.shmmax = (unsigned long) -1; sinfo.shmmin = 1; - sinfo.shmmni = MAX_SHM_NR; + sinfo.shmmni = __arraycount(shm_list); sinfo.shmseg = (unsigned long) -1; sinfo.shmall = (unsigned long) -1; - r = sys_datacopy(SELF, (vir_bytes)&sinfo, - who_e, (vir_bytes)ds, sizeof(struct shminfo)); - if (r != OK) - return EFAULT; - m->m_lc_ipc_shmctl.ret = (shm_list_nr - 1); - if (m->m_lc_ipc_shmctl.ret < 0) + if ((r = sys_datacopy(SELF, (vir_bytes)&sinfo, who_e, + (vir_bytes)ds, sizeof(sinfo))) != OK) + return r; + if (shm_list_nr > 0) + m->m_lc_ipc_shmctl.ret = shm_list_nr - 1; + else m->m_lc_ipc_shmctl.ret = 0; break; case SHM_INFO: - if (!ds) - return EFAULT; + memset(&s_info, 0, sizeof(s_info)); s_info.used_ids = shm_list_nr; s_info.shm_tot = 0; for (i = 0; i < shm_list_nr; i++) @@ -302,24 +357,14 @@ int do_shmctl(message *m) s_info.shm_swp = 0; s_info.swap_attempts = 0; s_info.swap_successes = 0; - r = sys_datacopy(SELF, (vir_bytes)&s_info, - who_e, (vir_bytes)ds, sizeof(struct shm_info)); - if (r != OK) - return EFAULT; - m->m_lc_ipc_shmctl.ret = shm_list_nr - 1; - if (m->m_lc_ipc_shmctl.ret < 0) + if ((r = sys_datacopy(SELF, (vir_bytes)&s_info, who_e, + (vir_bytes)ds, sizeof(s_info))) != OK) + return r; + if (shm_list_nr > 0) + m->m_lc_ipc_shmctl.ret = shm_list_nr - 1; + else m->m_lc_ipc_shmctl.ret = 0; break; - case SHM_STAT: - if (id < 0 || id >= shm_list_nr) - return EINVAL; - shm = &shm_list[id]; - r = sys_datacopy(SELF, (vir_bytes)&shm->shmid_ds, - who_e, (vir_bytes)ds, sizeof(struct shmid_ds)); - if (r != OK) - return EFAULT; - m->m_lc_ipc_shmctl.ret = shm->id; - break; default: return EINVAL; } @@ -329,13 +374,16 @@ int do_shmctl(message *m) #if 0 static void list_shm_ds(void) { - int i; + unsigned int i; printf("key\tid\tpage\n"); - for (i = 0; i < shm_list_nr; i++) + for (i = 0; i < shm_list_nr; i++) { + if (!(shm_list[i].shmid_ds.shm_perm.mode & SHM_ALLOC)) + continue; printf("%ld\t%d\t%lx\n", - shm_list[i].key, - shm_list[i].id, + shm_list[i].shmid_ds.shm_perm._key, + IXSEQ_TO_IPCID(i, shm_list[i].shmid_ds.shm_perm), shm_list[i].page); + } } #endif diff --git a/minix/usr.bin/trace/service/ipc.c b/minix/usr.bin/trace/service/ipc.c index d2984fc52..7be2a3491 100644 --- a/minix/usr.bin/trace/service/ipc.c +++ b/minix/usr.bin/trace/service/ipc.c @@ -326,7 +326,7 @@ ipc_semctl_out(struct trace_proc * proc, const message * m_out) return CT_DONE; case SETVAL: - put_value(proc, "val", "%d", m_out->m_lc_ipc_semctl.opt); + put_value(proc, "val", "%lu", m_out->m_lc_ipc_semctl.opt); return CT_DONE; diff --git a/sys/sys/sem.h b/sys/sys/sem.h index 01bb2cb6e..2ddcbca3f 100644 --- a/sys/sys/sem.h +++ b/sys/sys/sem.h @@ -187,11 +187,13 @@ struct sem_sysctl_info { /* actual size of an undo structure */ #define SEMUSZ (sizeof(struct sem_undo)+sizeof(struct sem_undo_entry)*SEMUME) +#ifndef __minix /* * Structures allocated in machdep.c */ extern struct seminfo seminfo; extern struct semid_ds *sema; /* semaphore id pool */ +#endif /* !__minix */ /* * Parameters to the semconfig system call diff --git a/sys/sys/shm.h b/sys/sys/shm.h index 157c52e47..6541fbd6c 100644 --- a/sys/sys/shm.h +++ b/sys/sys/shm.h @@ -215,12 +215,12 @@ struct shm_info unsigned long int swap_successes; }; -#define SHMMNI 4096 +#define SHMMNI 1024 #define SHMSEG 32 /* max shared segs per process */ -/* shm_mode upper byte flags */ -#define SHM_DEST 01000 /* segment will be destroyed on last detach */ -#define SHM_LOCKED 02000 /* segment will not be swapped */ +/* Public shm_perm.mode flags, synchronized with NetBSD kernel values above */ +#define SHM_DEST 0x0400 /* destroy on last detach (SHMSEG_REMOVED) */ +#define SHM_LOCKED 0x4000 /* pages will not be swapped (SHMSEG_WIRED) */ #endif /* defined(__minix) */ -- 2.44.0