The patch titled IPC: consolidate all xxxctl_down() functions has been added to the -mm tree. Its filename is ipc-consolidate-all-xxxctl_down-functions.patch Before you just go and hit "reply", please: a) Consider who else should be cc'ed b) Prefer to cc a suitable mailing list as well c) Ideally: find the original patch on the mailing list and do a reply-to-all to that, adding suitable additional cc's *** Remember to use Documentation/SubmitChecklist when testing your code *** See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find out what to do about this The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/ ------------------------------------------------------ Subject: IPC: consolidate all xxxctl_down() functions From: Pierre Peiffer <pierre.peiffer@xxxxxxxx> semctl_down(), msgctl_down() and shmctl_down() are used to handle the same set of commands for each kind of IPC. They all start to do the same job (they retrieve the ipc and do some permission checks) before handling the commands on their own. This patch proposes to consolidate this by moving these same pieces of code into one common function called ipcctl_pre_down(). It simplifies a little these xxxctl_down() functions and increases a little the maintainability. Signed-off-by: Pierre Peiffer <pierre.peiffer@xxxxxxxx> Acked-by: Serge Hallyn <serue@xxxxxxxxxx> Cc: Nadia Derbey <Nadia.Derbey@xxxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> --- ipc/msg.c | 48 +++++------------------------------------------- ipc/sem.c | 42 ++++-------------------------------------- ipc/shm.c | 42 ++++-------------------------------------- ipc/util.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ ipc/util.h | 2 ++ 5 files changed, 66 insertions(+), 119 deletions(-) diff -puN ipc/msg.c~ipc-consolidate-all-xxxctl_down-functions ipc/msg.c --- a/ipc/msg.c~ipc-consolidate-all-xxxctl_down-functions +++ a/ipc/msg.c @@ -142,21 +142,6 @@ void __init msg_init(void) } /* - * This routine is called in the paths where the rw_mutex is held to protect - * access to the idr tree. - */ -static inline struct msg_queue *msg_lock_check_down(struct ipc_namespace *ns, - int id) -{ - struct kern_ipc_perm *ipcp = ipc_lock_check_down(&msg_ids(ns), id); - - if (IS_ERR(ipcp)) - return (struct msg_queue *)ipcp; - - return container_of(ipcp, struct msg_queue, q_perm); -} - -/* * msg_lock_(check_) routines are called in the paths where the rw_mutex * is not held. */ @@ -437,35 +422,12 @@ static int msgctl_down(struct ipc_namesp return -EFAULT; } - down_write(&msg_ids(ns).rw_mutex); - msq = msg_lock_check_down(ns, msqid); - if (IS_ERR(msq)) { - err = PTR_ERR(msq); - goto out_up; - } - - ipcp = &msq->q_perm; - - err = audit_ipc_obj(ipcp); - if (err) - goto out_unlock; - - if (cmd == IPC_SET) { - err = audit_ipc_set_perm(msqid64.msg_qbytes, - msqid64.msg_perm.uid, - msqid64.msg_perm.gid, - msqid64.msg_perm.mode); - if (err) - goto out_unlock; - } + ipcp = ipcctl_pre_down(&msg_ids(ns), msqid, cmd, + &msqid64.msg_perm, msqid64.msg_qbytes); + if (IS_ERR(ipcp)) + return PTR_ERR(ipcp); - if (current->euid != ipcp->cuid && - current->euid != ipcp->uid && - !capable(CAP_SYS_ADMIN)) { - /* We _could_ check for CAP_CHOWN above, but we don't */ - err = -EPERM; - goto out_unlock; - } + msq = container_of(ipcp, struct msg_queue, q_perm); err = security_msg_queue_msgctl(msq, cmd); if (err) diff -puN ipc/sem.c~ipc-consolidate-all-xxxctl_down-functions ipc/sem.c --- a/ipc/sem.c~ipc-consolidate-all-xxxctl_down-functions +++ a/ipc/sem.c @@ -141,21 +141,6 @@ void __init sem_init (void) } /* - * This routine is called in the paths where the rw_mutex is held to protect - * access to the idr tree. - */ -static inline struct sem_array *sem_lock_check_down(struct ipc_namespace *ns, - int id) -{ - struct kern_ipc_perm *ipcp = ipc_lock_check_down(&sem_ids(ns), id); - - if (IS_ERR(ipcp)) - return (struct sem_array *)ipcp; - - return container_of(ipcp, struct sem_array, sem_perm); -} - -/* * sem_lock_(check_) routines are called in the paths where the rw_mutex * is not held. */ @@ -878,31 +863,12 @@ static int semctl_down(struct ipc_namesp if (copy_semid_from_user(&semid64, arg.buf, version)) return -EFAULT; } - down_write(&sem_ids(ns).rw_mutex); - sma = sem_lock_check_down(ns, semid); - if (IS_ERR(sma)) { - err = PTR_ERR(sma); - goto out_up; - } - - ipcp = &sma->sem_perm; - err = audit_ipc_obj(ipcp); - if (err) - goto out_unlock; + ipcp = ipcctl_pre_down(&sem_ids(ns), semid, cmd, &semid64.sem_perm, 0); + if (IS_ERR(ipcp)) + return PTR_ERR(ipcp); - if (cmd == IPC_SET) { - err = audit_ipc_set_perm(0, semid64.sem_perm.uid, - semid64.sem_perm.gid, - semid64.sem_perm.mode); - if (err) - goto out_unlock; - } - if (current->euid != ipcp->cuid && - current->euid != ipcp->uid && !capable(CAP_SYS_ADMIN)) { - err=-EPERM; - goto out_unlock; - } + sma = container_of(ipcp, struct sem_array, sem_perm); err = security_sem_semctl(sma, cmd); if (err) diff -puN ipc/shm.c~ipc-consolidate-all-xxxctl_down-functions ipc/shm.c --- a/ipc/shm.c~ipc-consolidate-all-xxxctl_down-functions +++ a/ipc/shm.c @@ -126,18 +126,6 @@ static inline struct shmid_kernel *shm_l return container_of(ipcp, struct shmid_kernel, shm_perm); } -static inline struct shmid_kernel *shm_lock_check_down( - struct ipc_namespace *ns, - int id) -{ - struct kern_ipc_perm *ipcp = ipc_lock_check_down(&shm_ids(ns), id); - - if (IS_ERR(ipcp)) - return (struct shmid_kernel *)ipcp; - - return container_of(ipcp, struct shmid_kernel, shm_perm); -} - /* * shm_lock_(check_) routines are called in the paths where the rw_mutex * is not held. @@ -621,33 +609,11 @@ static int shmctl_down(struct ipc_namesp return -EFAULT; } - down_write(&shm_ids(ns).rw_mutex); - shp = shm_lock_check_down(ns, shmid); - if (IS_ERR(shp)) { - err = PTR_ERR(shp); - goto out_up; - } - - ipcp = &shp->shm_perm; - - err = audit_ipc_obj(ipcp); - if (err) - goto out_unlock; - - if (cmd == IPC_SET) { - err = audit_ipc_set_perm(0, shmid64.shm_perm.uid, - shmid64.shm_perm.gid, - shmid64.shm_perm.mode); - if (err) - goto out_unlock; - } + ipcp = ipcctl_pre_down(&shm_ids(ns), shmid, cmd, &shmid64.shm_perm, 0); + if (IS_ERR(ipcp)) + return PTR_ERR(ipcp); - if (current->euid != ipcp->uid && - current->euid != ipcp->cuid && - !capable(CAP_SYS_ADMIN)) { - err = -EPERM; - goto out_unlock; - } + shp = container_of(ipcp, struct shmid_kernel, shm_perm); err = security_shm_shmctl(shp, cmd); if (err) diff -puN ipc/util.c~ipc-consolidate-all-xxxctl_down-functions ipc/util.c --- a/ipc/util.c~ipc-consolidate-all-xxxctl_down-functions +++ a/ipc/util.c @@ -824,6 +824,57 @@ void ipc_update_perm(struct ipc64_perm * | (in->mode & S_IRWXUGO); } +/** + * ipcctl_pre_down - retrieve an ipc and check permissions for some IPC_XXX cmd + * @ids: the table of ids where to look for the ipc + * @id: the id of the ipc to retrieve + * @cmd: the cmd to check + * @perm: the permission to set + * @extra_perm: one extra permission parameter used by msq + * + * This function does some common audit and permissions check for some IPC_XXX + * cmd and is called from semctl_down, shmctl_down and msgctl_down. + * It must be called without any lock held and + * - retrieves the ipc with the given id in the given table. + * - performs some audit and permission check, depending on the given cmd + * - returns the ipc with both ipc and rw_mutex locks held in case of success + * or an err-code without any lock held otherwise. + */ +struct kern_ipc_perm *ipcctl_pre_down(struct ipc_ids *ids, int id, int cmd, + struct ipc64_perm *perm, int extrat_perm) +{ + struct kern_ipc_perm *ipcp; + int err; + + down_write(&ids->rw_mutex); + ipcp = ipc_lock_check_down(ids, id); + if (IS_ERR(ipcp)) { + err = PTR_ERR(ipcp); + goto out_up; + } + + err = audit_ipc_obj(ipcp); + if (err) + goto out_unlock; + + if (cmd == IPC_SET) { + err = audit_ipc_set_perm(extrat_perm, perm->uid, + perm->gid, perm->mode); + if (err) + goto out_unlock; + } + if (current->euid == ipcp->cuid || + current->euid == ipcp->uid || capable(CAP_SYS_ADMIN)) + return ipcp; + + err = -EPERM; +out_unlock: + ipc_unlock(ipcp); +out_up: + up_write(&ids->rw_mutex); + return ERR_PTR(err); +} + #ifdef __ARCH_WANT_IPC_PARSE_VERSION diff -puN ipc/util.h~ipc-consolidate-all-xxxctl_down-functions ipc/util.h --- a/ipc/util.h~ipc-consolidate-all-xxxctl_down-functions +++ a/ipc/util.h @@ -113,6 +113,8 @@ struct kern_ipc_perm *ipc_lock(struct ip void kernel_to_ipc64_perm(struct kern_ipc_perm *in, struct ipc64_perm *out); void ipc64_perm_to_ipc_perm(struct ipc64_perm *in, struct ipc_perm *out); void ipc_update_perm(struct ipc64_perm *in, struct kern_ipc_perm *out); +struct kern_ipc_perm *ipcctl_pre_down(struct ipc_ids *ids, int id, int cmd, + struct ipc64_perm *perm, int extrat_perm); #if defined(__ia64__) || defined(__x86_64__) || defined(__hppa__) || defined(__XTENSA__) /* On IA-64, we always use the "64-bit version" of the IPC structures. */ _ Patches currently in -mm which might be from pierre.peiffer@xxxxxxxx are origin.patch ipc-use-ipc_buildid-directly-from-ipc_addid.patch ipc-use-ipc_buildid-directly-from-ipc_addid-cleanup.patch ipc-scale-msgmni-to-the-amount-of-lowmem.patch ipc-scale-msgmni-to-the-number-of-ipc-namespaces.patch ipc-define-the-slab_memory_callback-priority-as-a-constant.patch ipc-recompute-msgmni-on-memory-add--remove.patch ipc-invoke-the-ipcns-notifier-chain-as-a-work-item.patch ipc-recompute-msgmni-on-ipc-namespace-creation-removal.patch ipc-do-not-recompute-msgmni-anymore-if-explicitly-set-by-user.patch ipc-re-enable-msgmni-automatic-recomputing-msgmni-if-set-to-negative.patch ipc-semaphores-code-factorisation.patch ipc-shared-memory-introduce-shmctl_down.patch ipc-message-queues-introduce-msgctl_down.patch ipc-semaphores-move-the-rwmutex-handling-inside-semctl_down.patch ipc-semaphores-remove-one-unused-parameter-from-semctl_down.patch ipc-get-rid-of-the-use-_setbuf-structure.patch ipc-introduce-ipc_update_perm.patch ipc-consolidate-all-xxxctl_down-functions.patch - To unsubscribe from this list: send the line "unsubscribe mm-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html