1. 06 Nov, 2009 1 commit
    • Earl Chew's avatar
      fs: pipe.c null pointer dereference · 070609e1
      Earl Chew authored
      This patch fixes a null pointer exception in pipe_rdwr_open() which
      generates the stack trace:
      
      > Unable to handle kernel NULL pointer dereference at 0000000000000028 RIP:
      >  [<ffffffff802899a5>] pipe_rdwr_open+0x35/0x70
      >  [<ffffffff8028125c>] __dentry_open+0x13c/0x230
      >  [<ffffffff8028143d>] do_filp_open+0x2d/0x40
      >  [<ffffffff802814aa>] do_sys_open+0x5a/0x100
      >  [<ffffffff8021faf3>] sysenter_do_call+0x1b/0x67
      
      The failure mode is triggered by an attempt to open an anonymous
      pipe via /proc/pid/fd/* as exemplified by this script:
      
      =============================================================
      while : ; do
         { echo y ; sleep 1 ; } | { while read ; do echo z$REPLY; done ; } &
         PID=$!
         OUT=$(ps -efl | grep 'sleep 1' | grep -v grep |
              { read PID REST ; echo $PID; } )
         OUT="${OUT%% *}"
         DELAY=$((RANDOM * 1000 / 32768))
         usleep $((DELAY * 1000 + RANDOM % 1000 ))
         echo n > /proc/$OUT/fd/1                 # Trigger defect
      done
      =============================================================
      
      Note that the failure window is quite small and I could only
      reliably reproduce the defect by inserting a small delay
      in pipe_rdwr_open(). For example:
      
       static int
       pipe_rdwr_open(struct inode *inode, struct file *filp)
       {
             msleep(100);
             mutex_lock(&inode->i_mutex);
      
      Although the defect was observed in pipe_rdwr_open(), I think it
      makes sense to replicate the change through all the pipe_*_open()
      functions.
      
      The core of the change is to verify that inode->i_pipe has not
      been released before attempting to manipulate it. If inode->i_pipe
      is no longer present, return ENOENT to indicate so.
      
      The comment about potentially using atomic_t for i_pipe->readers
      and i_pipe->writers has also been removed because it is no longer
      relevant in this context. The inode->i_mutex lock must be used so
      that inode->i_pipe can be dealt with correctly.
      Signed-off-by: default avatarEarl Chew <earl_chew@agilent.com>
      Cc: stable@kernel.org
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      070609e1
  2. 29 Oct, 2009 17 commits
  3. 28 Oct, 2009 11 commits
    • Darren Hart's avatar
      futex: Correct queue_me and unqueue_me commentary · cd18252d
      Darren Hart authored
      The queue_me/unqueue_me commentary is oddly placed and out of date.
      Clean it up and correct the inaccurate bits.
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Acked-by: default avatarPeter Zijlstra <peterz@infradead.org>
      Cc: Steven Rostedt <rostedt@goodmis.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      Cc: John Stultz <johnstul@us.ibm.com>
      LKML-Reference: <20090922053015.8717.71713.stgit@Aeon>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      cd18252d
    • Darren Hart's avatar
      futex: Move drop_futex_key_refs out of spinlock'ed region · b9e40b50
      Darren Hart authored
      When requeuing tasks from one futex to another, the reference held
      by the requeued task to the original futex location needs to be
      dropped eventually.
      
      Dropping the reference may ultimately lead to a call to
      "iput_final" and subsequently call into filesystem- specific code -
      which may be non-atomic.
      
      It is therefore safer to defer this drop operation until after the
      futex_hash_bucket spinlock has been dropped.
      
      Originally-From: Helge Bahmann <hcb@chaoticmind.net>
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Cc: <stable@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      Cc: John Stultz <johnstul@linux.vnet.ibm.com>
      Cc: Sven-Thorsten Dietrich <sdietrich@novell.com>
      Cc: John Kacur <jkacur@redhat.com>
      LKML-Reference: <4AD7A298.5040802@us.ibm.com>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      b9e40b50
    • Darren Hart's avatar
      futex: Add memory barrier commentary to futex_wait_queue_me() · d6617954
      Darren Hart authored
      The memory barrier semantics of futex_wait_queue_me() are
      non-obvious. Add some commentary to try and clarify it.
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      Cc: John Stultz <johnstul@us.ibm.com>
      LKML-Reference: <20090924185447.694.38948.stgit@Aeon>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      d6617954
    • Darren Hart's avatar
      futex: Correct futex_wait_requeue_pi() commentary · eb78fc39
      Darren Hart authored
      The state machine described in the comments wasn't updated with
      a follow-on fix.  Address that and cleanup the corresponding
      commentary in the function.
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Steven Rostedt <rostedt@goodmis.org>
      Cc: Thomas Gleixner <tglx@linutronix.de>
      LKML-Reference: <4A737C2A.9090001@us.ibm.com>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      eb78fc39
    • Thomas Gleixner's avatar
      futex: Fix locking imbalance · 11bc48db
      Thomas Gleixner authored
      Rich reported a lock imbalance in the futex code:
      
         http://bugzilla.kernel.org/show_bug.cgi?id=14288
      
      It's caused by the displacement of the retry_private label in
      futex_wake_op(). The code unlocks the hash bucket locks in the
      error handling path and retries without locking them again which
      makes the next unlock fail.
      
      Move retry_private so we lock the hash bucket locks when we retry.
      Reported-by: default avatarRich Ercolany <rercola@acm.jhu.edu>
      Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
      Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
      Cc: Darren Hart <dvhltc@us.ibm.com>
      Cc: stable-2.6.31 <stable@kernel.org>
      LKML-Reference: <new-submission>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      11bc48db
    • Darren Hart's avatar
      futex: Correct futex_wait_requeue_pi() commentary · 9231abe1
      Darren Hart authored
      Correct various typos and formatting inconsistencies in the
      commentary of futex_wait_requeue_pi().
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Acked-by: default avatarPeter Zijlstra <peterz@infradead.org>
      Cc: Steven Rostedt <rostedt@goodmis.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      Cc: John Stultz <johnstul@us.ibm.com>
      LKML-Reference: <20090922052958.8717.21932.stgit@Aeon>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      9231abe1
    • Darren Hart's avatar
      futex: Make function kernel-doc commentary consistent · 6ca0f2a0
      Darren Hart authored
      Make the existing function kernel-doc consistent throughout
      futex.c, following Documentation/kernel-doc-nano-howto.txt as
      closely as possible.
      
      When unsure, at least be consistent within futex.c.
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Acked-by: default avatarPeter Zijlstra <peterz@infradead.org>
      Cc: Steven Rostedt <rostedt@goodmis.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      Cc: John Stultz <johnstul@us.ibm.com>
      LKML-Reference: <20090922053022.8717.13339.stgit@Aeon>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      6ca0f2a0
    • Darren Hart's avatar
      futex: Correct futex_q woken state commentary · 0699fd94
      Darren Hart authored
      Use kernel-doc format to describe struct futex_q.
      
      Correct the wakeup definition to eliminate the statement about
      waking the waiter between the plist_del() and the q->lock_ptr = 0.
      
      Note in the comment that PI futexes have a different definition of
      the woken state.
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Acked-by: default avatarPeter Zijlstra <peterz@infradead.org>
      Cc: Steven Rostedt <rostedt@goodmis.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      Cc: John Stultz <johnstul@us.ibm.com>
      LKML-Reference: <20090922053029.8717.62798.stgit@Aeon>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      0699fd94
    • Darren Hart's avatar
      futex: Check for NULL keys in match_futex · 29b33bb7
      Darren Hart authored
      If userspace tries to perform a requeue_pi on a non-requeue_pi waiter,
      it will find the futex_q->requeue_pi_key to be NULL and OOPS.
      
      Check for NULL in match_futex() instead of doing explicit NULL pointer
      checks on all call sites.  While match_futex(NULL, NULL) returning
      false is a little odd, it's still correct as we expect valid key
      references.
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Ingo Molnar <mingo@elte.hu>
      CC: Eric Dumazet <eric.dumazet@gmail.com>
      CC: Dinakar Guniguntala <dino@in.ibm.com>
      CC: John Stultz <johnstul@us.ibm.com>
      Cc: stable@kernel.org
      LKML-Reference: <4AD60687.10306@us.ibm.com>
      Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
      29b33bb7
    • Thomas Gleixner's avatar
      futex: Fix spurious wakeup for requeue_pi really · 43746940
      Thomas Gleixner authored
      The requeue_pi path doesn't use unqueue_me() (and the racy lock_ptr ==
      NULL test) nor does it use the wake_list of futex_wake() which where
      the reason for commit 41890f24 (futex: Handle spurious wake up)
      
      See debugging discussing on LKML Message-ID: <4AD4080C.20703@us.ibm.com>
      
      The changes in this fix to the wait_requeue_pi path were considered to
      be a likely unecessary, but harmless safety net. But it turns out that
      due to the fact that for unknown $@#!*( reasons EWOULDBLOCK is defined
      as EAGAIN we built an endless loop in the code path which returns
      correctly EWOULDBLOCK.
      
      Spurious wakeups in wait_requeue_pi code path are unlikely so we do
      the easy solution and return EWOULDBLOCK^WEAGAIN to user space and let
      it deal with the spurious wakeup.
      
      Cc: Darren Hart <dvhltc@us.ibm.com>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: John Stultz <johnstul@linux.vnet.ibm.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      LKML-Reference: <4AE23C74.1090502@us.ibm.com>
      Cc: stable@kernel.org
      Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
      43746940
    • Thomas Gleixner's avatar
      futex: Detect mismatched requeue targets · e814515d
      Thomas Gleixner authored
      There is currently no check to ensure that userspace uses the same
      futex requeue target (uaddr2) in futex_requeue() that the waiter used
      in futex_wait_requeue_pi().  A mismatch here could very unexpected
      results as the waiter assumes it either wakes on uaddr1 or uaddr2. We
      could detect this on wakeup in the waiter, but the cleanup is more
      intense after the improper requeue has occured.
      
      This patch stores the waiter's expected requeue target in a new
      requeue_pi_key pointer in the futex_q which futex_requeue() checks
      prior to attempting to do a proxy lock acquistion or a requeue when
      requeue_pi=1. If they don't match, return -EINVAL from futex_requeue,
      aborting the requeue of any remaining waiters.
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: John Kacur <jkacur@redhat.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      Cc: John Stultz <johnstul@us.ibm.com>
      LKML-Reference: <20090814003650.14634.63916.stgit@Aeon>
      Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
      
      Conflicts:
      
      	kernel/futex.c
      Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
      e814515d
  4. 20 Oct, 2009 2 commits
  5. 13 Oct, 2009 6 commits
  6. 12 Oct, 2009 1 commit
    • Darren Hart's avatar
      futex: Fix wakeup race by setting TASK_INTERRUPTIBLE before queue_me() · a03d1035
      Darren Hart authored
      PI futexes do not use the same plist_node_empty() test for wakeup.
      It was possible for the waiter (in futex_wait_requeue_pi()) to set
      TASK_INTERRUPTIBLE after the waker assigned the rtmutex to the
      waiter. The waiter would then note the plist was not empty and call
      schedule(). The task would not be found by any subsequeuent futex
      wakeups, resulting in a userspace hang.
      
      By moving the setting of TASK_INTERRUPTIBLE to before the call to
      queue_me(), the race with the waker is eliminated. Since we no
      longer call get_user() from within queue_me(), there is no need to
      delay the setting of TASK_INTERRUPTIBLE until after the call to
      queue_me().
      
      The FUTEX_LOCK_PI operation is not affected as futex_lock_pi()
      relies entirely on the rtmutex code to handle schedule() and
      wakeup.  The requeue PI code is affected because the waiter starts
      as a non-PI waiter and is woken on a PI futex.
      
      Remove the crusty old comment about holding spinlocks() across
      get_user() as we no longer do that. Correct the locking statement
      with a description of why the test is performed.
      Signed-off-by: default avatarDarren Hart <dvhltc@us.ibm.com>
      Acked-by: default avatarPeter Zijlstra <peterz@infradead.org>
      Cc: Steven Rostedt <rostedt@goodmis.org>
      Cc: Eric Dumazet <eric.dumazet@gmail.com>
      Cc: Dinakar Guniguntala <dino@in.ibm.com>
      Cc: John Stultz <johnstul@us.ibm.com>
      LKML-Reference: <20090922053038.8717.97838.stgit@Aeon>
      Signed-off-by: default avatarIngo Molnar <mingo@elte.hu>
      a03d1035
  7. 09 Oct, 2009 1 commit
  8. 08 Oct, 2009 1 commit