Re: [PATCH 2/6] Additional (get|put)_futex_key() fixes

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 12 Mar 2009, Ingo Molnar wrote:
> 
> * Darren Hart <dvhltc@us.ibm.com> wrote:
> 
> > futex_requeue and futex_lock_pi still had some bad 
> > (get|put)_futex_key() usage. This patch adds the missing 
> > put_futex_keys() and corrects a goto in futex_lock_pi() to 
> > avoid a double get.
> > 
> > Build and boot tested on a 4 way Intel x86_64 workstation.  
> > Passes basic pthread_mutex and PI tests out of 
> > ltp/testcases/realtime.
> 
> hm, how bad is the impact - do we need this in v2.6.29?

I think so. We leak key references in some of the error/retry code
pathes.  Darrens patch does not apply to mainline. Backport below.

Thanks,

	tglx
---
Subject: futex: fix key reference leaks
From: Darren Hart <dvhltc@us.ibm.com>
Date: Thu, 12 Mar 2009 12:10:01 +0100

Impact: bugfix

futex_wake_op, futex_requeue, futex_lock_pi and futex_unlock_pi still
had some bad (get|put)_futex_key() usage. This patch adds the missing
put_futex_keys() and corrects a goto in futex_lock_pi() to avoid a
double get.

[ tglx: backport to mainline ]

Signed-off-by: Darren Hart <dvhltc@us.ibm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---

 kernel/futex.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/futex.c
===================================================================
--- linux-2.6.orig/kernel/futex.c
+++ linux-2.6/kernel/futex.c
@@ -803,6 +803,9 @@ retry:
 			goto retry;
 		}
 
+		put_futex_key(fshared, &key2);
+		put_futex_key(fshared, &key1);
+
 		ret = get_user(dummy, uaddr2);
 		if (ret)
 			return ret;
@@ -881,12 +884,15 @@ retry:
 			if (hb1 != hb2)
 				spin_unlock(&hb2->lock);
 
+			put_futex_key(fshared, &key2);
+			put_futex_key(fshared, &key1);
+
 			ret = get_user(curval, uaddr1);
 
 			if (!ret)
 				goto retry;
 
-			goto out_put_keys;
+			return ret;
 		}
 		if (curval != *cmpval) {
 			ret = -EAGAIN;
@@ -1459,7 +1465,7 @@ retry_locked:
 			 */
 			queue_unlock(&q, hb);
 			cond_resched();
-			goto retry;
+			goto retry_unlocked;
 
 		case -ESRCH:
 			/*
@@ -1598,6 +1604,7 @@ uaddr_faulted:
 		goto retry_unlocked;
 	}
 
+	put_futex_key(fshared, &q.key);
 	ret = get_user(uval, uaddr);
 	if (!ret)
 		goto retry;
@@ -1709,6 +1716,8 @@ pi_faulted:
 		goto retry_unlocked;
 	}
 
+	put_futex_key(fshared, &key);
+
 	ret = get_user(uval, uaddr);
 	if (!ret)
 		goto retry;