Date: Wed, 17 Sep 2003 10:10:53 -0700 (PDT) From: Hrishikesh Dandekar <hdandeka@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 38192 for review Message-ID: <200309171710.h8HHArq2001172@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=38192 Change 38192 by hdandeka@hdandeka_yash on 2003/09/17 10:10:45 Integrate the POSIX semaphore related changes. Affected files ... .. //depot/projects/trustedbsd/mac/sys/kern/uipc_sem.c#10 integrate .. //depot/projects/trustedbsd/mac/sys/posix4/ksem.h#2 integrate Differences ... ==== //depot/projects/trustedbsd/mac/sys/kern/uipc_sem.c#10 (text+ko) ==== @@ -61,7 +61,7 @@ static struct ksem *sem_lookup_byname(const char *name); static int sem_create(struct thread *td, const char *name, struct ksem **ksret, mode_t mode, unsigned int value); -static void sem_free(struct ksem *ksnew); +static void sem_free(struct ksem *ksnew, int checkrefs); static int sem_perm(struct thread *td, struct ksem *ks); static void sem_enter(struct proc *p, struct ksem *ks); static int sem_leave(struct proc *p, struct ksem *ks); @@ -75,6 +75,8 @@ semid_t *idp); static int kern_sem_open(struct thread *td, int dir, const char *name, int oflag, mode_t mode, unsigned int value, semid_t *idp); +static int ksem_open_existing(struct thread *td, struct ksem *ks, int dir, + semid_t *idpu, semid_t *idpk); static int kern_sem_unlink(struct thread *td, const char *name); #ifndef SEM_MAX @@ -85,6 +87,8 @@ #define SEM_TO_ID(x) ((intptr_t)(x)) #define ID_TO_SEM(x) id_to_sem(x) +#define SEM_FREE(ks) sem_free(ks, 1) +#define SEM_DROP(ks) sem_free(ks, 0) #ifndef MAC struct kuser { @@ -104,6 +108,8 @@ struct cv ks_cv; /* waiters sleep here */ int ks_waiters; /* number of waiters */ LIST_HEAD(, kuser) ks_users; /* pids using this sem */ + struct mtx ks_mtx; /* mutex protecting this semaphore */ + int ks_unlinked; /* Whether the named sem is unlinked */ }; #else struct kuser; @@ -115,10 +121,6 @@ * created via sem_open that have not yet been unlinked. */ LIST_HEAD(, ksem) ksem_head = LIST_HEAD_INITIALIZER(&ksem_head); -/* - * semaphores still in use but have been sem_unlink()'d go here. - */ -LIST_HEAD(, ksem) ksem_deadhead = LIST_HEAD_INITIALIZER(&ksem_deadhead); static struct mtx sem_lock; static MALLOC_DEFINE(M_SEM, "sems", "semaphore data"); @@ -129,6 +131,9 @@ static eventhandler_tag sem_exit_tag, sem_exec_tag; +#ifndef SEM_DEBUG +#define SEM_DEBUG +#endif #ifdef SEM_DEBUG #define DP(x) printf x #else @@ -139,7 +144,7 @@ void sem_ref(struct ksem *ks) { - + mtx_assert(&sem_lock, MA_OWNED); ks->ks_ref++; DP(("sem_ref: ks = %p, ref = %d\n", ks, ks->ks_ref)); } @@ -149,9 +154,9 @@ sem_rel(struct ksem *ks) { + mtx_assert(&sem_lock, MA_OWNED); + ks->ks_ref--; DP(("sem_rel: ks = %p, ref = %d\n", ks, ks->ks_ref - 1)); - if (--ks->ks_ref == 0) - sem_free(ks); } static __inline struct ksem *id_to_sem(semid_t id); @@ -163,6 +168,7 @@ { struct ksem *ks; + mtx_assert(&sem_lock,MA_OWNED); DP(("id_to_sem: id = %0x,%p\n", id, (struct ksem *)id)); LIST_FOREACH(ks, &ksem_head, ks_entry) { DP(("id_to_sem: ks = %p\n", ks)); @@ -178,12 +184,15 @@ { struct ksem *ks; + mtx_assert(&sem_lock, MA_OWNED); LIST_FOREACH(ks, &ksem_head, ks_entry) if (ks->ks_name != NULL && strcmp(ks->ks_name, name) == 0) return (ks); return (NULL); } +/* Used by both sem_init and sem_open to create a new semaphore. */ + static int sem_create(td, name, ksret, mode, value) struct thread *td; @@ -196,14 +205,15 @@ struct proc *p; struct ucred *uc; size_t len; - int error; DP(("sem_create\n")); p = td->td_proc; uc = td->td_ucred; + /* XXX Use p31b_getcfg(CTL_P1003_1B_SEM_VALUE_MAX) instead? */ if (value > SEM_VALUE_MAX) return (EINVAL); - ret = malloc(sizeof(*ret), M_SEM, M_WAITOK | M_ZERO); + if( (ret = malloc(sizeof(*ret), M_SEM, M_WAITOK | M_ZERO)) == NULL) + return (ENOMEM); if (name != NULL) { len = strlen(name); if (len > SEM_MAX_NAMELEN) { @@ -215,39 +225,44 @@ free(ret, M_SEM); return (EINVAL); } - ret->ks_name = malloc(len + 1, M_SEM, M_WAITOK); + if( (ret->ks_name = malloc(len + 1, M_SEM, M_WAITOK)) == NULL){ + free(ret, M_SEM); + return (ENOMEM); + } strcpy(ret->ks_name, name); } else { ret->ks_name = NULL; } ret->ks_mode = mode; ret->ks_value = value; - ret->ks_ref = 1; + ret->ks_ref = 0; ret->ks_waiters = 0; ret->ks_uid = uc->cr_uid; ret->ks_gid = uc->cr_gid; ret->ks_onlist = 0; cv_init(&ret->ks_cv, "sem"); LIST_INIT(&ret->ks_users); + mtx_init(&ret->ks_mtx, "ks_mtx", "ks_mtx", MTX_DEF); if (name != NULL) - sem_enter(td->td_proc, ret); + sem_enter(td->td_proc, ret); /* This invokes sem_ref */ + else + ret->ks_ref = 1; #ifdef MAC mac_init_posix_ksem(ret); mac_create_posix_ksem(uc, ret); #endif - *ksret = ret; mtx_lock(&sem_lock); + nsems++; if (nsems >= p31b_getcfg(CTL_P1003_1B_SEM_NSEMS_MAX)) { - /*XXX Should sem_leave be here at all ? */ - sem_leave(td->td_proc, ret); - sem_free(ret); - error = ENFILE; - } else { - nsems++; - error = 0; - } + if (name != NULL) + sem_leave(td->td_proc, ret); /* This invokes sem_rel */ + SEM_DROP(ret); /* sem_free does a nsem-- */ + mtx_unlock(&sem_lock); + return (ENFILE); + } mtx_unlock(&sem_lock); - return (error); + *ksret = ret; + return (0); } #ifndef _SYS_SYSPROTO_H_ @@ -279,15 +294,14 @@ semid_t id; int error; - error = sem_create(td, NULL, &ks, S_IRWXU | S_IRWXG, value); - if (error) + if((error = sem_create(td, NULL, &ks, S_IRWXU | S_IRWXG, value))) return (error); id = SEM_TO_ID(ks); if (dir == UIO_USERSPACE) { error = copyout(&id, idp, sizeof(id)); if (error) { mtx_lock(&sem_lock); - sem_rel(ks); + SEM_DROP(ks); mtx_unlock(&sem_lock); return (error); } @@ -322,12 +336,62 @@ error = copyinstr(uap->name, name, SEM_MAX_NAMELEN + 1, &done); if (error) - return (error); + return (-1); DP((">>> sem_open start\n")); error = kern_sem_open(td, UIO_USERSPACE, name, uap->oflag, uap->mode, uap->value, uap->idp); DP(("<<< sem_open end\n")); - return (error); + if(error) + return (-1); + return (0); +} + +static int +ksem_open_existing(struct thread *td, struct ksem *ks, int dir, semid_t *idpu, + semid_t *idpk) +{ + int error = 0; + mtx_assert(&sem_lock,MA_OWNED); + mtx_assert(&ks->ks_mtx,MA_NOTOWNED); + if((error = sem_perm(td, ks))) { + mtx_unlock(&sem_lock); + return (error); + } + /* + * If already queued up for unlinking. + * Then according to spec cant let reconnect to this semaphore. + */ + if(ks->ks_unlinked) { + mtx_unlock(&sem_lock); + return (EPERM); + } + *idpk = SEM_TO_ID(ks); + sem_ref(ks); /* Pump up the refs to avoid the race with SEM_FREE */ + mtx_unlock(&sem_lock); +#ifdef MAC + mtx_lock(&ks->ks_mtx); + if((error = mac_check_posix_sem_openexisting(td->td_ucred, ks))) { + DP(("MAC Framework: mac_check_posix_sem_openexisting access denied\n")); + mtx_unlock(&ks->ks_mtx); + goto err_open_existing; + } + mtx_unlock(&ks->ks_mtx); +#endif + if (dir == UIO_USERSPACE) { + if ((error = copyout(idpk, idpu, sizeof(*idpk)))) { + goto err_open_existing; + } + } else { + *idpu = *idpk; + } + sem_enter(td->td_proc, ks); +err_open_existing: + mtx_lock(&sem_lock); + sem_rel(ks); /* Pump down */ + if(ks->ks_unlinked && LIST_EMPTY(&ks->ks_users)) + SEM_FREE(ks); + mtx_unlock(&sem_lock); + return(error); } static int @@ -341,7 +405,7 @@ semid_t *idp; { struct ksem *ksnew, *ks; - int error; + int error = 0; semid_t id; ksnew = NULL; @@ -369,8 +433,7 @@ * We may block during creation, so drop the lock. */ mtx_unlock(&sem_lock); - error = sem_create(td, name, &ksnew, mode, value); - if (error != 0) + if((error = sem_create(td, name, &ksnew, mode, value))) return (error); id = SEM_TO_ID(ksnew); if (dir == UIO_USERSPACE) { @@ -379,7 +442,7 @@ if (error) { mtx_lock(&sem_lock); sem_leave(td->td_proc, ksnew); - sem_rel(ksnew); + SEM_DROP(ksnew); mtx_unlock(&sem_lock); return (error); } @@ -396,53 +459,26 @@ if (ks != NULL) { /* we lost... */ sem_leave(td->td_proc, ksnew); - sem_rel(ksnew); + SEM_DROP(ksnew); /* we lost and we can't loose... */ if ((oflag & O_EXCL) != 0) { mtx_unlock(&sem_lock); return (EEXIST); } + /* Use the sem created by the winner */ + else { + error = ksem_open_existing(td, ks, dir, idp, &id); + } } else { DP(("sem_create: about to add to list...\n")); LIST_INSERT_HEAD(&ksem_head, ksnew, ks_entry); DP(("sem_create: setting list bit...\n")); ksnew->ks_onlist = 1; DP(("sem_create: done, about to unlock...\n")); + mtx_unlock(&sem_lock); } - mtx_unlock(&sem_lock); } else { - /* - * if we aren't the creator, then enforce permissions. - */ - if((error = sem_perm(td, ks))) - goto err; -#ifdef MAC - if((error = mac_check_posix_sem_openexisting(td->td_ucred, ks))) { - DP(("MAC Framework: mac_check_posix_sem_openexisting access denied\n")); - goto err; - } -#endif - sem_ref(ks); -err: - mtx_unlock(&sem_lock); - if (error) - return (error); - id = SEM_TO_ID(ks); - if (dir == UIO_USERSPACE) { - error = copyout(&id, idp, sizeof(id)); - if (error) { - mtx_lock(&sem_lock); - sem_rel(ks); - mtx_unlock(&sem_lock); - return (error); - } - } else { - *idp = id; - } - sem_enter(td->td_proc, ks); - mtx_lock(&sem_lock); - sem_rel(ks); - mtx_unlock(&sem_lock); + error = ksem_open_existing(td, ks, dir, idp, &id); } return (error); } @@ -466,21 +502,27 @@ } static void -sem_free(struct ksem *ks) +sem_free(struct ksem *ks, int checkrefs) { - + mtx_assert(&sem_lock, MA_OWNED); + mtx_assert(&ks->ks_mtx, MA_NOTOWNED); + if(checkrefs && (ks->ks_ref > 0)) + return; nsems--; if (ks->ks_onlist) LIST_REMOVE(ks, ks_entry); + if (ks->ks_name != NULL) free(ks->ks_name, M_SEM); cv_destroy(&ks->ks_cv); #ifdef MAC mac_destroy_posix_ksem(ks); #endif + mtx_destroy(&ks->ks_mtx); free(ks, M_SEM); } + static __inline struct kuser *sem_getuser(struct proc *p, struct ksem *ks); static __inline struct kuser * @@ -489,7 +531,8 @@ struct ksem *ks; { struct kuser *k; - + + mtx_assert(&sem_lock, MA_OWNED); LIST_FOREACH(k, &ks->ks_users, ku_next) if (k->ku_pid == p->p_pid) return (k); @@ -501,9 +544,15 @@ struct thread *td; struct ksem *ks; { + struct kuser *k; + int ret = 0; + + mtx_assert(&sem_lock, MA_OWNED); + k = sem_getuser(td->td_proc, ks); + if ((ks->ks_name == NULL && sem_perm(td, ks)) || k != NULL) + ret = 1; + return ret; - return ((ks->ks_name == NULL && sem_perm(td, ks)) - || sem_getuser(td->td_proc, ks) != NULL); } static int @@ -511,20 +560,21 @@ struct proc *p; struct ksem *ks; { - struct kuser *k; + struct kuser *k=NULL; DP(("sem_leave: ks = %p\n", ks)); + mtx_assert(&sem_lock, MA_OWNED); + DP(("sem_leave: ks = %p, k = %p\n", ks, k)); k = sem_getuser(p, ks); - DP(("sem_leave: ks = %p, k = %p\n", ks, k)); - if (k != NULL) { - LIST_REMOVE(k, ku_next); - sem_rel(ks); - DP(("sem_leave: about to free k\n")); - free(k, M_SEM); - DP(("sem_leave: returning\n")); - return (0); + if (k == NULL) { + return (EINVAL); } - return (EINVAL); + LIST_REMOVE(k, ku_next); + sem_rel(ks); + DP(("sem_leave: about to free k\n")); + free(k, M_SEM); + DP(("sem_leave: returning\n")); + return (0); } static void @@ -534,7 +584,10 @@ { struct kuser *ku, *k; - ku = malloc(sizeof(*ku), M_SEM, M_WAITOK | M_ZERO); + mtx_assert(&sem_lock, MA_NOTOWNED); + mtx_assert(&ks->ks_mtx, MA_NOTOWNED); + if( (ku = malloc(sizeof(*ku), M_SEM, M_WAITOK | M_ZERO)) == NULL) + return; ku->ku_pid = p->p_pid; mtx_lock(&sem_lock); k = sem_getuser(p, ks); @@ -579,26 +632,24 @@ mtx_lock(&sem_lock); ks = sem_lookup_byname(name); - if (ks == NULL) + if (ks == NULL) { error = ENOENT; - else { - if ((error = sem_perm(td, ks))) - goto err; + goto err_unlink; + } + if ((error = sem_perm(td, ks))) + goto err_unlink; #ifdef MAC - if((error = mac_check_posix_sem_unlink(td->td_ucred, ks))) { - DP(("MAC Framework: mac_check_posix_sem_unlink access \ - denied\n")); - goto err; - } + if((error = mac_check_posix_sem_unlink(td->td_ucred, ks))) { + DP(("MAC Framework: mac_check_posix_sem_unlink access \ + denied\n")); + goto err_unlink; + } #endif - } DP(("sem_unlink: '%s' ks = %p, error = %d\n", name, ks, error)); - if (error == 0) { - LIST_REMOVE(ks, ks_entry); - LIST_INSERT_HEAD(&ksem_deadhead, ks, ks_entry); - sem_rel(ks); - } -err: + ks->ks_unlinked = 1; + if(LIST_EMPTY(&ks->ks_users)) + SEM_FREE(ks); +err_unlink: mtx_unlock(&sem_lock); return (error); } @@ -631,15 +682,18 @@ /* this is not a valid operation for unnamed sems */ if (ks != NULL && ks->ks_name != NULL) { #ifdef MAC - if((error = mac_check_posix_sem_close(td->td_ucred, ks))) { + if ((error = mac_check_posix_sem_close(td->td_ucred, ks))) { DP(("MAC Framework: mac_check_posix_sem_close access \ denied\n")); - goto err; + goto err_close; } #endif - error = sem_leave(td->td_proc, ks); + if ((error = sem_leave(td->td_proc, ks))) + goto err_close; + if (ks->ks_unlinked && LIST_EMPTY(&ks->ks_users)) + SEM_FREE(ks); } -err: +err_close: mtx_unlock(&sem_lock); return (error); } @@ -665,29 +719,37 @@ semid_t id; { struct ksem *ks; - int error; + int error = 0; mtx_lock(&sem_lock); ks = ID_TO_SEM(id); if (ks == NULL || !sem_hasopen(td, ks)) { - error = EINVAL; - goto err; + mtx_unlock(&sem_lock); + return (EINVAL); } + sem_ref(ks);/* Pump up the refs to avoid the race with SEM_FREE */ + mtx_unlock(&sem_lock); + + mtx_lock(&ks->ks_mtx); if (ks->ks_value == SEM_VALUE_MAX) { error = EOVERFLOW; - goto err; + goto err_post; } #ifdef MAC - if((error = mac_check_posix_sem_post(td->td_ucred, ks))) { + if ((error = mac_check_posix_sem_post(td->td_ucred, ks))) { DP(("MAC Framework: mac_check_posix_sem_post access denied\n")); - goto err; + goto err_post; } #endif ++ks->ks_value; if (ks->ks_waiters > 0) cv_signal(&ks->ks_cv); - error = 0; -err: +err_post: + mtx_unlock(&ks->ks_mtx); + mtx_lock(&sem_lock); + sem_rel(ks); /* Pump down */ + if (ks->ks_unlinked && LIST_EMPTY(&ks->ks_users)) + SEM_FREE(ks); mtx_unlock(&sem_lock); return (error); } @@ -730,43 +792,49 @@ int tryflag; { struct ksem *ks; - int error; + int error = 0; DP((">>> kern_sem_wait entered!\n")); mtx_lock(&sem_lock); ks = ID_TO_SEM(id); if (ks == NULL) { DP(("kern_sem_wait ks == NULL\n")); - error = EINVAL; - goto err; + mtx_unlock(&sem_lock); + return (EINVAL); } - sem_ref(ks); if (!sem_hasopen(td, ks)) { DP(("kern_sem_wait hasopen failed\n")); - error = EINVAL; - goto err; + mtx_unlock(&sem_lock); + return (EINVAL); } + sem_ref(ks);/* Pump up the refs to avoid the race with SEM_FREE */ + mtx_unlock(&sem_lock); + + mtx_lock(&ks->ks_mtx); #ifdef MAC - if((error = mac_check_posix_sem_wait(td->td_ucred, ks))) { + if ((error = mac_check_posix_sem_wait(td->td_ucred, ks))) { DP(("MAC Framework: mac_check_posix_sem_wait access denied\n")); - goto err; + goto err_wait; } #endif DP(("kern_sem_wait value = %d, tryflag %d\n", ks->ks_value, tryflag)); if (ks->ks_value == 0) { ks->ks_waiters++; - error = tryflag ? EAGAIN : cv_wait_sig(&ks->ks_cv, &sem_lock); + error = tryflag ? EAGAIN : cv_wait_sig(&ks->ks_cv, &ks->ks_mtx); ks->ks_waiters--; if (error) - goto err; + goto err_wait; } ks->ks_value--; error = 0; -err: - if (ks != NULL) - sem_rel(ks); +err_wait: + mtx_unlock(&ks->ks_mtx); + DP(("<<< kern_sem_wait leaving, error = %d\n", error)); + mtx_lock(&sem_lock); + sem_rel(ks); /* Pump down */ + if (ks->ks_unlinked && LIST_EMPTY(&ks->ks_users)) + SEM_FREE(ks); mtx_unlock(&sem_lock); - DP(("<<< kern_sem_wait leaving, error = %d\n", error)); return (error); } @@ -791,16 +859,26 @@ mtx_unlock(&sem_lock); return (EINVAL); } + sem_ref(ks);/* Pump up the refs to avoid the race with SEM_FREE */ + mtx_unlock(&sem_lock); + + mtx_lock(&ks->ks_mtx); #ifdef MAC if((error = mac_check_posix_sem_getvalue(td->td_ucred, ks))) { DP(("MAC Framework: mac_check_posix_sem_getvalue access denied\n")); - mtx_unlock(&sem_lock); - return (error); + mtx_unlock(&ks->ks_mtx); + goto err_getvalue; } #endif val = ks->ks_value; + mtx_unlock(&ks->ks_mtx); + error = copyout(&val, uap->val, sizeof(val)); +err_getvalue: + mtx_lock(&sem_lock); + sem_rel(ks); /* Pump down */ + if (ks->ks_unlinked && LIST_EMPTY(&ks->ks_users)) + SEM_FREE(ks); mtx_unlock(&sem_lock); - error = copyout(&val, uap->val, sizeof(val)); return (error); } @@ -816,28 +894,24 @@ struct ksem_destroy_args *uap; { struct ksem *ks; - int error; + int error = 0; mtx_lock(&sem_lock); ks = ID_TO_SEM(uap->id); if (ks == NULL || !sem_hasopen(td, ks) || ks->ks_name != NULL) { error = EINVAL; - goto err; + goto err_destroy; } - if (ks->ks_waiters != 0) { - error = EBUSY; - goto err; - } #ifdef MAC if((error = mac_check_posix_sem_destroy(td->td_ucred, ks))) { DP(("MAC Framework: mac_check_posix_sem_destroy access denied\n")); - goto err; + goto err_destroy; } #endif - sem_rel(ks); - error = 0; -err: + ks->ks_unlinked = 1; /* Indicate that the sem needs to be destroyed */ + SEM_FREE(ks); +err_destroy: mtx_unlock(&sem_lock); return (error); } @@ -853,13 +927,9 @@ ks = LIST_FIRST(&ksem_head); while (ks != NULL) { ksnext = LIST_NEXT(ks, ks_entry); - sem_leave(p, ks); - ks = ksnext; - } - ks = LIST_FIRST(&ksem_deadhead); - while (ks != NULL) { - ksnext = LIST_NEXT(ks, ks_entry); - sem_leave(p, ks); + if(!sem_leave(p, ks)) + if (ks->ks_unlinked && LIST_EMPTY(&ks->ks_users)) + SEM_FREE(ks); ks = ksnext; } mtx_unlock(&sem_lock); ==== //depot/projects/trustedbsd/mac/sys/posix4/ksem.h#2 (text+ko) ==== @@ -69,7 +69,9 @@ struct cv ks_cv; /* waiters sleep here */ int ks_waiters; /* number of waiters */ LIST_HEAD(, kuser) ks_users; /* pids using this sem */ + struct mtx ks_mtx; /* mutex protecting the ks_users list */ struct label ks_label; /* MAC label */ + int ks_unlinked; /* Whether the named sem is unlinked */ }; #endif /* _KERNEL */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200309171710.h8HHArq2001172>