>Number:         25963
>Category:       kern
>Synopsis:       incorrect refcounting in coda symlink vnops leads to panic
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jun 18 13:29:00 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator:     Greg Troxel
>Release:        NetBSD 1.6ZI
>Organization:
        Greg Troxel <gdt@ir.bbn.com>
>Environment:
	
	
System: netbsd-current from early february, but note that this file
has not changed since 2003-10-30
Architecture: i386
Machine: i386
>Description:
The coda symlink code has not been updated for the new lookup locking rules.

>How-To-Repeat:

Make a lot of symlinks in coda.  Note the eventual crash.

>Fix:

Thanks to Bill Studenmund for helping me through figuring this out and
suggesting how to fix.

Index: src/sys/coda/coda_vnops.c
===================================================================
RCS file: /SINEW-CVS/netbsd/src/sys/coda/coda_vnops.c,v
retrieving revision 1.1.1.1
retrieving revision 1.2
diff -u -r1.1.1.1 -r1.2
--- src/sys/coda/coda_vnops.c	30 Oct 2003 02:07:38 -0000	1.1.1.1
+++ src/sys/coda/coda_vnops.c	18 Jun 2004 12:31:56 -0000	1.2
@@ -1584,6 +1584,7 @@
     struct proc *p = cnp->cn_proc;
 /* locals */
     int error;
+    u_long saved_cn_flags;
     /* 
      * XXX I'm assuming the following things about coda_symlink's
      * arguments: 
@@ -1629,26 +1630,28 @@
     /* Invalidate the parent's attr cache, the modification time has changed */
     tdcp->c_flags &= ~C_VATTR;
 
-    if (!error)
-    {
-	struct nameidata nd;
-	NDINIT(&nd, LOOKUP, FOLLOW|LOCKLEAF, UIO_SYSSPACE, nm, p);
-	nd.ni_cnd.cn_cred = cred;
-	nd.ni_loopcnt = 0;
-	nd.ni_startdir = tdvp;
-	nd.ni_cnd.cn_pnbuf = (char *)nm;
-	nd.ni_cnd.cn_nameptr = nd.ni_cnd.cn_pnbuf;
-	nd.ni_pathlen = len;
-	vput(tdvp);
-	error = lookup(&nd);
-	*ap->a_vpp = nd.ni_vp;
-    }
-
-    /* 
-     * Free the name buffer 
-     */
-    if ((cnp->cn_flags & SAVESTART) == 0) {
-	PNBUF_PUT(cnp->cn_pnbuf);
+    if (!error) {
+	/*
+	 * VOP_SYMLINK is not defined to pay attention to cnp->cn_flags;
+	 * these are defined only for VOP_LOOKUP.   We desire to reuse
+	 * cnp for a VOP_LOOKUP operation, and must be sure to not pass
+	 * stray flags passed to us.  Such stray flags can occur because
+	 * sys_symlink makes a namei call and then reuses the
+	 * componentname structure.
+	 */
+	/*
+	 * XXX Arguably we should create our own componentname structure
+	 * and not reuse the one that was passed in.
+	 */
+	saved_cn_flags = cnp->cn_flags;
+	cnp->cn_flags &= ~(MODMASK | OPMASK);
+	cnp->cn_flags |= LOOKUP;
+	error = VOP_LOOKUP(tdvp, ap->a_vpp, cnp);
+	cnp->cn_flags = saved_cn_flags;
+	/* Either an error occurs, or ap->a_vpp is locked. */
+    } else {
+	/* error, so unlock and deference parent */
+        vput(tdvp);
     }
 
  exit:    

>Release-Note:
>Audit-Trail:
>Unformatted: