All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: torvalds@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,
	linux-arch@vger.kernel.org, bernd@bzed.de, joy@entuzijast.net,
	fabbione@ubuntu.com, arnd@arndb.de
Subject: Re: Fix for sparc64 cpu hangs.
Date: Tue, 06 Nov 2007 21:13:56 -0800 (PST)	[thread overview]
Message-ID: <20071106.211356.60087540.davem@davemloft.net> (raw)
In-Reply-To: <20071106.203433.144763156.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Tue, 06 Nov 2007 20:34:33 -0800 (PST)

> [FUTEX]: Fix address computation in compat code.

Sorry, I just noticed there is a second handle_futex_death()
call in compat_exit_robust_list() which has the same
address computation bug.

Here is an updated patch:

[FUTEX]: Fix address computation in compat code.

compat_exit_robust_list() computes a pointer to the
futex entry in userspace as follows:

	(void __user *)entry + futex_offset

'entry' is a 'struct robust_list __user *', and
'futex_offset' is a 'compat_long_t' (typically a 's32').

Things explode if the 32-bit sign bit is set in futex_offset.

Type promotion sign extends futex_offset to a 64-bit value before
adding it to 'entry'.

This triggered a problem on sparc64 running 32-bit applications which
would lock up a cpu looping forever in the fault handling for the
userspace load in handle_futex_death().

Compat userspace runs with address masking (wherein the cpu zeros out
the top 32-bits of every effective address given to a memory operation
instruction) so the sparc64 fault handler accounts for this by
zero'ing out the top 32-bits of the fault address too.

Since the kernel properly uses the compat_uptr interfaces, kernel side
accesses to compat userspace work too since they will only use
addresses with the top 32-bit clear.

Because of this compat futex layer bug we get into the following loop
when executing the get_user() load near the top of handle_futex_death():

1) load from address '0xfffffffff7f16bd8', FAULT
2) fault handler clears upper 32-bits, processes fault
   for address '0xf7f16bd8' which succeeds
3) goto #1

I want to thank Bernd Zeimetz, Josip Rodin, and Fabio Massimo Di Nitto
for their tireless efforts helping me track down this bug.

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
index 00b5726..1931457 100644
--- a/kernel/futex_compat.c
+++ b/kernel/futex_compat.c
@@ -30,6 +30,15 @@ fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **entry,
 	return 0;
 }
 
+static void __user *futex_uaddr(struct robust_list *entry,
+				compat_long_t futex_offset)
+{
+	compat_uptr_t base = ptr_to_compat(entry);
+	void __user *uaddr = compat_ptr(base + futex_offset);
+
+	return uaddr;
+}
+
 /*
  * Walk curr->robust_list (very carefully, it's a userspace list!)
  * and mark any locks found there dead, and notify any waiters.
@@ -76,11 +85,13 @@ void compat_exit_robust_list(struct task_struct *curr)
 		 * A pending lock might already be on the list, so
 		 * dont process it twice:
 		 */
-		if (entry != pending)
-			if (handle_futex_death((void __user *)entry + futex_offset,
-						curr, pi))
-				return;
+		if (entry != pending) {
+			void __user *uaddr = futex_uaddr(entry,
+							 futex_offset);
 
+			if (handle_futex_death(uaddr, curr, pi))
+				return;
+		}
 		if (rc)
 			return;
 		uentry = next_uentry;
@@ -94,9 +105,11 @@ void compat_exit_robust_list(struct task_struct *curr)
 
 		cond_resched();
 	}
-	if (pending)
-		handle_futex_death((void __user *)pending + futex_offset,
-				   curr, pip);
+	if (pending) {
+		void __user *uaddr = futex_uaddr(pending, futex_offset);
+
+		handle_futex_death(uaddr, curr, pip);
+	}
 }
 
 asmlinkage long

WARNING: multiple messages have this Message-ID (diff)
From: David Miller <davem@davemloft.net>
To: torvalds@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,
	linux-arch@vger.kernel.org, bernd@bzed.de, joy@entuzijast.net,
	fabbione@ubuntu.com, arnd@arndb.de
Subject: Re: Fix for sparc64 cpu hangs.
Date: Wed, 07 Nov 2007 05:13:56 +0000	[thread overview]
Message-ID: <20071106.211356.60087540.davem@davemloft.net> (raw)
In-Reply-To: <20071106.203433.144763156.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Tue, 06 Nov 2007 20:34:33 -0800 (PST)

> [FUTEX]: Fix address computation in compat code.

Sorry, I just noticed there is a second handle_futex_death()
call in compat_exit_robust_list() which has the same
address computation bug.

Here is an updated patch:

[FUTEX]: Fix address computation in compat code.

compat_exit_robust_list() computes a pointer to the
futex entry in userspace as follows:

	(void __user *)entry + futex_offset

'entry' is a 'struct robust_list __user *', and
'futex_offset' is a 'compat_long_t' (typically a 's32').

Things explode if the 32-bit sign bit is set in futex_offset.

Type promotion sign extends futex_offset to a 64-bit value before
adding it to 'entry'.

This triggered a problem on sparc64 running 32-bit applications which
would lock up a cpu looping forever in the fault handling for the
userspace load in handle_futex_death().

Compat userspace runs with address masking (wherein the cpu zeros out
the top 32-bits of every effective address given to a memory operation
instruction) so the sparc64 fault handler accounts for this by
zero'ing out the top 32-bits of the fault address too.

Since the kernel properly uses the compat_uptr interfaces, kernel side
accesses to compat userspace work too since they will only use
addresses with the top 32-bit clear.

Because of this compat futex layer bug we get into the following loop
when executing the get_user() load near the top of handle_futex_death():

1) load from address '0xfffffffff7f16bd8', FAULT
2) fault handler clears upper 32-bits, processes fault
   for address '0xf7f16bd8' which succeeds
3) goto #1

I want to thank Bernd Zeimetz, Josip Rodin, and Fabio Massimo Di Nitto
for their tireless efforts helping me track down this bug.

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
index 00b5726..1931457 100644
--- a/kernel/futex_compat.c
+++ b/kernel/futex_compat.c
@@ -30,6 +30,15 @@ fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **entry,
 	return 0;
 }
 
+static void __user *futex_uaddr(struct robust_list *entry,
+				compat_long_t futex_offset)
+{
+	compat_uptr_t base = ptr_to_compat(entry);
+	void __user *uaddr = compat_ptr(base + futex_offset);
+
+	return uaddr;
+}
+
 /*
  * Walk curr->robust_list (very carefully, it's a userspace list!)
  * and mark any locks found there dead, and notify any waiters.
@@ -76,11 +85,13 @@ void compat_exit_robust_list(struct task_struct *curr)
 		 * A pending lock might already be on the list, so
 		 * dont process it twice:
 		 */
-		if (entry != pending)
-			if (handle_futex_death((void __user *)entry + futex_offset,
-						curr, pi))
-				return;
+		if (entry != pending) {
+			void __user *uaddr = futex_uaddr(entry,
+							 futex_offset);
 
+			if (handle_futex_death(uaddr, curr, pi))
+				return;
+		}
 		if (rc)
 			return;
 		uentry = next_uentry;
@@ -94,9 +105,11 @@ void compat_exit_robust_list(struct task_struct *curr)
 
 		cond_resched();
 	}
-	if (pending)
-		handle_futex_death((void __user *)pending + futex_offset,
-				   curr, pip);
+	if (pending) {
+		void __user *uaddr = futex_uaddr(pending, futex_offset);
+
+		handle_futex_death(uaddr, curr, pip);
+	}
 }
 
 asmlinkage long

  reply	other threads:[~2007-11-07  5:13 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-07  4:34 Fix for sparc64 cpu hangs David Miller
2007-11-07  4:34 ` David Miller
2007-11-07  5:13 ` David Miller [this message]
2007-11-07  5:13   ` David Miller
2007-11-09 20:22   ` Andrew Morton
2007-11-09 20:22     ` Andrew Morton
2007-11-09 22:14     ` David Miller
2007-11-09 22:14       ` David Miller
2007-11-07 14:25 ` Josip Rodin
2007-11-07 14:35 ` Bernd Zeimetz
2007-11-08  0:01 ` David Miller
2007-11-11  6:04 ` David Miller
2007-11-11  6:13 ` David Miller
2007-11-11  6:27 ` Bernd Zeimetz
2007-11-12 13:16 ` Josip Rodin
2007-11-16 21:17 ` Bernd Zeimetz
2007-11-20  6:09 ` David Miller
2007-12-06  8:49 ` David Miller
2007-12-06 10:43 ` Bernd Zeimetz
2007-12-06 11:08 ` David Miller
2007-12-06 12:09 ` Bernd Zeimetz
2007-12-06 13:52 ` David Miller
2007-12-07  8:59 ` David Miller
2007-12-08  0:14 ` Bernd Zeimetz
2007-12-09  8:38 ` David Miller
2007-12-10  9:16 ` Bernd Zeimetz
2007-12-10  9:18 ` David Miller
2007-12-11 14:19 ` David Miller
2007-12-12 16:05 ` Bernd Zeimetz
2007-12-12 16:23 ` David Miller
2007-12-13 20:54 ` Bernd Zeimetz
2007-12-16 22:41 ` Bernd Zeimetz
2007-12-16 22:48 ` Josip Rodin
2007-12-16 23:30 ` David Miller
2007-12-17  9:40 ` Josip Rodin
2007-12-17  9:57 ` David Miller
2007-12-17 10:10 ` Josip Rodin
2007-12-17 10:21 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071106.211356.60087540.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=arnd@arndb.de \
    --cc=bernd@bzed.de \
    --cc=fabbione@ubuntu.com \
    --cc=joy@entuzijast.net \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.