lkml.org 
[lkml]   [2016]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: fs: uninterruptible hang in handle_userfault
    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)
    {
    \
     
     \ /
      Last update: 2016-03-02 19:21    [W:2.738 / U:1.860 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site