Date: Fri, 23 Jun 2006 21:05:55 GMT From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 99892 for review Message-ID: <200606232105.k5NL5tBc059581@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=99892 Change 99892 by jhb@jhb_mutex on 2006/06/23 21:05:48 - Add a kern_semctl() and use it to cleanup linux_semctl() and svr4_semctl(). (Both of which are MPSAFE now.) Affected files ... .. //depot/projects/smpng/sys/compat/linux/linux_ipc.c#24 edit .. //depot/projects/smpng/sys/compat/svr4/svr4_ipc.c#12 edit .. //depot/projects/smpng/sys/kern/sysv_sem.c#36 edit .. //depot/projects/smpng/sys/sys/syscallsubr.h#29 edit Differences ... ==== //depot/projects/smpng/sys/compat/linux/linux_ipc.c#24 (text+ko) ==== @@ -491,69 +491,56 @@ linux_semctl(struct thread *td, struct linux_semctl_args *args) { struct l_semid_ds linux_semid; - struct __semctl_args /* { - int semid; - int semnum; - int cmd; - union semun *arg; - } */ bsd_args; struct l_seminfo linux_seminfo; - int error; - union semun *unptr; - caddr_t sg; - - sg = stackgap_init(); - - /* Make sure the arg parameter can be copied in. */ - unptr = stackgap_alloc(&sg, sizeof(union semun)); - bcopy(&args->arg, unptr, sizeof(union semun)); - - bsd_args.semid = args->semid; - bsd_args.semnum = args->semnum; - bsd_args.arg = unptr; + struct semid_ds semid; + union semun semun; + int cmd, error; switch (args->cmd & ~LINUX_IPC_64) { case LINUX_IPC_RMID: - bsd_args.cmd = IPC_RMID; + cmd = IPC_RMID; break; case LINUX_GETNCNT: - bsd_args.cmd = GETNCNT; + cmd = GETNCNT; break; case LINUX_GETPID: - bsd_args.cmd = GETPID; + cmd = GETPID; break; case LINUX_GETVAL: - bsd_args.cmd = GETVAL; + cmd = GETVAL; break; case LINUX_GETZCNT: - bsd_args.cmd = GETZCNT; + cmd = GETZCNT; break; case LINUX_SETVAL: - bsd_args.cmd = SETVAL; + cmd = SETVAL; + semun.val = args->arg.val; break; case LINUX_IPC_SET: - bsd_args.cmd = IPC_SET; + cmd = IPC_SET; error = linux_semid_pullup(args->cmd & LINUX_IPC_64, &linux_semid, (caddr_t)PTRIN(args->arg.buf)); if (error) return (error); - unptr->buf = stackgap_alloc(&sg, sizeof(struct semid_ds)); - linux_to_bsd_semid_ds(&linux_semid, unptr->buf); - return __semctl(td, &bsd_args); + linux_to_bsd_semid_ds(&linux_semid, &semid); + semun.buf = &semid; + return kern_semctl(td, args->semid, args->semnum, cmd, &semun, + UIO_SYSSPACE); case LINUX_IPC_STAT: case LINUX_SEM_STAT: if((args->cmd & ~LINUX_IPC_64) == LINUX_IPC_STAT) - bsd_args.cmd = IPC_STAT; + cmd = IPC_STAT; else - bsd_args.cmd = SEM_STAT; - unptr->buf = stackgap_alloc(&sg, sizeof(struct semid_ds)); - error = __semctl(td, &bsd_args); + cmd = SEM_STAT; + semun.buf = &semid; + error = kern_semctl(td, args->semid, args->semnum, cmd, &semun, + UIO_SYSSPACE); if (error) - return error; - td->td_retval[0] = (bsd_args.cmd == SEM_STAT) ? - IXSEQ_TO_IPCID(bsd_args.semid, unptr->buf->sem_perm) : + return (error); + td->td_retval[0] = (cmd == SEM_STAT) ? + IXSEQ_TO_IPCID(args->semid, semid.sem_perm) : 0; - bsd_to_linux_semid_ds(unptr->buf, &linux_semid); + bsd_to_linux_semid_ds(&semid, &linux_semid); return (linux_semid_pushdown(args->cmd & LINUX_IPC_64, &linux_semid, (caddr_t)PTRIN(args->arg.buf))); case LINUX_IPC_INFO: @@ -580,7 +567,8 @@ args->cmd & ~LINUX_IPC_64); return EINVAL; } - return __semctl(td, &bsd_args); + return kern_semctl(td, args->semid, args->semnum, cmd, &semun, + UIO_USERSPACE); } int ==== //depot/projects/smpng/sys/compat/svr4/svr4_ipc.c#12 (text+ko) ==== @@ -105,8 +105,6 @@ struct svr4_semid_ds *); static void svr4_to_bsd_semid_ds(const struct svr4_semid_ds *, struct semid_ds *); -static int svr4_setsemun(caddr_t *sgp, union semun **argp, - union semun *usp); static int svr4_semop(struct thread *, void *); static int svr4_semget(struct thread *, void *); static int svr4_semctl(struct thread *, void *); @@ -194,16 +192,6 @@ bds->sem_pad2 = sds->sem_pad2; } -static int -svr4_setsemun(sgp, argp, usp) - caddr_t *sgp; - union semun **argp; - union semun *usp; -{ - *argp = stackgap_alloc(sgp, sizeof(union semun)); - return copyout((caddr_t)usp, *argp, sizeof(union semun)); -} - struct svr4_sys_semctl_args { int what; int semid; @@ -217,108 +205,71 @@ struct thread *td; void *v; { - int error; struct svr4_sys_semctl_args *uap = v; - struct __semctl_args ap; struct svr4_semid_ds ss; - struct semid_ds bs, *bsp; - caddr_t sg = stackgap_init(); - - ap.semid = uap->semid; - ap.semnum = uap->semnum; + struct semid_ds bs; + union semun semun; + int cmd, error; switch (uap->cmd) { case SVR4_SEM_GETZCNT: + cmd = GETZCNT; + break; + case SVR4_SEM_GETNCNT: + cmd = GETNCNT; + break; + case SVR4_SEM_GETPID: + cmd = GETPID; + break; + case SVR4_SEM_GETVAL: - switch (uap->cmd) { - case SVR4_SEM_GETZCNT: - ap.cmd = GETZCNT; - break; - case SVR4_SEM_GETNCNT: - ap.cmd = GETNCNT; - break; - case SVR4_SEM_GETPID: - ap.cmd = GETPID; - break; - case SVR4_SEM_GETVAL: - ap.cmd = GETVAL; - break; - } - return __semctl(td, &ap); + cmd = GETVAL; + break; case SVR4_SEM_SETVAL: - error = svr4_setsemun(&sg, &ap.arg, &uap->arg); - if (error) - return error; - ap.cmd = SETVAL; - return __semctl(td, &ap); + cmd = SETVAL; + break; case SVR4_SEM_GETALL: - error = svr4_setsemun(&sg, &ap.arg, &uap->arg); - if (error) - return error; - ap.cmd = GETVAL; - return __semctl(td, &ap); + cmd = GETVAL; + break; case SVR4_SEM_SETALL: - error = svr4_setsemun(&sg, &ap.arg, &uap->arg); - if (error) - return error; - ap.cmd = SETVAL; - return __semctl(td, &ap); + cmd = SETVAL; + break; case SVR4_IPC_STAT: - ap.cmd = IPC_STAT; - bsp = stackgap_alloc(&sg, sizeof(bs)); - error = svr4_setsemun(&sg, &ap.arg, - (union semun *)&bsp); + cmd = IPC_STAT; + semun.buf = &bs; + error = kern_semctl(td, uap->semid, uap->semnum, cmd, &semun, + UIO_SYSSPACE); if (error) - return error; - if ((error = __semctl(td, &ap)) != 0) - return error; - error = copyin((caddr_t)bsp, (caddr_t)&bs, sizeof(bs)); - if (error) return error; bsd_to_svr4_semid_ds(&bs, &ss); return copyout(&ss, uap->arg.buf, sizeof(ss)); case SVR4_IPC_SET: - ap.cmd = IPC_SET; - bsp = stackgap_alloc(&sg, sizeof(bs)); - error = svr4_setsemun(&sg, &ap.arg, - (union semun *)&bsp); - if (error) - return error; + cmd = IPC_SET; error = copyin(uap->arg.buf, (caddr_t) &ss, sizeof ss); if (error) return error; svr4_to_bsd_semid_ds(&ss, &bs); - error = copyout(&bs, bsp, sizeof(bs)); - if (error) - return error; - return __semctl(td, &ap); + semun.buf = &bs; + return kern_semctl(td, uap->semid, uap->semnum, cmd, &semun, + UIO_SYSSPACE); case SVR4_IPC_RMID: - ap.cmd = IPC_RMID; - bsp = stackgap_alloc(&sg, sizeof(bs)); - error = svr4_setsemun(&sg, &ap.arg, - (union semun *)&bsp); - if (error) - return error; - error = copyin(uap->arg.buf, &ss, sizeof ss); - if (error) - return error; - svr4_to_bsd_semid_ds(&ss, &bs); - error = copyout(&bs, bsp, sizeof(bs)); - if (error) - return error; - return __semctl(td, &ap); + cmd = IPC_RMID; + break; default: return EINVAL; } + + return kern_semctl(td, uap->semid, uap->semnum, cmd, &uap->arg, + UIO_USERSPACE); } struct svr4_sys_semget_args { ==== //depot/projects/smpng/sys/kern/sysv_sem.c#36 (text+ko) ==== @@ -53,8 +53,10 @@ #include <sys/mutex.h> #include <sys/sem.h> #include <sys/syscall.h> +#include <sys/syscallsubr.h> #include <sys/sysent.h> #include <sys/sysctl.h> +#include <sys/uio.h> #include <sys/malloc.h> #include <sys/jail.h> #include <sys/mac.h> @@ -554,12 +556,35 @@ struct thread *td; struct __semctl_args *uap; { - int semid = uap->semid; - int semnum = uap->semnum; - int cmd = uap->cmd; + union semun real_arg; + union semun *arg; + int error; + + switch (uap->cmd) { + case SEM_STAT: + case IPC_SET: + case IPC_STAT: + case GETALL: + case SETVAL: + case SETALL: + error = copyin(uap->arg, &real_arg, sizeof(real_arg)); + if (error) + return (error); + arg = &real_arg; + break; + default: + arg = NULL; + break; + } + return (kern_semctl(td, uap->semid, uap->semnum, uap->cmd, arg, + UIO_USERSPACE)); +} + +int +kern_semctl(struct thread *td, int semid, int semnum, int cmd, union semun *arg, + enum uio_seg bufseg) +{ u_short *array; - union semun *arg = uap->arg; - union semun real_arg; struct ucred *cred = td->td_ucred; int i, rval, error; struct semid_ds sbuf; @@ -578,8 +603,6 @@ case SEM_STAT: if (semid < 0 || semid >= seminfo.semmni) return (EINVAL); - if ((error = copyin(arg, &real_arg, sizeof(real_arg))) != 0) - return (error); semakptr = &sema[semid]; sema_mtxp = &sema_mtx[semid]; mtx_lock(sema_mtxp); @@ -598,8 +621,11 @@ } #endif mtx_unlock(sema_mtxp); - error = copyout(&semakptr->u, real_arg.buf, - sizeof(struct semid_ds)); + if (bufseg == UIO_USERSPACE) + error = copyout(&semakptr->u, arg->buf, + sizeof(struct semid_ds)); + else + bcopy(&semakptr->u, arg->buf, sizeof(struct semid_ds)); rval = IXSEQ_TO_IPCID(semid, semakptr->u.sem_perm); if (error == 0) td->td_retval[0] = rval; @@ -629,7 +655,7 @@ switch (cmd) { case IPC_RMID: mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_M))) goto done2; @@ -654,12 +680,14 @@ break; case IPC_SET: - if ((error = copyin(arg, &real_arg, sizeof(real_arg))) != 0) - goto done2; - if ((error = copyin(real_arg.buf, &sbuf, sizeof(sbuf))) != 0) - goto done2; + if (bufseg == UIO_USERSPACE) { + error = copyin(arg->buf, &sbuf, sizeof(sbuf)); + if (error) + goto done2; + } else + bcopy(arg->buf, &sbuf, sizeof(sbuf)); mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_M))) goto done2; @@ -671,22 +699,23 @@ break; case IPC_STAT: - if ((error = copyin(arg, &real_arg, sizeof(real_arg))) != 0) - goto done2; mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_R))) goto done2; sbuf = semakptr->u; mtx_unlock(sema_mtxp); - error = copyout(&semakptr->u, real_arg.buf, - sizeof(struct semid_ds)); + if (bufseg == UIO_USERSPACE) + error = copyout(&semakptr->u, arg->buf, + sizeof(struct semid_ds)); + else + bcopy(&semakptr->u, arg->buf, sizeof(struct semid_ds)); break; case GETNCNT: mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_R))) goto done2; @@ -699,7 +728,7 @@ case GETPID: mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_R))) goto done2; @@ -712,7 +741,7 @@ case GETVAL: mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_R))) goto done2; @@ -724,25 +753,31 @@ break; case GETALL: - if ((error = copyin(arg, &real_arg, sizeof(real_arg))) != 0) - goto done2; + /* + * Given the fact that this copies out a variable amount + * of data into userland, I don't see any feasible way of + * doing this with a kmem copy of the userland buffer. + */ + if (bufseg == UIO_SYSSPACE) + return (EINVAL); + array = malloc(sizeof(*array) * semakptr->u.sem_nsems, M_TEMP, M_WAITOK); mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_R))) goto done2; for (i = 0; i < semakptr->u.sem_nsems; i++) array[i] = semakptr->u.sem_base[i].semval; mtx_unlock(sema_mtxp); - error = copyout(array, real_arg.array, - i * sizeof(real_arg.array[0])); + error = copyout(array, arg->array, + i * sizeof(arg->array[0])); break; case GETZCNT: mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_R))) goto done2; @@ -754,10 +789,8 @@ break; case SETVAL: - if ((error = copyin(arg, &real_arg, sizeof(real_arg))) != 0) - goto done2; mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; if ((error = ipcperm(td, &semakptr->u.sem_perm, IPC_W))) goto done2; @@ -765,11 +798,11 @@ error = EINVAL; goto done2; } - if (real_arg.val < 0 || real_arg.val > seminfo.semvmx) { + if (arg->val < 0 || arg->val > seminfo.semvmx) { error = ERANGE; goto done2; } - semakptr->u.sem_base[semnum].semval = real_arg.val; + semakptr->u.sem_base[semnum].semval = arg->val; SEMUNDO_LOCK(); semundo_clear(semid, semnum); SEMUNDO_UNLOCK(); @@ -777,20 +810,25 @@ break; case SETALL: + /* + * Given the fact that this copies in variable amounts of + * userland data in a loop, I don't see any feasible way + * of doing this with a kmem copy of the userland buffer. + */ + if (bufseg == UIO_SYSSPACE) + return (EINVAL); mtx_lock(sema_mtxp); raced: - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; count = semakptr->u.sem_nsems; mtx_unlock(sema_mtxp); - if ((error = copyin(arg, &real_arg, sizeof(real_arg))) != 0) - goto done2; array = malloc(sizeof(*array) * count, M_TEMP, M_WAITOK); - error = copyin(real_arg.array, array, count * sizeof(*array)); + error = copyin(arg->array, array, count * sizeof(*array)); if (error) break; mtx_lock(sema_mtxp); - if ((error = semvalid(uap->semid, semakptr)) != 0) + if ((error = semvalid(semid, semakptr)) != 0) goto done2; /* we could have raced? */ if (count != semakptr->u.sem_nsems) { ==== //depot/projects/smpng/sys/sys/syscallsubr.h#29 (text+ko) ==== @@ -41,6 +41,7 @@ struct msqid_ds; struct rlimit; struct rusage; +union semun; struct sockaddr; struct stat; struct kevent; @@ -123,6 +124,8 @@ int kern_rmdir(struct thread *td, char *path, enum uio_seg pathseg); int kern_sched_rr_get_interval(struct thread *td, pid_t pid, struct timespec *ts); +int kern_semctl(struct thread *td, int semid, int semnum, int cmd, + union semun *arg, enum uio_seg bufseg); int kern_select(struct thread *td, int nd, fd_set *fd_in, fd_set *fd_ou, fd_set *fd_ex, struct timeval *tvp); int kern_sendfile(struct thread *td, struct sendfile_args *uap,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606232105.k5NL5tBc059581>