Could you send me a new version of the patch, that saves off the flag
  values? Also, I'd say save the old flags, set the whole flag word to what
  you want, then restore the old contents. For instance, do we want
  SAVEPARENT, etc.

Patch at end.

I am pretty sure we do not want SAVEPARENT.  The code unlocks/unrefs
the parent on error, and this should happen on the normal case too.

Sorry for the long delay.  My 'bash the flags' patch was working well
enough that this was lower on my list....

  I can commit it to -current, but I don't have 1.6 to test against, so I'm
  hesitant about a pullup.

I have just tested the below patch on 1.6.1_RC3 (because that's what
my project and its repo are at).  It seems to work fine.  Since the
existing code is clearly broken (by inspection and it crashes), if the
patch works for you on current, I'd say 1.6-stable is better off with
the patch than without.

        Greg Troxel <gdt@ir.bbn.com>

Index: coda_vnops.c
===================================================================
RCS file: /QUIST-CVS/netbsd/src/sys/coda/coda_vnops.c,v
retrieving revision 1.1.1.2
retrieving revision 1.4
diff -u -r1.1.1.2 -r1.4
--- coda_vnops.c	2001/12/06 04:27:40	1.1.1.2
+++ coda_vnops.c	2003/05/28 20:34:34	1.4
@@ -1597,6 +1597,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: 
@@ -1642,26 +1643,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: