Warning: empty root dentry name after lazy mount removal

Hi,

Since commit 8ed936b "vfs: Lazily remove mounts on unlinked files and directories.", I’m seeing a new warning from prepend_path() at fs/dcache.c:2937 saying:

Root dentry has weird name <>

Our particular occurrence happens because we are using an older version of iproute (< v3.10, commit bcb9d40, but this also works with the master and just reverting that commit), and the following scenario:

	term1# ip netns add foo
	term1# ip netns exec foo sh
	term1# ls -l /proc/self/fd/4
	lr-x------ 1 root root 64 May 15 11:02 /proc/self/fd/4 -> /var/run/netns/foo
	term2# ip netns del foo
	term1# ls -l /proc/self/fd/4
	[dmesg] WARNING: [...]
	[dmesg] Root dentry has weird name <>
	lr-x------ 1 root root 64 May 15 11:04 /proc/self/fd/4 -> /

Relevant moutinfo chunks before the ip netns del:

	29 0 8:2 / / rw,relatime - ext4 /dev/sda2 rw,data=ordered
	82 29 0:3 / /var/run/netns/foo rw,nosuid,nodev,noexec,relatime - proc proc rw

What I could observe is that after the namespace deletion, that mount 82 get isolated (mnt->parent = mnt), and so prepend_path considers it as the global root. But the name of that dentry has d_name.len == 0, which triggers the WARN in that function.

I’m still struggling to understand all the relations between the VFS structures and at which point they get initialized precisely. Do you have any idea how to fix this problem?

Quoted 5 lines · Ivan Delalande

Hi,

I’m still struggling to understand all the relations between the VFS structures and at which point they get initialized precisely. Do you have any idea how to fix this problem?

I'm attaching a minimal script that reproduces this on 4.1-rc4. I'm taking a look, but Eric or Al will probably figure this out before I get the chance :)

This works for me, can you confirm it works for you folks as well?

Thank you,
Eric

From: "Eric W. Biederman" <[email protected]>
Date: Wed, 20 May 2015 16:39:00 -0500
Subject: [PATCH] vfs: Fix, simpliy and stop using d_dname for the /proc/*/ns/* files.

Now that these files are no longer in proc we can stop playing games with d_alloc_pseudo and d_dname. This change causes a couple of user visible changes:

- Opening /proc/*/ns/* and then readlink on /proc/self/fd/N now sees a prepended / in the filename.

- /proc/mountinfo now gives useful information on what is mounted.

- Bind mounting /proc/*/ns/*, opening the mounted file, removing the bind mount, and readlink on /proc/self/fd/N now sees / (as it should)

Quoted 3 lines · Omar Sandoval

I'm attaching a minimal script that reproduces this on 4.1-rc4. I'm taking a look, but Eric or Al will probably figure this out before I get the chance :)

This works for me, can you confirm it works for you folks as well?

From: "Eric W. Biederman" <[email protected]>
Date: Wed, 20 May 2015 16:39:00 -0500
Subject: [PATCH] vfs: Fix, simpliy and stop using d_dname for the /proc/*/ns/* files.

Now that these files are no longer in proc we can stop playing games with d_alloc_pseudo and d_dname. This change causes a couple of user visible changes:

- Opening /proc/*/ns/* and then readlink on /proc/self/fd/N now sees a prepended / in the filename.

- /proc/mountinfo now gives useful information on what is mounted.

- Bind mounting /proc/*/ns/*, opening the mounted file, removing the bind mount, and readlink on /proc/self/fd/N now sees / (as it should) instead of triggering a warning and a kernel stack backtrace from prepend_path.

Cc [email protected]

Reported-by Ivan Delalande <[email protected]>

Reported-by Omar Sandoval <[email protected]>

Signed-off-by "Eric W. Biederman" <[email protected]>

fs/nsfs.c slow: · line 87

Quoted 9 lines · Eric W. Biederman

fs/nsfs.c +2 -2

-	dentry = d_alloc_pseudo(mnt->mnt_sb, &qname);
+	dentry = d_alloc_name(mnt->mnt_root, name);
 	if (!dentry) {
 		iput(inode);
 		mntput(mnt);
 		return ERR_PTR(-ENOMEM);
 	}
-	d_instantiate(dentry, inode);
+	d_add(dentry, inode);

Careful - that might have non-trivial effects. Namely, you are making the root dentry of that sucker a contention point and adding to hash pollution... It's probably not going to cause visible problems, but it's worth profiling just to be sure.

Besides, you are violating a bunch of rules here - several hashed children of the same directory with the same name all at once... not nice.

Quoted 16 lines · Al Viro

fs/nsfs.c slow: · line 87

Quoted 9 lines · Eric W. Biederman

fs/nsfs.c +2 -2

-	dentry = d_alloc_pseudo(mnt->mnt_sb, &qname);
+	dentry = d_alloc_name(mnt->mnt_root, name);
 	if (!dentry) {
 		iput(inode);
 		mntput(mnt);
 		return ERR_PTR(-ENOMEM);
 	}
-	d_instantiate(dentry, inode);
+	d_add(dentry, inode);

Careful - that might have non-trivial effects. Namely, you are making the root dentry of that sucker a contention point and adding to hash pollution... It's probably not going to cause visible problems, but it's worth profiling just to be sure.

Quoted 3 lines

Besides, you are violating a bunch of rules here - several hashed children of the same directory with the same name all at once... not nice.

Hey Eric, did you have any thought about Al’s concerns?

Quoted 17 lines · Ivan Delalande

Quoted 16 lines · Al Viro

fs/nsfs.c slow: · line 87

Quoted 9 lines · Eric W. Biederman

fs/nsfs.c +2 -2

-	dentry = d_alloc_pseudo(mnt->mnt_sb, &qname);
+	dentry = d_alloc_name(mnt->mnt_root, name);
 	if (!dentry) {
 		iput(inode);
 		mntput(mnt);
 		return ERR_PTR(-ENOMEM);
 	}
-	d_instantiate(dentry, inode);
+	d_add(dentry, inode);

Careful - that might have non-trivial effects. Namely, you are making the root dentry of that sucker a contention point and adding to hash pollution... It's probably not going to cause visible problems, but it's worth profiling just to be sure.

Quoted 5 lines

Quoted 3 lines

Besides, you are violating a bunch of rules here - several hashed children of the same directory with the same name all at once... not nice.

Hey Eric, did you have any thought about Al’s concerns?

Massive difference in perspective for the most part. It did cause me to step back and really look at what that code is doing and why.

For the immediate problem the issue is that the WARN_ON is warning about something nothing in the kernel has done for 5 years and in practice as you have seen is actually wrong. So deleting the warning message appears the best way to handle the situation you are seeing.