All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] FRV: Semaphore implementation race fix
@ 2005-02-04 17:31 David Howells
  0 siblings, 0 replies; only message in thread
From: David Howells @ 2005-02-04 17:31 UTC (permalink / raw)
  To: torvalds, akpm; +Cc: linux-kernel


The attached patch fixes a race in the FRV arch's semaphore implementation.
The same type of fixes were applied to the rw-semaphore implementations to fix
the same races there.

The race involved the on-stack record linked into the semaphore's queue by the
down() executed by a process now sleeping on the semaphore going away and the
sleeping task going away before the process that woke it up during up()
processing had finished with those structures.

Signed-Off-By: David Howells <dhowells@redhat.com>
---
warthog>diffstat -p1 frv-semaphore-2611rc3.diff 
 arch/frv/kernel/semaphore.c |   26 ++++++++++++++++++++------
 1 files changed, 20 insertions(+), 6 deletions(-)

diff -uNrp /warthog/kernels/linux-2.6.11-rc3/arch/frv/kernel/semaphore.c linux-2.6.11-rc3-frv/arch/frv/kernel/semaphore.c
--- /warthog/kernels/linux-2.6.11-rc3/arch/frv/kernel/semaphore.c	2005-02-04 11:49:30.000000000 +0000
+++ linux-2.6.11-rc3-frv/arch/frv/kernel/semaphore.c	2005-02-04 12:34:46.000000000 +0000
@@ -43,17 +43,18 @@ void __down(struct semaphore *sem, unsig
 	struct task_struct *tsk = current;
 	struct sem_waiter waiter;
 
-	semtrace(sem,"Entering __down");
+	semtrace(sem, "Entering __down");
 
 	/* set up my own style of waitqueue */
-	waiter.task	= tsk;
+	waiter.task = tsk;
+	get_task_struct(tsk);
 
 	list_add_tail(&waiter.list, &sem->wait_list);
 
 	/* we don't need to touch the semaphore struct anymore */
 	spin_unlock_irqrestore(&sem->wait_lock, flags);
 
-	/* wait to be given the lock */
+	/* wait to be given the semaphore */
 	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
 
 	for (;;) {
@@ -64,7 +65,7 @@ void __down(struct semaphore *sem, unsig
 	}
 
 	tsk->state = TASK_RUNNING;
-	semtrace(sem,"Leaving __down");
+	semtrace(sem, "Leaving __down");
 }
 
 EXPORT_SYMBOL(__down);
@@ -83,6 +84,7 @@ int __down_interruptible(struct semaphor
 
 	/* set up my own style of waitqueue */
 	waiter.task = tsk;
+	get_task_struct(tsk);
 
 	list_add_tail(&waiter.list, &sem->wait_list);
 
@@ -91,7 +93,7 @@ int __down_interruptible(struct semaphor
 
 	spin_unlock_irqrestore(&sem->wait_lock, flags);
 
-	/* wait to be given the lock */
+	/* wait to be given the semaphore */
 	ret = 0;
 	for (;;) {
 		if (list_empty(&waiter.list))
@@ -116,6 +118,8 @@ int __down_interruptible(struct semaphor
 	}
 
 	spin_unlock_irqrestore(&sem->wait_lock, flags);
+	if (ret == -EINTR)
+		put_task_struct(current);
 	goto out;
 }
 
@@ -127,14 +131,24 @@ EXPORT_SYMBOL(__down_interruptible);
  */
 void __up(struct semaphore *sem)
 {
+	struct task_struct *tsk;
 	struct sem_waiter *waiter;
 
 	semtrace(sem,"Entering __up");
 
 	/* grant the token to the process at the front of the queue */
 	waiter = list_entry(sem->wait_list.next, struct sem_waiter, list);
+
+	/* We must be careful not to touch 'waiter' after we set ->task = NULL.
+	 * It is an allocated on the waiter's stack and may become invalid at
+	 * any time after that point (due to a wakeup from another source).
+	 */
 	list_del_init(&waiter->list);
-	wake_up_process(waiter->task);
+	tsk = waiter->task;
+	mb();
+	waiter->task = NULL;
+	wake_up_process(tsk);
+	put_task_struct(tsk);
 
 	semtrace(sem,"Leaving __up");
 }

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2005-02-04 17:39 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-04 17:31 [PATCH] FRV: Semaphore implementation race fix David Howells

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.