LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dave Hansen <haveblue@us.ibm.com>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Al Viro <viro@parcelfarce.linux.theplanet.co.uk>
Cc: Matthew Wilcox <matthew@wil.cx>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
	Greg KH <greg@kroah.com>, Kay Sievers <kay.sievers@vrfy.org>,
	Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Miklos Szeredi <miklos@szeredi.hu>
Subject: fix for boot-time mnt_want_write() bug
Date: Thu, 13 Mar 2008 13:41:55 -0700	[thread overview]
Message-ID: <1205440916.4971.58.camel@nimitz.home.sr71.net> (raw)
In-Reply-To: <200803130021.m2D0LlFm005300@www262.sakura.ne.jp>

Al, Andrew, please pull this patch into your trees.

First of all, this is a hard bug to trigger.  I think it requires page
alloc (or slab) debugging.  It also requires that a vfsmnt has been
freed and its memory not been mapped as something else.  It must also
have had a recent mnt_writer at the time of its __mntput().  The area
where the vfsmnt was must fault when accessed.

The problem occurs when we unmount and __mntput() a vfsmount.  We go
find any cpu_writers for that mount and clear the cpu_writer->count to
zero.  That is supposed to mean that no one will ever go try and
coalesce the cpu_writer->count int to the mnt->__mnt_writers.  Buuuuuut,
that isn't quite what happens.  We only check in __clear_mnt_count() for
a NULL mount:

void __clear_mnt_count(mnt, cpu_writer)
{
 	if (!cpu_writer->mnt)
 		return;
 	atomic_add(cpu_writer->count, &cpu_writer->mnt->__mnt_writers);
 	cpu_writer->count = 0;
}

and we go ahead and dereference the mnt (which may be invalid here).  If
it *WAS* invalid, the cpu_writer->count is always 0, and we don't
actually do anything in practice to the invalid memory location except
access it.  Adding a 0 doesn't _hurt_ anything, even if there is
something else in the memory.  That's why we didn't notice this before.
Miklos, you were very right to get nervous about this area in your
review.

Either one of the hunks in the patch would have fixed Tetsuo's oops.
But, let's include both for completeness.  They're both operating on hot
cachelines at the time so it shouldn't really impact anything.

---

 linux-2.6.git-dave/fs/namespace.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff -puN fs/namespace.c~robind-oops-fix-bootup-1 fs/namespace.c
--- linux-2.6.git/fs/namespace.c~robind-oops-fix-bootup-1	2008-03-13 13:26:08.000000000 -0700
+++ linux-2.6.git-dave/fs/namespace.c	2008-03-13 13:26:08.000000000 -0700
@@ -186,6 +186,12 @@ static inline void __clear_mnt_count(str
 {
 	if (!cpu_writer->mnt)
 		return;
+	/*
+	 * This is in case anyone ever leaves an invalid,
+	 * old ->mnt and a count of 0.
+	 */
+	if (!cpu_writer->count)
+		return;
 	atomic_add(cpu_writer->count, &cpu_writer->mnt->__mnt_writers);
 	cpu_writer->count = 0;
 }
@@ -577,6 +583,12 @@ static inline void __mntput(struct vfsmo
 		spin_lock(&cpu_writer->lock);
 		atomic_add(cpu_writer->count, &mnt->__mnt_writers);
 		cpu_writer->count = 0;
+		/*
+		 * Might as well do this so that no one
+		 * ever sees the pointer and expects
+		 * it to be valid.
+		 */
+		cpu_writer->mnt = NULL;
 		spin_unlock(&cpu_writer->lock);
 	}
 	/*
_


-- Dave


  reply	other threads:[~2008-03-13 20:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-12  1:37 [2.6.25-rc5-mm1] BUG() at mnt_want_write() penguin-kernel
2008-03-12  3:03 ` Dave Hansen
2008-03-12  3:22   ` Andrew Morton
2008-03-12  9:25     ` Peter Zijlstra
2008-03-12 16:52       ` Matthew Wilcox
2008-03-13  0:21         ` Tetsuo Handa
2008-03-13 20:41           ` Dave Hansen [this message]
2008-03-14  0:15             ` fix for boot-time mnt_want_write() bug Tetsuo Handa
2008-03-12  5:46   ` [2.6.25-rc5-mm1] BUG() at mnt_want_write() Tetsuo Handa
2008-03-12  6:35   ` Tetsuo Handa
2008-03-12  3:43 ` Dave Hansen

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=1205440916.4971.58.camel@nimitz.home.sr71.net \
    --to=haveblue@us.ibm.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=greg@kroah.com \
    --cc=hch@lst.de \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=miklos@szeredi.hu \
    --cc=mingo@elte.hu \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=viro@parcelfarce.linux.theplanet.co.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;
as well as URLs for NNTP newsgroup(s).