releases/2.6.27.46/fix-race-in-tty_fasync-properly.patch

   1 From 80e1e823989ec44d8e35bdfddadbddcffec90424 Mon Sep 17 00:00:00 2001
   2 From: Linus Torvalds <torvalds@linux-foundation.org>
   3 Date: Sun, 7 Feb 2010 10:11:23 -0800
   4 Subject: Fix race in tty_fasync() properly
   5
   6 From: Linus Torvalds <torvalds@linux-foundation.org>
   7
   8 commit 80e1e823989ec44d8e35bdfddadbddcffec90424 upstream.
   9
  10 This reverts commit 703625118069 ("tty: fix race in tty_fasync") and
  11 commit b04da8bfdfbb ("fnctl: f_modown should call write_lock_irqsave/
  12 restore") that tried to fix up some of the fallout but was incomplete.
  13
  14 It turns out that we really cannot hold 'tty->ctrl_lock' over calling
  15 __f_setown, because not only did that cause problems with interrupt
  16 disables (which the second commit fixed), it also causes a potential
  17 ABBA deadlock due to lock ordering.
  18
  19 Thanks to Tetsuo Handa for following up on the issue, and running
  20 lockdep to show the problem.  It goes roughly like this:
  21
  22  - f_getown gets filp->f_owner.lock for reading without interrupts
  23    disabled, so an interrupt that happens while that lock is held can
  24    cause a lockdep chain from f_owner.lock -> sighand->siglock.
  25
  26  - at the same time, the tty->ctrl_lock -> f_owner.lock chain that
  27    commit 703625118069 introduced, together with the pre-existing
  28    sighand->siglock -> tty->ctrl_lock chain means that we have a lock
  29    dependency the other way too.
  30
  31 So instead of extending tty->ctrl_lock over the whole __f_setown() call,
  32 we now just take a reference to the 'pid' structure while holding the
  33 lock, and then release it after having done the __f_setown.  That still
  34 guarantees that 'struct pid' won't go away from under us, which is all
  35 we really ever needed.
  36
  37 Reported-and-tested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
  38 Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
  39 Acked-by: Américo Wang <xiyou.wangcong@gmail.com>
  40 Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  41 Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  42
  43 ---
  44  drivers/char/tty_io.c |    4 +++-
  45  fs/fcntl.c            |    6 ++----
  46  2 files changed, 5 insertions(+), 5 deletions(-)
  47
  48 --- a/drivers/char/tty_io.c
  49 +++ b/drivers/char/tty_io.c
  50 @@ -2437,8 +2437,10 @@ static int tty_fasync(int fd, struct fil
  51                         pid = task_pid(current);
  52                         type = PIDTYPE_PID;
  53                 }
  54 -               retval = __f_setown(filp, pid, type, 0);
  55 +               get_pid(pid);
  56                 spin_unlock_irqrestore(&tty->ctrl_lock, flags);
  57 +               retval = __f_setown(filp, pid, type, 0);
  58 +               put_pid(pid);
  59                 if (retval)
  60                         goto out;
  61         } else {
  62 --- a/fs/fcntl.c
  63 +++ b/fs/fcntl.c
  64 @@ -200,9 +200,7 @@ static int setfl(int fd, struct file * f
  65  static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
  66                       uid_t uid, uid_t euid, int force)
  67  {
  68 -       unsigned long flags;
  69 -
  70 -       write_lock_irqsave(&filp->f_owner.lock, flags);
  71 +       write_lock_irq(&filp->f_owner.lock);
  72         if (force || !filp->f_owner.pid) {
  73                 put_pid(filp->f_owner.pid);
  74                 filp->f_owner.pid = get_pid(pid);
  75 @@ -210,7 +208,7 @@ static void f_modown(struct file *filp,
  76                 filp->f_owner.uid = uid;
  77                 filp->f_owner.euid = euid;
  78         }
  79 -       write_unlock_irqrestore(&filp->f_owner.lock, flags);
  80 +       write_unlock_irq(&filp->f_owner.lock);
  81  }
  82
  83  int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,