[lkml]   [2010]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[05/93] Fix race in tty_fasync() properly
    2.6.32-stable review patch.  If anyone has any objections, please let us know.


    From: Linus Torvalds <>

    commit 80e1e823989ec44d8e35bdfddadbddcffec90424 upstream.

    This reverts commit 703625118069 ("tty: fix race in tty_fasync") and
    commit b04da8bfdfbb ("fnctl: f_modown should call write_lock_irqsave/
    restore") that tried to fix up some of the fallout but was incomplete.

    It turns out that we really cannot hold 'tty->ctrl_lock' over calling
    __f_setown, because not only did that cause problems with interrupt
    disables (which the second commit fixed), it also causes a potential
    ABBA deadlock due to lock ordering.

    Thanks to Tetsuo Handa for following up on the issue, and running
    lockdep to show the problem. It goes roughly like this:

    - f_getown gets filp->f_owner.lock for reading without interrupts
    disabled, so an interrupt that happens while that lock is held can
    cause a lockdep chain from f_owner.lock -> sighand->siglock.

    - at the same time, the tty->ctrl_lock -> f_owner.lock chain that
    commit 703625118069 introduced, together with the pre-existing
    sighand->siglock -> tty->ctrl_lock chain means that we have a lock
    dependency the other way too.

    So instead of extending tty->ctrl_lock over the whole __f_setown() call,
    we now just take a reference to the 'pid' structure while holding the
    lock, and then release it after having done the __f_setown. That still
    guarantees that 'struct pid' won't go away from under us, which is all
    we really ever needed.

    Reported-and-tested-by: Tetsuo Handa <>
    Acked-by: Greg Kroah-Hartman <>
    Acked-by: Américo Wang <>
    Signed-off-by: Linus Torvalds <>
    Signed-off-by: Greg Kroah-Hartman <>

    drivers/char/tty_io.c | 4 +++-
    fs/fcntl.c | 6 ++----
    2 files changed, 5 insertions(+), 5 deletions(-)

    --- a/drivers/char/tty_io.c
    +++ b/drivers/char/tty_io.c
    @@ -1930,8 +1930,10 @@ static int tty_fasync(int fd, struct fil
    pid = task_pid(current);
    type = PIDTYPE_PID;
    - retval = __f_setown(filp, pid, type, 0);
    + get_pid(pid);
    spin_unlock_irqrestore(&tty->ctrl_lock, flags);
    + retval = __f_setown(filp, pid, type, 0);
    + put_pid(pid);
    if (retval)
    goto out;
    } else {
    --- a/fs/fcntl.c
    +++ b/fs/fcntl.c
    @@ -199,9 +199,7 @@ static int setfl(int fd, struct file * f
    static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
    int force)
    - unsigned long flags;
    - write_lock_irqsave(&filp->f_owner.lock, flags);
    + write_lock_irq(&filp->f_owner.lock);
    if (force || !filp-> {
    filp-> = get_pid(pid);
    @@ -213,7 +211,7 @@ static void f_modown(struct file *filp,
    filp->f_owner.euid = cred->euid;
    - write_unlock_irqrestore(&filp->f_owner.lock, flags);
    + write_unlock_irq(&filp->f_owner.lock);

    int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2010-02-19 18:03    [W:0.073 / U:13.616 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site