lkml.org 
[lkml]   [2014]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    Subject[BUG] signal: sighand unprotected when accessed by /proc
    We were able to trigger this bug in -rt, and by review, I'm thinking
    that this could very well be a mainline bug too. I had our QA team add
    a trace patch to the kernel to prove my analysis, and it did.

    Here's the patch:

    http://rostedt.homelinux.com/private/sighand-trace.patch

    Let me try to explain the bug:


    CPU0 CPU1
    ---- ----
    [ read of /proc/<pid>/stat ]
    get_task_struct();
    [...]
    [ <pid> exits ]
    [ parent does wait on <pid> ]
    wait_task_zombie()
    release_task()
    proc_flush_task()
    /* the above removes new access
    to the /proc system */
    __exit_signal()
    __cleanup_sighand(sighand);
    atomic_dec_and_test(sighand->count);
    do_task_stat()
    lock_task_sighand(task);
    sighand = rcu_dereference(tsk->sighand);

    kmem_cache_free(sighand);

    if (sighand != NULL)
    spin_lock(sighand->siglock);

    ** BOOM! use after free **


    Seems there is no protection between reading the sighand from proc and
    freeing it. The sighand->count is not updated, and the sighand is not
    freed via rcu. Wouldn't matter, because the use of sighand is not
    totally protected by rcu.

    We take a lot of care to free tsk->signal but we are a bit sloppy in
    protecting sighand.

    Here's the trace from our last crash:

    ps-23716 1....... 11201242344176: __lock_task_sighand: lock sighand=ffff8801022aee80 from sh:23718
    make-23672 1.....11 11201242409218: release_task: freeing sighand ffff8801022aee80 for sh:23718
    make-23672 1.....11 11201242409980: __cleanup_sighand: free sighand ffff8801022aee80
    make-23672 1.....11 11201242410380: __cleanup_sighand: sighand ffff8801022aee80 freed
    ps-23716 1d...211 11201243007052: __list_del_entry: BUG HIT!

    Let me explain the differences between -rt and mainline here.

    One, the spinlock in -rt is an rtmutex. The list_del_entry() bug is the
    task trying to remove itself from sighand->lock->wait_list. As the lock
    has been freed, the list head of the rtmutex is corrupted.

    Another difference here is that all this ran on CPU 1. In mainline,
    that could not happen because the spinlock prevents preemption. But
    still, I don't see anything that could stop this from happening on
    multiple CPUs.

    Again, the bug does exist in mainline. I believe the only reason that
    we do not trigger it (often) is that there's a lot that the reading of
    the proc needs to do between opening the proc files and then reading
    the sighand->lock. Much more than the clean up path between
    removing /proc and freeing sighand. But with an interrupt going off at
    the right time slowing down the fast path, we can still hit this race.

    This is the beauty of the -rt patch. It can simulate hard to hit races
    nicely, due to the preemptive nature of the spinlocks.

    I do not see any trivial fix for this. I figured I show this to the
    signal gurus before trying to redesign the way sighand gets freed or
    used by /proc.

    -- Steve




    \
     
     \ /
      Last update: 2014-06-03 23:41    [W:4.121 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site