From: Linus Torvalds <torvalds@linux-foundation.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: gregkh@suse.de, taviso@google.com, viro@ZenIV.linux.org.uk,
linux-kernel@vger.kernel.org, ebiederm@xmission.com,
alan@lxorguk.ukuu.org.uk, jdike@addtoit.com, jln@google.com,
mpm@selenic.com
Subject: Re: [2.6.33-rc5] tty: possible irq lock inversion dependency in tty_fasync
Date: Sat, 6 Feb 2010 22:46:01 -0800 (PST) [thread overview]
Message-ID: <alpine.LFD.2.00.1002062244310.3829@localhost.localdomain> (raw)
In-Reply-To: <alpine.LFD.2.00.1002062226000.3829@localhost.localdomain>
On Sat, 6 Feb 2010, Linus Torvalds wrote:
>
> Yeah. I think we need to just revert that commit.
>
> Or maybe we could just do the following, rather than revert it outright:
> just get a ref to the 'struct pid' while holding the spinlock, and then
> releasing it after doing the __f_setown() call.
Btw, if we do this, then we should probably revert commit
b04da8bfdfbbd79544cab2fadfdc12e87eb01600 at the same time.
Resulting patch would then look like the appended.
Linus
---
drivers/char/tty_io.c | 4 +++-
fs/fcntl.c | 6 ++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index c6f3b48..dcb9083 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1951,8 +1951,10 @@ static int tty_fasync(int fd, struct file *filp, int on)
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 {
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 5ef953e..97e01dc 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -199,9 +199,7 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
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->f_owner.pid) {
put_pid(filp->f_owner.pid);
filp->f_owner.pid = get_pid(pid);
@@ -213,7 +211,7 @@ static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
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,
next prev parent reply other threads:[~2010-02-07 6:46 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-07 5:52 [2.6.33-rc5] tty: possible irq lock inversion dependency in tty_fasync Tetsuo Handa
2010-02-07 6:31 ` Linus Torvalds
2010-02-07 6:46 ` Linus Torvalds [this message]
2010-02-07 7:27 ` Greg KH
2010-02-07 8:12 ` [2.6.33-rc5] tty: possible irq lock inversion dependency intty_fasync Tetsuo Handa
2010-02-07 6:46 ` [2.6.33-rc5] tty: possible irq lock inversion dependency in tty_fasync Américo Wang
2010-02-07 6:59 ` Linus Torvalds
2010-02-07 16:06 ` Américo Wang
2010-02-07 7:00 ` Eric W. Biederman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LFD.2.00.1002062244310.3829@localhost.localdomain \
--to=torvalds@linux-foundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=ebiederm@xmission.com \
--cc=gregkh@suse.de \
--cc=jdike@addtoit.com \
--cc=jln@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mpm@selenic.com \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=taviso@google.com \
--cc=viro@ZenIV.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox