Bug 94269 - [nullfs] procfs shows wrong data if executable is running from nullfs
Summary: [nullfs] procfs shows wrong data if executable is running from nullfs
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 6.1-PRERELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Konstantin Belousov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-09 13:00 UTC by Anatoli Klassen
Modified: 2009-05-31 16:04 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Anatoli Klassen 2006-03-09 13:00:12 UTC
I have a nullfs-mounted dir with some executables and a mounted procfs (under
/proc).  Now if one of the executables tries to read /proc/curproc/file it
got an error or sometimes wrong path.

How-To-Repeat: # mount -t procfs procfs /proc
# mount -t nullfs /bin /mnt
# /bin/ls -l /proc/curproc/file
lr--r--r--  1 root  wheel  0 Mar  9 13:53 /proc/curproc/file -> /bin/ls
# /mnt/ls -l /proc/curproc/file
lr--r--r--  1 root  wheel  0 Mar  9 13:53 /proc/curproc/file -> unknown

The second ls shows wrong info.
Comment 1 Robert Watson freebsd_committer freebsd_triage 2008-03-08 20:59:03 UTC
State Changed
From-To: open->suspended

This is an accurate bug report -- procfs and other related tools rely on 
the name cache to generate names for file system objects on demand. 
However, nullfs doesn't use the name cache, as the underlying file system 
layer will already be performing its own caching on the name space. 

Transition this to suspended as it's not clear what the right fix for 
this is.  From a performance perspective, it doesn't make a lot of sense 
to use the name cache for nullfs, and there is otherwise no good source 
of name data with which to build a name for an arbitrary vnode.  There is 
also no cache invalidation of "higher layers" with file system stacking 
for the name cache, so quite a lot of work would have to be done in order 
to implement it with proper name cache invalidation. 

Thanks for the report; hopefully we can interest someone in further 
investigating this in the future.
Comment 2 Mark Linimon freebsd_committer freebsd_triage 2009-05-18 05:25:33 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer(s).
Comment 3 Konstantin Belousov freebsd_committer freebsd_triage 2009-05-18 15:20:53 UTC
State Changed
From-To: suspended->open

I have prototyped VOP_VPTOCNP bypass for CURRENT. 


Comment 4 Konstantin Belousov freebsd_committer freebsd_triage 2009-05-18 15:20:53 UTC
Responsible Changed
From-To: freebsd-fs->kib

Take.
Comment 5 dfilter service freebsd_committer freebsd_triage 2009-05-31 15:58:58 UTC
Author: kib
Date: Sun May 31 14:58:43 2009
New Revision: 193175
URL: http://svn.freebsd.org/changeset/base/193175

Log:
  Implement the bypass routine for VOP_VPTOCNP in nullfs.
  Among other things, this makes procfs <pid>/file working for executables
  started from nullfs mount.
  
  Tested by:	pho
  PR:	94269, 104938

Modified:
  head/sys/fs/nullfs/null_vnops.c

Modified: head/sys/fs/nullfs/null_vnops.c
==============================================================================
--- head/sys/fs/nullfs/null_vnops.c	Sun May 31 14:57:43 2009	(r193174)
+++ head/sys/fs/nullfs/null_vnops.c	Sun May 31 14:58:43 2009	(r193175)
@@ -741,6 +741,55 @@ null_vptofh(struct vop_vptofh_args *ap)
 	return VOP_VPTOFH(lvp, ap->a_fhp);
 }
 
+static int
+null_vptocnp(struct vop_vptocnp_args *ap)
+{
+	struct vnode *vp = ap->a_vp;
+	struct vnode **dvp = ap->a_vpp;
+	struct vnode *lvp, *ldvp;
+	int error, locked;
+
+	if (vp->v_type == VDIR)
+		return (vop_stdvptocnp(ap));
+
+	locked = VOP_ISLOCKED(vp);
+	lvp = NULLVPTOLOWERVP(vp);
+	vhold(lvp);
+	VOP_UNLOCK(vp, 0); /* vp is held by vn_vptocnp_locked that called us */
+	ldvp = lvp;
+	error = vn_vptocnp(&ldvp, ap->a_buf, ap->a_buflen);
+	vdrop(lvp);
+	if (error != 0) {
+		vn_lock(vp, locked | LK_RETRY);
+		return (ENOENT);
+	}
+
+	/*
+	 * Exclusive lock is required by insmntque1 call in
+	 * null_nodeget()
+	 */
+	error = vn_lock(ldvp, LK_EXCLUSIVE);
+	if (error != 0) {
+		vn_lock(vp, locked | LK_RETRY);
+		vdrop(ldvp);
+		return (ENOENT);
+	}
+	vref(ldvp);
+	vdrop(ldvp);
+	error = null_nodeget(vp->v_mount, ldvp, dvp);
+	if (error == 0) {
+#ifdef DIAGNOSTIC
+		NULLVPTOLOWERVP(*dvp);
+#endif
+		vhold(*dvp);
+		vput(*dvp);
+	} else
+		vput(ldvp);
+
+	vn_lock(vp, locked | LK_RETRY);
+	return (error);
+}
+
 /*
  * Global vfs data structures
  */
@@ -762,6 +811,6 @@ struct vop_vector null_vnodeops = {
 	.vop_setattr =		null_setattr,
 	.vop_strategy =		VOP_EOPNOTSUPP,
 	.vop_unlock =		null_unlock,
-	.vop_vptocnp =		vop_stdvptocnp,
+	.vop_vptocnp =		null_vptocnp,
 	.vop_vptofh =		null_vptofh,
 };
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 Konstantin Belousov freebsd_committer freebsd_triage 2009-05-31 16:04:20 UTC
State Changed
From-To: open->closed

Patch is in HEAD, no MFC planned.