Commit 802d0db8 authored by Linus Torvalds's avatar Linus Torvalds
Browse files

Merge branch 'ipc-cleanups' 

Merge ipc fixes and cleanups from my IPC branch.

The ipc locking has always been pretty ugly, and the scalability fixes
to some degree made it even less readable.  We had two cases of double
unlocks in error paths due to this (one rcu read unlock, one semaphore
unlock), and this fixes the bugs I found while trying to clean things up
a bit so that we are less likely to have more.

* ipc-cleanups:
  ipc: simplify rcu_read_lock() in semctl_nolock()
  ipc: simplify semtimedop/semctl_main() common error path handling
  ipc: move sem_obtain_lock() rcu locking into the only caller
  ipc: fix double sem unlock in semctl error path
  ipc: move the rcu_read_lock() from sem_lock_and_putref() into callers
  ipc: sem_putref() does not need the semaphore lock any more
  ipc: move rcu_read_unlock() out of sem_unlock() and into callers
parents 1aaf6d3d 941b0304

ipc/sem.c

+42 −44
Original line number Original line Diff line number Diff line
@@ -264,12 +264,13 @@ static inline void sem_unlock(struct sem_array *sma, int locknum) 
		struct sem *sem = sma->sem_base + locknum;

		struct sem *sem = sma->sem_base + locknum;

		spin_unlock(&sem->lock);

		spin_unlock(&sem->lock);

	}

	}

	rcu_read_unlock();

}

}





/*

/*

 * sem_lock_(check_) routines are called in the paths where the rw_mutex

 * sem_lock_(check_) routines are called in the paths where the rw_mutex

 * is not held.

 * is not held.

 *

 * The caller holds the RCU read lock.

 */

 */

static inline struct sem_array *sem_obtain_lock(struct ipc_namespace *ns,

static inline struct sem_array *sem_obtain_lock(struct ipc_namespace *ns,

			int id, struct sembuf *sops, int nsops, int *locknum)

			int id, struct sembuf *sops, int nsops, int *locknum)
@@ -277,12 +278,9 @@ static inline struct sem_array *sem_obtain_lock(struct ipc_namespace *ns, 
	struct kern_ipc_perm *ipcp;

	struct kern_ipc_perm *ipcp;

	struct sem_array *sma;

	struct sem_array *sma;





	rcu_read_lock();

	ipcp = ipc_obtain_object(&sem_ids(ns), id);

	ipcp = ipc_obtain_object(&sem_ids(ns), id);

	if (IS_ERR(ipcp)) {

	if (IS_ERR(ipcp))

		sma = ERR_CAST(ipcp);

		return ERR_CAST(ipcp);

		goto err;

	}





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

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

	*locknum = sem_lock(sma, sops, nsops);

	*locknum = sem_lock(sma, sops, nsops);
@@ -294,10 +292,7 @@ static inline struct sem_array *sem_obtain_lock(struct ipc_namespace *ns, 
		return container_of(ipcp, struct sem_array, sem_perm);

		return container_of(ipcp, struct sem_array, sem_perm);





	sem_unlock(sma, *locknum);

	sem_unlock(sma, *locknum);

	sma = ERR_PTR(-EINVAL);

	return ERR_PTR(-EINVAL);

err:

	rcu_read_unlock();

	return sma;

}

}





static inline struct sem_array *sem_obtain_object(struct ipc_namespace *ns, int id)

static inline struct sem_array *sem_obtain_object(struct ipc_namespace *ns, int id)
@@ -323,15 +318,13 @@ static inline struct sem_array *sem_obtain_object_check(struct ipc_namespace *ns 




static inline void sem_lock_and_putref(struct sem_array *sma)

static inline void sem_lock_and_putref(struct sem_array *sma)

{

{

	rcu_read_lock();

	sem_lock(sma, NULL, -1);

	sem_lock(sma, NULL, -1);

	ipc_rcu_putref(sma);

	ipc_rcu_putref(sma);

}

}





static inline void sem_putref(struct sem_array *sma)

static inline void sem_putref(struct sem_array *sma)

{

{

	sem_lock_and_putref(sma);

	ipc_rcu_putref(sma);

	sem_unlock(sma, -1);

}

}





static inline void sem_rmid(struct ipc_namespace *ns, struct sem_array *s)

static inline void sem_rmid(struct ipc_namespace *ns, struct sem_array *s)
@@ -435,6 +428,7 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params) 
	sma->sem_nsems = nsems;

	sma->sem_nsems = nsems;

	sma->sem_ctime = get_seconds();

	sma->sem_ctime = get_seconds();

	sem_unlock(sma, -1);

	sem_unlock(sma, -1);

	rcu_read_unlock();





	return sma->sem_perm.id;

	return sma->sem_perm.id;

}

}
@@ -874,6 +868,7 @@ static void freeary(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp) 
	/* Remove the semaphore set from the IDR */

	/* Remove the semaphore set from the IDR */

	sem_rmid(ns, sma);

	sem_rmid(ns, sma);

	sem_unlock(sma, -1);

	sem_unlock(sma, -1);

	rcu_read_unlock();





	wake_up_sem_queue_do(&tasks);

	wake_up_sem_queue_do(&tasks);

	ns->used_sems -= sma->sem_nsems;

	ns->used_sems -= sma->sem_nsems;
@@ -953,8 +948,8 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid, 




		memset(&tbuf, 0, sizeof(tbuf));

		memset(&tbuf, 0, sizeof(tbuf));





		if (cmd == SEM_STAT) {

		rcu_read_lock();

		rcu_read_lock();

		if (cmd == SEM_STAT) {

			sma = sem_obtain_object(ns, semid);

			sma = sem_obtain_object(ns, semid);

			if (IS_ERR(sma)) {

			if (IS_ERR(sma)) {

				err = PTR_ERR(sma);

				err = PTR_ERR(sma);
@@ -962,7 +957,6 @@ static int semctl_nolock(struct ipc_namespace *ns, int semid, 
			}

			}

			id = sma->sem_perm.id;

			id = sma->sem_perm.id;

		} else {

		} else {

			rcu_read_lock();

			sma = sem_obtain_object_check(ns, semid);

			sma = sem_obtain_object_check(ns, semid);

			if (IS_ERR(sma)) {

			if (IS_ERR(sma)) {

				err = PTR_ERR(sma);

				err = PTR_ERR(sma);
@@ -1055,6 +1049,7 @@ static int semctl_setval(struct ipc_namespace *ns, int semid, int semnum, 
	/* maybe some queued-up processes were waiting for this */

	/* maybe some queued-up processes were waiting for this */

	do_smart_update(sma, NULL, 0, 0, &tasks);

	do_smart_update(sma, NULL, 0, 0, &tasks);

	sem_unlock(sma, -1);

	sem_unlock(sma, -1);

	rcu_read_unlock();

	wake_up_sem_queue_do(&tasks);

	wake_up_sem_queue_do(&tasks);

	return 0;

	return 0;

}

}
@@ -1081,17 +1076,12 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, 
	nsems = sma->sem_nsems;

	nsems = sma->sem_nsems;





	err = -EACCES;

	err = -EACCES;

	if (ipcperms(ns, &sma->sem_perm,

	if (ipcperms(ns, &sma->sem_perm, cmd == SETALL ? S_IWUGO : S_IRUGO))

			cmd == SETALL ? S_IWUGO : S_IRUGO)) {

		goto out_rcu_wakeup;

		rcu_read_unlock();

		goto out_wakeup;

	}





	err = security_sem_semctl(sma, cmd);

	err = security_sem_semctl(sma, cmd);

	if (err) {

	if (err)

		rcu_read_unlock();

		goto out_rcu_wakeup;

		goto out_wakeup;

	}





	err = -EACCES;

	err = -EACCES;

	switch (cmd) {

	switch (cmd) {
@@ -1104,19 +1094,23 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, 
		if(nsems > SEMMSL_FAST) {

		if(nsems > SEMMSL_FAST) {

			if (!ipc_rcu_getref(sma)) {

			if (!ipc_rcu_getref(sma)) {

				sem_unlock(sma, -1);

				sem_unlock(sma, -1);

				rcu_read_unlock();

				err = -EIDRM;

				err = -EIDRM;

				goto out_free;

				goto out_free;

			}

			}

			sem_unlock(sma, -1);

			sem_unlock(sma, -1);

			rcu_read_unlock();

			sem_io = ipc_alloc(sizeof(ushort)*nsems);

			sem_io = ipc_alloc(sizeof(ushort)*nsems);

			if(sem_io == NULL) {

			if(sem_io == NULL) {

				sem_putref(sma);

				sem_putref(sma);

				return -ENOMEM;

				return -ENOMEM;

			}

			}





			rcu_read_lock();

			sem_lock_and_putref(sma);

			sem_lock_and_putref(sma);

			if (sma->sem_perm.deleted) {

			if (sma->sem_perm.deleted) {

				sem_unlock(sma, -1);

				sem_unlock(sma, -1);

				rcu_read_unlock();

				err = -EIDRM;

				err = -EIDRM;

				goto out_free;

				goto out_free;

			}

			}
@@ -1124,6 +1118,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, 
		for (i = 0; i < sma->sem_nsems; i++)

		for (i = 0; i < sma->sem_nsems; i++)

			sem_io[i] = sma->sem_base[i].semval;

			sem_io[i] = sma->sem_base[i].semval;

		sem_unlock(sma, -1);

		sem_unlock(sma, -1);

		rcu_read_unlock();

		err = 0;

		err = 0;

		if(copy_to_user(array, sem_io, nsems*sizeof(ushort)))

		if(copy_to_user(array, sem_io, nsems*sizeof(ushort)))

			err = -EFAULT;

			err = -EFAULT;
@@ -1161,9 +1156,11 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, 
				goto out_free;

				goto out_free;

			}

			}

		}

		}

		rcu_read_lock();

		sem_lock_and_putref(sma);

		sem_lock_and_putref(sma);

		if (sma->sem_perm.deleted) {

		if (sma->sem_perm.deleted) {

			sem_unlock(sma, -1);

			sem_unlock(sma, -1);

			rcu_read_unlock();

			err = -EIDRM;

			err = -EIDRM;

			goto out_free;

			goto out_free;

		}

		}
@@ -1185,10 +1182,8 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, 
	/* GETVAL, GETPID, GETNCTN, GETZCNT: fall-through */

	/* GETVAL, GETPID, GETNCTN, GETZCNT: fall-through */

	}

	}

	err = -EINVAL;

	err = -EINVAL;

	if (semnum < 0 || semnum >= nsems) {

	if (semnum < 0 || semnum >= nsems)

		rcu_read_unlock();

		goto out_rcu_wakeup;

		goto out_wakeup;

	}





	sem_lock(sma, NULL, -1);

	sem_lock(sma, NULL, -1);

	curr = &sma->sem_base[semnum];

	curr = &sma->sem_base[semnum];
@@ -1210,7 +1205,8 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, 




out_unlock:

out_unlock:

	sem_unlock(sma, -1);

	sem_unlock(sma, -1);

out_wakeup:

out_rcu_wakeup:

	rcu_read_unlock();

	wake_up_sem_queue_do(&tasks);

	wake_up_sem_queue_do(&tasks);

out_free:

out_free:

	if(sem_io != fast_sem_io)

	if(sem_io != fast_sem_io)
@@ -1272,7 +1268,7 @@ static int semctl_down(struct ipc_namespace *ns, int semid, 
	err = security_sem_semctl(sma, cmd);

	err = security_sem_semctl(sma, cmd);

	if (err) {

	if (err) {

		rcu_read_unlock();

		rcu_read_unlock();

		goto out_unlock;

		goto out_up;

	}

	}





	switch(cmd){

	switch(cmd){
@@ -1295,6 +1291,7 @@ static int semctl_down(struct ipc_namespace *ns, int semid, 




out_unlock:

out_unlock:

	sem_unlock(sma, -1);

	sem_unlock(sma, -1);

	rcu_read_unlock();

out_up:

out_up:

	up_write(&sem_ids(ns).rw_mutex);

	up_write(&sem_ids(ns).rw_mutex);

	return err;

	return err;
@@ -1443,9 +1440,11 @@ static struct sem_undo *find_alloc_undo(struct ipc_namespace *ns, int semid) 
	}

	}





	/* step 3: Acquire the lock on semaphore array */

	/* step 3: Acquire the lock on semaphore array */

	rcu_read_lock();

	sem_lock_and_putref(sma);

	sem_lock_and_putref(sma);

	if (sma->sem_perm.deleted) {

	if (sma->sem_perm.deleted) {

		sem_unlock(sma, -1);

		sem_unlock(sma, -1);

		rcu_read_unlock();

		kfree(new);

		kfree(new);

		un = ERR_PTR(-EIDRM);

		un = ERR_PTR(-EIDRM);

		goto out;

		goto out;
@@ -1472,7 +1471,6 @@ static struct sem_undo *find_alloc_undo(struct ipc_namespace *ns, int semid) 




success:

success:

	spin_unlock(&ulp->lock);

	spin_unlock(&ulp->lock);

	rcu_read_lock();

	sem_unlock(sma, -1);

	sem_unlock(sma, -1);

out:

out:

	return un;

	return un;
@@ -1579,22 +1577,16 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, 
	}

	}





	error = -EFBIG;

	error = -EFBIG;

	if (max >= sma->sem_nsems) {

	if (max >= sma->sem_nsems)

		rcu_read_unlock();

		goto out_rcu_wakeup;

		goto out_wakeup;

	}





	error = -EACCES;

	error = -EACCES;

	if (ipcperms(ns, &sma->sem_perm, alter ? S_IWUGO : S_IRUGO)) {

	if (ipcperms(ns, &sma->sem_perm, alter ? S_IWUGO : S_IRUGO))

		rcu_read_unlock();

		goto out_rcu_wakeup;

		goto out_wakeup;

	}





	error = security_sem_semop(sma, sops, nsops, alter);

	error = security_sem_semop(sma, sops, nsops, alter);

	if (error) {

	if (error)

		rcu_read_unlock();

		goto out_rcu_wakeup;

		goto out_wakeup;

	}





	/*

	/*

	 * semid identifiers are not unique - find_alloc_undo may have

	 * semid identifiers are not unique - find_alloc_undo may have
@@ -1648,6 +1640,7 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, 
sleep_again:

sleep_again:

	current->state = TASK_INTERRUPTIBLE;

	current->state = TASK_INTERRUPTIBLE;

	sem_unlock(sma, locknum);

	sem_unlock(sma, locknum);

	rcu_read_unlock();





	if (timeout)

	if (timeout)

		jiffies_left = schedule_timeout(jiffies_left);

		jiffies_left = schedule_timeout(jiffies_left);
@@ -1669,6 +1662,7 @@ sleep_again: 
		goto out_free;

		goto out_free;

	}

	}





	rcu_read_lock();

	sma = sem_obtain_lock(ns, semid, sops, nsops, &locknum);

	sma = sem_obtain_lock(ns, semid, sops, nsops, &locknum);





	/*

	/*
@@ -1680,6 +1674,7 @@ sleep_again: 
	 * Array removed? If yes, leave without sem_unlock().

	 * Array removed? If yes, leave without sem_unlock().

	 */

	 */

	if (IS_ERR(sma)) {

	if (IS_ERR(sma)) {

		rcu_read_unlock();

		goto out_free;

		goto out_free;

	}

	}
@@ -1709,7 +1704,8 @@ sleep_again: 




out_unlock_free:

out_unlock_free:

	sem_unlock(sma, locknum);

	sem_unlock(sma, locknum);

out_wakeup:

out_rcu_wakeup:

	rcu_read_unlock();

	wake_up_sem_queue_do(&tasks);

	wake_up_sem_queue_do(&tasks);

out_free:

out_free:

	if(sops != fast_sops)

	if(sops != fast_sops)
@@ -1801,6 +1797,7 @@ void exit_sem(struct task_struct *tsk) 
			 * exactly the same semid. Nothing to do.

			 * exactly the same semid. Nothing to do.

			 */

			 */

			sem_unlock(sma, -1);

			sem_unlock(sma, -1);

			rcu_read_unlock();

			continue;

			continue;

		}

		}
@@ -1841,6 +1838,7 @@ void exit_sem(struct task_struct *tsk) 
		INIT_LIST_HEAD(&tasks);

		INIT_LIST_HEAD(&tasks);

		do_smart_update(sma, NULL, 0, 1, &tasks);

		do_smart_update(sma, NULL, 0, 1, &tasks);

		sem_unlock(sma, -1);

		sem_unlock(sma, -1);

		rcu_read_unlock();

		wake_up_sem_queue_do(&tasks);

		wake_up_sem_queue_do(&tasks);





		kfree_rcu(un, rcu);

		kfree_rcu(un, rcu);

CRA Git | Maintained and supported by SUSTech CRA and CCSE