linux-kernel - Re: [lkp] [namei] fda89e6574: kernel BUG at fs/namei.c:679!

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160314031001.GY17997@ZenIV.linux.org.uk>
Date:	Mon, 14 Mar 2016 03:10:01 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	kernel test robot <ying.huang@...ux.intel.com>
Cc:	lkp@...org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [lkp] [namei] fda89e6574: kernel BUG at fs/namei.c:679!

On Mon, Mar 14, 2016 at 08:48:26AM +0800, kernel test robot wrote:
> FYI, we noticed the below changes on
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git work.lookups
> commit fda89e65743179d09e55bc6c265d06fa5efa8803 ("namei: untanlge lookup_fast()")

Unfortunately, while with my normal .config it reliably triggers an oops
in x86_pmu_enable() (with or without those patches), yours triggers nothing
but a pile of OOMs.  How much RAM do you give those suckers?  I'm _not_
testing those on bare hardware, obviously - it's KVM image.

FWIW, see below for hopefully cleaner fix (will fold once I manage to trigger
the damn thing and verify that fix indeed fixes).  It's on top of offending
commit.  Folks, could you please check if it fixes that crap on your setup?

diff --git a/fs/namei.c b/fs/namei.c
index 7a5f79f..d721821 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1519,6 +1519,7 @@ static int lookup_fast(struct nameidata *nd,
 	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent = nd->path.dentry;
 	int err;
+	int status = 1;
 
 	/*
 	 * Rename seqlock is not required here because in the off chance
@@ -1555,54 +1556,45 @@ static int lookup_fast(struct nameidata *nd,
 			return -ECHILD;
 
 		*seqp = seq;
-		if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE)) {
-			int status = d_revalidate(dentry, nd->flags);
-			if (unlikely(status <= 0)) {
-				if (unlazy_walk(nd, dentry, seq))
-					return -ECHILD;
-				if (status == -ECHILD)
-					status = d_revalidate(dentry, nd->flags);
-				if (status <= 0) {
-					if (!status) {
-						d_invalidate(dentry);
-						status = 1;
-					}
-					dput(dentry);
-					return status;
-				}
-			}
+		if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
+			status = d_revalidate(dentry, nd->flags);
+		if (unlikely(status <= 0)) {
+			if (unlazy_walk(nd, dentry, seq))
+				return -ECHILD;
+			if (status == -ECHILD)
+				status = d_revalidate(dentry, nd->flags);
+		} else {
+			/*
+			 * Note: do negative dentry check after revalidation in
+			 * case that drops it.
+			 */
+			if (unlikely(negative))
+				return -ENOENT;
+			path->mnt = mnt;
+			path->dentry = dentry;
+			if (likely(__follow_mount_rcu(nd, path, inode, seqp)))
+				return 0;
+			if (unlazy_walk(nd, dentry, seq))
+				return -ECHILD;
 		}
-		/*
-		 * Note: do negative dentry check after revalidation in
-		 * case that drops it.
-		 */
-		if (unlikely(negative))
-			return -ENOENT;
-		path->mnt = mnt;
-		path->dentry = dentry;
-		if (likely(__follow_mount_rcu(nd, path, inode, seqp)))
-			return 0;
-		if (unlazy_walk(nd, dentry, seq))
-			return -ECHILD;
 	} else {
 		dentry = __d_lookup(parent, &nd->last);
 		if (unlikely(!dentry))
 			return 1;
-		if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE)) {
-			int status = d_revalidate(dentry, nd->flags);
-			if (unlikely(status <= 0)) {
-				if (!status) {
-					d_invalidate(dentry);
-					status = 1;
-				}
-				dput(dentry);
-				return status;
-			}
-		}
-		if (unlikely(d_is_negative(dentry))) {
-			dput(dentry);
-			return -ENOENT;
+		if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
+			status = d_revalidate(dentry, nd->flags);
+	}
+	if (unlikely(status <= 0)) {
+		if (!status) {
+			d_invalidate(dentry);
+			status = 1;
 		}
+		dput(dentry);
+		return status;
+	}
+	if (unlikely(d_is_negative(dentry))) {
+		dput(dentry);
+		return -ENOENT;
 	}
 
 	path->mnt = mnt;