linux-kernel - Re: [PATCH 2/6] Additional (get|put)_futex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.0903121207520.29264@localhost.localdomain>
Date:	Thu, 12 Mar 2009 14:42:43 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Ingo Molnar <mingo@...e.hu>
cc:	Darren Hart <dvhltc@...ibm.com>, linux-kernel@...r.kernel.org,
	Peter Zijlstra <peterz@...radead.org>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH 2/6] Additional (get|put)_futex_key() fixes

On Thu, 12 Mar 2009, Ingo Molnar wrote:
> 
> * Darren Hart <dvhltc@...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@...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@...ibm.com>
Signed-off-by: Thomas Gleixner <tglx@...utronix.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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/