Re: fs: uninterruptible hang in handle_userfault

[Andrea Arcangeli <aarcange@redhat.com>]

On Wed, Mar 02, 2016 at 09:03:01AM -0800, Linus Torvalds wrote:
> It's not just "exit_futex()" (what is that? I assume you mean

That come from a cleanup (appended below but not very well tested) I
did initially to consolidate the futex exit code before attempting to
relocate its call location.

> exit_robust_list()) that triggers the problem, it's also the
> 
>         put_user(0, tsk->clear_child_tid);
> 
> in mm_release().

>From the stack trace it didn't appear to refault there and it was
still stuck in exit_futex, but this could end up in the same problem
and your fix already took care of this one as well.

> So it's not just about futexes.
> 
> The might be other final user space accesses lurking too that I didn't
> even think about.
> 
> Anyway, I committed (a) as the safest version with the least side
> effects. If people think some more about this and come up with
> solutions how to avoid these kinds of "very late user space accesses"
> cleanly, I think that would be great.

Agreed.

Thanks,
Andrea

>From 03f7e43aab4e4b6f02599f4ffffe4675581f691e Mon Sep 17 00:00:00 2001
From: Andrea Arcangeli <aarcange@redhat.com>
Date: Wed, 2 Mar 2016 18:25:50 +0100
Subject: [PATCH 1/1] futex: cleanup #ifdef FUTEX and consolidate futex exit
 path

There's no reason to expose futex internal details to the common exit path.

Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 include/linux/futex.h |  8 ++------
 kernel/fork.c         | 16 +---------------
 kernel/futex.c        | 22 ++++++++++++++++++++--
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/linux/futex.h b/include/linux/futex.h
index 6435f46..89da7d6 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -53,18 +53,14 @@ union futex_key {
 #define FUTEX_KEY_INIT (union futex_key) { .both = { .ptr = NULL } }
 
 #ifdef CONFIG_FUTEX
-extern void exit_robust_list(struct task_struct *curr);
-extern void exit_pi_state_list(struct task_struct *curr);
+extern void exit_futex(struct task_struct *tsk);
 #ifdef CONFIG_HAVE_FUTEX_CMPXCHG
 #define futex_cmpxchg_enabled 1
 #else
 extern int futex_cmpxchg_enabled;
 #endif
 #else
-static inline void exit_robust_list(struct task_struct *curr)
-{
-}
-static inline void exit_pi_state_list(struct task_struct *curr)
+static inline void exit_futex(struct task_struct *tsk)
 {
 }
 #endif
diff --git a/kernel/fork.c b/kernel/fork.c
index 2e391c7..81974dd 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -855,21 +855,7 @@ static int wait_for_vfork_done(struct task_struct *child,
  */
 void mm_release(struct task_struct *tsk, struct mm_struct *mm)
 {
-	/* Get rid of any futexes when releasing the mm */
-#ifdef CONFIG_FUTEX
-	if (unlikely(tsk->robust_list)) {
-		exit_robust_list(tsk);
-		tsk->robust_list = NULL;
-	}
-#ifdef CONFIG_COMPAT
-	if (unlikely(tsk->compat_robust_list)) {
-		compat_exit_robust_list(tsk);
-		tsk->compat_robust_list = NULL;
-	}
-#endif
-	if (unlikely(!list_empty(&tsk->pi_state_list)))
-		exit_pi_state_list(tsk);
-#endif
+	exit_futex(tsk);
 
 	uprobe_free_utask(tsk);
 
diff --git a/kernel/futex.c b/kernel/futex.c
index 5d6ce64..eb2ea48 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -65,6 +65,7 @@
 #include <linux/freezer.h>
 #include <linux/bootmem.h>
 #include <linux/fault-inject.h>
+#include <linux/compat.h>
 
 #include <asm/futex.h>
 
@@ -752,7 +753,7 @@ static struct task_struct * futex_find_get_task(pid_t pid)
  * Kernel cleans up PI-state, but userspace is likely hosed.
  * (Robust-futex cleanup is separate and might save the day for userspace.)
  */
-void exit_pi_state_list(struct task_struct *curr)
+static void exit_pi_state_list(struct task_struct *curr)
 {
 	struct list_head *next, *head = &curr->pi_state_list;
 	struct futex_pi_state *pi_state;
@@ -2975,7 +2976,7 @@ static inline int fetch_robust_entry(struct robust_list __user **entry,
  *
  * We silently return on any sign of list-walking problem.
  */
-void exit_robust_list(struct task_struct *curr)
+static void exit_robust_list(struct task_struct *curr)
 {
 	struct robust_list_head __user *head = curr->robust_list;
 	struct robust_list __user *entry, *next_entry, *pending;
@@ -3038,6 +3039,23 @@ void exit_robust_list(struct task_struct *curr)
 				   curr, pip);
 }
 
+void exit_futex(struct task_struct *tsk)
+{
+	/* Get rid of any futexes when releasing the mm */
+	if (unlikely(tsk->robust_list)) {
+		exit_robust_list(tsk);
+		tsk->robust_list = NULL;
+	}
+#ifdef CONFIG_COMPAT
+	if (unlikely(tsk->compat_robust_list)) {
+		compat_exit_robust_list(tsk);
+		tsk->compat_robust_list = NULL;
+	}
+#endif
+	if (unlikely(!list_empty(&tsk->pi_state_list)))
+		exit_pi_state_list(tsk);
+}
+
 long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
 		u32 __user *uaddr2, u32 val2, u32 val3)
 {