Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1345378

Re: fs: NULL deref in atime_needs_update

From Al Viro <viro@ZenIV.linux.org.uk>
Newsgroups linux.kernel
Subject Re: fs: NULL deref in atime_needs_update
Date 2016-02-28 21:10 +0100
Message-ID <r7fPc-7Pr-21@gated-at.bofh.it> (permalink)
References (5 earlier) <r6yKe-1lH-19@gated-at.bofh.it> <r6Vx8-18U-5@gated-at.bofh.it> <r7bLz-4E3-3@gated-at.bofh.it> <r7c4X-52Z-13@gated-at.bofh.it> <r7d0Z-5MI-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

On Sun, Feb 28, 2016 at 05:01:34PM +0000, Al Viro wrote:

> Erm...  What's to order ->d_inode and ->d_flags fetches there?  David?
> Looks like the barrier in d_is_negative() is on the wrong side of fetch.
> Confused...

OK, as per David's suggestion, let's flip them around, bringing the
barrier in d_is_negative() between them.  Dmitry, could you try this on
top of mainline?  Again, it's until the first warning.

diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index c6d7d3d..86f81e3 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -323,6 +323,7 @@ static struct dentry *autofs4_mountpoint_changed(struct path *path)
 		struct dentry *new = d_lookup(parent, &dentry->d_name);
 		if (!new)
 			return NULL;
+		WARN_ON(d_is_negative(new));
 		ino = autofs4_dentry_ino(new);
 		ino->last_used = jiffies;
 		dput(path->dentry);
diff --git a/fs/namei.c b/fs/namei.c
index 9c590e0..630d222 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1209,6 +1209,7 @@ static int follow_managed(struct path *path, struct nameidata *nd)
 		/* Handle an automount point */
 		if (managed & DCACHE_NEED_AUTOMOUNT) {
 			ret = follow_automount(path, nd, &need_mntput);
+			WARN_ON(d_is_negative(path->dentry));
 			if (ret < 0)
 				break;
 			continue;
@@ -1260,6 +1261,7 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
 {
 	for (;;) {
 		struct mount *mounted;
+		void *p;
 		/*
 		 * Don't forget we might have a non-mountpoint managed dentry
 		 * that wants to block transit.
@@ -1289,7 +1291,9 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
 		 * dentry sequence number here after this d_inode read,
 		 * because a mount-point is always pinned.
 		 */
-		*inode = path->dentry->d_inode;
+		p = *inode = path->dentry->d_inode;
+		if (unlikely(!p))
+			WARN_ON(!read_seqretry(&mount_lock, nd->m_seq));
 	}
 	return !read_seqretry(&mount_lock, nd->m_seq) &&
 		!(path->dentry->d_flags & DCACHE_NEED_AUTOMOUNT);
@@ -1550,8 +1554,8 @@ static int lookup_fast(struct nameidata *nd,
 		 * This sequence count validates that the inode matches
 		 * the dentry name information from lookup.
 		 */
-		*inode = d_backing_inode(dentry);
 		negative = d_is_negative(dentry);
+		*inode = d_backing_inode(dentry);
 		if (read_seqcount_retry(&dentry->d_seq, seq))
 			return -ECHILD;
 
@@ -1580,6 +1584,7 @@ static int lookup_fast(struct nameidata *nd,
 		 */
 		if (negative)
 			return -ENOENT;
+		WARN_ON(!*inode);		// ->d_seq was fucked somehow
 		path->mnt = mnt;
 		path->dentry = dentry;
 		if (likely(__follow_mount_rcu(nd, path, inode, seqp)))
@@ -1613,8 +1618,10 @@ unlazy:
 	path->mnt = mnt;
 	path->dentry = dentry;
 	err = follow_managed(path, nd);
-	if (likely(!err))
+	if (likely(!err)) {
 		*inode = d_backing_inode(path->dentry);
+		WARN_ON(!*inode);
+	}
 	return err;
 
 need_lookup:
@@ -1717,6 +1724,7 @@ static inline int should_follow_link(struct nameidata *nd, struct path *link,
 		if (read_seqcount_retry(&link->dentry->d_seq, seq))
 			return -ECHILD;
 	}
+	WARN_ON(!inode);		// now, _that_ should not happen.
 	return pick_link(nd, link, inode, seq);
 }
 
@@ -3111,8 +3119,10 @@ static int do_last(struct nameidata *nd,
 			nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
 		/* we _can_ be in RCU mode here */
 		error = lookup_fast(nd, &path, &inode, &seq);
-		if (likely(!error))
+		if (likely(!error)) {
+			WARN_ON(!inode);
 			goto finish_lookup;
+		}
 
 		if (error < 0)
 			return error;
@@ -3203,6 +3213,7 @@ retry_lookup:
 		return -ENOENT;
 	}
 	inode = d_backing_inode(path.dentry);
+	WARN_ON(!inode);
 finish_lookup:
 	if (nd->depth)
 		put_link(nd);
diff --git a/fs/namespace.c b/fs/namespace.c
index 4fb1691..4128a5c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1060,6 +1060,8 @@ static void cleanup_mnt(struct mount *mnt)
 	 * so mnt_get_writers() below is safe.
 	 */
 	WARN_ON(mnt_get_writers(mnt));
+	WARN_ON(!mnt->mnt.mnt_root->d_inode);	// some joker has managed to
+						// make mnt_root negative on us
 	if (unlikely(mnt->mnt_pins.first))
 		mnt_pin_kill(mnt);
 	fsnotify_vfsmount_delete(&mnt->mnt);

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: fs: NULL deref in atime_needs_update Ian Kent <raven@themaw.net> - 2016-02-24 04:20 +0100
  Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-24 05:50 +0100
    Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-24 11:10 +0100
      Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-24 11:20 +0100
        Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-24 14:40 +0100
          Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-24 16:20 +0100
            Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-25 09:30 +0100
              Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-25 17:40 +0100
                Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-26 22:30 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-26 23:10 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-27 23:30 +0100
                Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-28 17:10 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-28 18:10 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-28 21:10 +0100
                Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-29 10:40 +0100
                Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-29 13:40 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-29 17:20 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-29 14:10 +0100
                Re: fs: NULL deref in atime_needs_update David Howells <dhowells@redhat.com> - 2016-02-29 14:50 +0100
                Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-29 17:00 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-29 17:30 +0100
                Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-02-29 19:30 +0100
                Re: fs: NULL deref in atime_needs_update Dmitry Vyukov <dvyukov@google.com> - 2016-03-01 10:00 +0100
                Re: fs: NULL deref in atime_needs_update Linus Torvalds <torvalds@linux-foundation.org> - 2016-02-29 17:50 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-29 18:00 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-29 18:30 +0100
                Re: fs: NULL deref in atime_needs_update Linus Torvalds <torvalds@linux-foundation.org> - 2016-02-29 18:30 +0100
                Re: fs: NULL deref in atime_needs_update Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-26 22:30 +0100

csiph-web