Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7b4cc5d8 authored by Davidlohr Bueso's avatar Davidlohr Bueso Committed by Linus Torvalds
Browse files

ipc: move locking out of ipcctl_pre_down_nolock 



This function currently acquires both the rw_mutex and the rcu lock on
successful lookups, leaving the callers to explicitly unlock them,
creating another two level locking situation.

Make the callers (including those that still use ipcctl_pre_down())
explicitly lock and unlock the rwsem and rcu lock.

Signed-off-by: default avatarDavidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Rik van Riel <riel@redhat.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent cf9d5d78

ipc/msg.c

+17 −7
Original line number Diff line number Diff line
@@ -407,31 +407,38 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd, 
			return -EFAULT;

	}



	down_write(&msg_ids(ns).rw_mutex);

	rcu_read_lock();



	ipcp = ipcctl_pre_down(ns, &msg_ids(ns), msqid, cmd,

			       &msqid64.msg_perm, msqid64.msg_qbytes);

	if (IS_ERR(ipcp))

		return PTR_ERR(ipcp);

	if (IS_ERR(ipcp)) {

		err = PTR_ERR(ipcp);

		/* the ipc lock is not held upon failure */

		goto out_unlock1;

	}



	msq = container_of(ipcp, struct msg_queue, q_perm);



	err = security_msg_queue_msgctl(msq, cmd);

	if (err)

		goto out_unlock;

		goto out_unlock0;



	switch (cmd) {

	case IPC_RMID:

		/* freeque unlocks the ipc object and rcu */

		freeque(ns, ipcp);

		goto out_up;

	case IPC_SET:

		if (msqid64.msg_qbytes > ns->msg_ctlmnb &&

		    !capable(CAP_SYS_RESOURCE)) {

			err = -EPERM;

			goto out_unlock;

			goto out_unlock0;

		}



		err = ipc_update_perm(&msqid64.msg_perm, ipcp);

		if (err)

			goto out_unlock;

			goto out_unlock0;



		msq->q_qbytes = msqid64.msg_qbytes;
@@ -448,8 +455,11 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd, 
	default:

		err = -EINVAL;

	}

out_unlock:

	msg_unlock(msq);



out_unlock0:

	ipc_unlock_object(&msq->q_perm);

out_unlock1:

	rcu_read_unlock();

out_up:

	up_write(&msg_ids(ns).rw_mutex);

	return err;

ipc/sem.c

+16 −11
Original line number Diff line number Diff line
@@ -1289,39 +1289,44 @@ static int semctl_down(struct ipc_namespace *ns, int semid, 
			return -EFAULT;

	}



	down_write(&sem_ids(ns).rw_mutex);

	rcu_read_lock();



	ipcp = ipcctl_pre_down_nolock(ns, &sem_ids(ns), semid, cmd,

				      &semid64.sem_perm, 0);

	if (IS_ERR(ipcp))

		return PTR_ERR(ipcp);

	if (IS_ERR(ipcp)) {

		err = PTR_ERR(ipcp);

		/* the ipc lock is not held upon failure */

		goto out_unlock1;

	}



	sma = container_of(ipcp, struct sem_array, sem_perm);



	err = security_sem_semctl(sma, cmd);

	if (err) {

		rcu_read_unlock();

		goto out_up;

	}

	if (err)

		goto out_unlock1;



	switch (cmd) {

	case IPC_RMID:

		sem_lock(sma, NULL, -1);

		/* freeary unlocks the ipc object and rcu */

		freeary(ns, ipcp);

		goto out_up;

	case IPC_SET:

		sem_lock(sma, NULL, -1);

		err = ipc_update_perm(&semid64.sem_perm, ipcp);

		if (err)

			goto out_unlock;

			goto out_unlock0;

		sma->sem_ctime = get_seconds();

		break;

	default:

		rcu_read_unlock();

		err = -EINVAL;

		goto out_up;

		goto out_unlock1;

	}



out_unlock:

out_unlock0:

	sem_unlock(sma, -1);

out_unlock1:

	rcu_read_unlock();

out_up:

	up_write(&sem_ids(ns).rw_mutex);

ipc/shm.c

+17 −6
Original line number Diff line number Diff line
@@ -757,31 +757,42 @@ static int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd, 
			return -EFAULT;

	}



	down_write(&shm_ids(ns).rw_mutex);

	rcu_read_lock();



	ipcp = ipcctl_pre_down(ns, &shm_ids(ns), shmid, cmd,

			       &shmid64.shm_perm, 0);

	if (IS_ERR(ipcp))

		return PTR_ERR(ipcp);

	if (IS_ERR(ipcp)) {

		err = PTR_ERR(ipcp);

		/* the ipc lock is not held upon failure */

		goto out_unlock1;

	}



	shp = container_of(ipcp, struct shmid_kernel, shm_perm);



	err = security_shm_shmctl(shp, cmd);

	if (err)

		goto out_unlock;

		goto out_unlock0;



	switch (cmd) {

	case IPC_RMID:

		/* do_shm_rmid unlocks the ipc object and rcu */

		do_shm_rmid(ns, ipcp);

		goto out_up;

	case IPC_SET:

		err = ipc_update_perm(&shmid64.shm_perm, ipcp);

		if (err)

			goto out_unlock;

			goto out_unlock0;

		shp->shm_ctim = get_seconds();

		break;

	default:

		err = -EINVAL;

	}

out_unlock:

	shm_unlock(shp);



out_unlock0:

	ipc_unlock_object(&shp->shm_perm);

out_unlock1:

	rcu_read_unlock();

out_up:

	up_write(&shm_ids(ns).rw_mutex);

	return err;

ipc/util.c

+6 −15
Original line number Diff line number Diff line
@@ -746,8 +746,10 @@ int ipc_update_perm(struct ipc64_perm *in, struct kern_ipc_perm *out) 
 * 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

 *  - returns the ipc with the ipc lock held in case of success

 *    or an err-code without any lock held otherwise.

 *

 * Call holding the both the rw_mutex and the rcu read lock.

 */

struct kern_ipc_perm *ipcctl_pre_down(struct ipc_namespace *ns,

				      struct ipc_ids *ids, int id, int cmd,
@@ -772,13 +774,10 @@ struct kern_ipc_perm *ipcctl_pre_down_nolock(struct ipc_namespace *ns, 
	int err = -EPERM;

	struct kern_ipc_perm *ipcp;



	down_write(&ids->rw_mutex);

	rcu_read_lock();



	ipcp = ipc_obtain_object_check(ids, id);

	if (IS_ERR(ipcp)) {

		err = PTR_ERR(ipcp);

		goto out_up;

		goto err;

	}



	audit_ipc_obj(ipcp);
@@ -789,16 +788,8 @@ struct kern_ipc_perm *ipcctl_pre_down_nolock(struct ipc_namespace *ns, 
	euid = current_euid();

	if (uid_eq(euid, ipcp->cuid) || uid_eq(euid, ipcp->uid)  ||

	    ns_capable(ns->user_ns, CAP_SYS_ADMIN))

		return ipcp;



out_up:

	/*

	 * Unsuccessful lookup, unlock and return

	 * the corresponding error.

	 */

	rcu_read_unlock();

	up_write(&ids->rw_mutex);



		return ipcp; /* successful lookup */

err:

	return ERR_PTR(err);

}