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@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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)
{
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
