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

Thomas Gleixner
2009-03-12T13:44:20Z

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;