From owner-freebsd-current Tue Jun 15 1:40:36 1999 Delivered-To: freebsd-current@freebsd.org Received: from apollo.backplane.com (apollo.backplane.com [209.157.86.2]) by hub.freebsd.org (Postfix) with ESMTP id 0B93A14DC8 for ; Tue, 15 Jun 1999 01:40:30 -0700 (PDT) (envelope-from dillon@apollo.backplane.com) Received: (from dillon@localhost) by apollo.backplane.com (8.9.3/8.9.1) id BAA15539; Tue, 15 Jun 1999 01:39:14 -0700 (PDT) (envelope-from dillon) Date: Tue, 15 Jun 1999 01:39:14 -0700 (PDT) From: Matthew Dillon Message-Id: <199906150839.BAA15539@apollo.backplane.com> To: Ville-Pertti Keinonen Cc: current@freebsd.org Subject: Re: NFS vnode reference issues on server References: <199906150421.VAA14439@apollo.backplane.com.newsgate.clinet.fi> <86d7yxoqj6.fsf@not.demophon.com> Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG :Matthew Dillon writes: : :> Ok, something for people following the code to look over if they have :> the time. This in nfs_subs.c, nfs_namei(). Question: ndp->ni_vp :> is non-NULL and appears to be referenced as of the time a badlink :> occurs, linklen is 0, or the link is too long. Do we have to :> release ndp->ni_vp and NULL it out in this case? I believe so. : :Yes, and it would seem that you should also do this for the previous :error return (ELOOP). There are no special cases for specific errors :so all exits with error conditions should behave identically. Callers :bail out without doing anything further with the nameidata on errors, :apparently in all cases. : :A reference to the directory vnode appears to be lost in most cases of :errors, as well (once again, the reference in the nameidata). In :*some* cases, there is a reference in retdirp, but it's a different :reference. Yup. I think I caught them all. Here is a new preliminary nfs_namei(). I think I caught all the weird cases. The ref counts related to *retdirp weren't broken, but they were confusing. ELOOP was broken too ... I had missed that. (Anyone who wants to) Take a look at this one and tell me what you think. -Matt Matthew Dillon #ifndef NFS_NOSERVER /* * Set up nameidata for a lookup() call and do it. * * If pubflag is set, this call is done for a lookup operation on the * public filehandle. In that case we allow crossing mountpoints and * absolute pathnames. However, the caller is expected to check that * the lookup result is within the public fs, and deny access if * it is not. * * If an error is returned, ndp->ni_vp will be left NULL. * * dirp may be set whether an error is returned or not, and must be * released by the caller. */ int nfs_namei(ndp, fhp, len, slp, nam, mdp, dposp, retdirp, p, kerbflag, pubflag) register struct nameidata *ndp; fhandle_t *fhp; int len; struct nfssvc_sock *slp; struct sockaddr *nam; struct mbuf **mdp; caddr_t *dposp; struct vnode **retdirp; struct proc *p; int kerbflag, pubflag; { register int i, rem; register struct mbuf *md; register char *fromcp, *tocp, *cp; struct iovec aiov; struct uio auio; struct vnode *dp; int error, rdonly, linklen; struct componentname *cnp = &ndp->ni_cnd; *retdirp = (struct vnode *)0; cnp->cn_pnbuf = zalloc(namei_zone); /* * Copy the name from the mbuf list to ndp->ni_pnbuf * and set the various ndp fields appropriately. */ fromcp = *dposp; tocp = cnp->cn_pnbuf; md = *mdp; rem = mtod(md, caddr_t) + md->m_len - fromcp; cnp->cn_hash = 0; for (i = 0; i < len; i++) { while (rem == 0) { md = md->m_next; if (md == NULL) { error = EBADRPC; goto out; } fromcp = mtod(md, caddr_t); rem = md->m_len; } if (*fromcp == '\0' || (!pubflag && *fromcp == '/')) { error = EACCES; goto out; } cnp->cn_hash += (unsigned char)*fromcp; *tocp++ = *fromcp++; rem--; } *tocp = '\0'; *mdp = md; *dposp = fromcp; len = nfsm_rndup(len)-len; if (len > 0) { if (rem >= len) *dposp += len; else if ((error = nfs_adv(mdp, dposp, len, rem)) != 0) goto out; } /* * Extract and set starting directory. */ error = nfsrv_fhtovp(fhp, FALSE, &dp, ndp->ni_cnd.cn_cred, slp, nam, &rdonly, kerbflag, pubflag); if (error) goto out; if (dp->v_type != VDIR) { vrele(dp); error = ENOTDIR; goto out; } if (rdonly) cnp->cn_flags |= RDONLY; /* * Set return directory. Reference to dp is implicitly transfered * to the returned pointer */ *retdirp = dp; if (pubflag) { /* * Oh joy. For WebNFS, handle those pesky '%' escapes, * and the 'native path' indicator. */ cp = zalloc(namei_zone); fromcp = cnp->cn_pnbuf; tocp = cp; if ((unsigned char)*fromcp >= WEBNFS_SPECCHAR_START) { switch ((unsigned char)*fromcp) { case WEBNFS_NATIVE_CHAR: /* * 'Native' path for us is the same * as a path according to the NFS spec, * just skip the escape char. */ fromcp++; break; /* * More may be added in the future, range 0x80-0xff */ default: error = EIO; zfree(namei_zone, cp); goto out; } } /* * Translate the '%' escapes, URL-style. */ while (*fromcp != '\0') { if (*fromcp == WEBNFS_ESC_CHAR) { if (fromcp[1] != '\0' && fromcp[2] != '\0') { fromcp++; *tocp++ = HEXSTRTOI(fromcp); fromcp += 2; continue; } else { error = ENOENT; zfree(namei_zone, cp); goto out; } } else *tocp++ = *fromcp++; } *tocp = '\0'; zfree(namei_zone, cnp->cn_pnbuf); cnp->cn_pnbuf = cp; } ndp->ni_pathlen = (tocp - cnp->cn_pnbuf) + 1; ndp->ni_segflg = UIO_SYSSPACE; if (pubflag) { ndp->ni_rootdir = rootvnode; ndp->ni_loopcnt = 0; if (cnp->cn_pnbuf[0] == '/') dp = rootvnode; } else { cnp->cn_flags |= NOCROSSMOUNT; } /* * Initialize for scan, set ni_startdir and bump ref on dp again * becuase lookup() will dereference ni_startdir. */ cnp->cn_proc = p; VREF(dp); ndp->ni_startdir = dp; for (;;) { cnp->cn_nameptr = cnp->cn_pnbuf; /* * Call lookup() to do the real work. If an error occurs, * ndp->ni_vp and ni_dvp are left uninitialized or NULL and * we do not have to dereference anything before returning. * In either case ni_startdir will be dereferenced and NULLed * out. */ error = lookup(ndp); if (error) break; /* * Check for encountering a symbolic link. Trivial * termination occurs if no symlink encountered. */ if ((cnp->cn_flags & ISSYMLINK) == 0) { nfsrv_object_create(ndp->ni_vp); if (cnp->cn_flags & (SAVENAME | SAVESTART)) { cnp->cn_flags |= HASBUF; return (0); } /* * no error, but do not save buffer either. Break * out of loop ( basically goto out; ). */ break; } /* * Validate symlink */ if ((cnp->cn_flags & LOCKPARENT) && ndp->ni_pathlen == 1) VOP_UNLOCK(ndp->ni_dvp, 0, p); if (!pubflag) { error = EINVAL; goto badlink2; } if (ndp->ni_loopcnt++ >= MAXSYMLINKS) { error = ELOOP; goto badlink2; } if (ndp->ni_pathlen > 1) cp = zalloc(namei_zone); else cp = cnp->cn_pnbuf; aiov.iov_base = cp; aiov.iov_len = MAXPATHLEN; auio.uio_iov = &aiov; auio.uio_iovcnt = 1; auio.uio_offset = 0; auio.uio_rw = UIO_READ; auio.uio_segflg = UIO_SYSSPACE; auio.uio_procp = (struct proc *)0; auio.uio_resid = MAXPATHLEN; error = VOP_READLINK(ndp->ni_vp, &auio, cnp->cn_cred); if (error) { badlink1: if (ndp->ni_pathlen > 1) zfree(namei_zone, cp); badlink2: vrele(ndp->ni_dvp); vput(ndp->ni_vp); ndp->ni_vp = NULL; ndp->ni_dvp = NULL; break; } linklen = MAXPATHLEN - auio.uio_resid; if (linklen == 0) { error = ENOENT; goto badlink1; } if (linklen + ndp->ni_pathlen >= MAXPATHLEN) { error = ENAMETOOLONG; goto badlink1; } /* * Adjust or replace path */ if (ndp->ni_pathlen > 1) { bcopy(ndp->ni_next, cp + linklen, ndp->ni_pathlen); zfree(namei_zone, cnp->cn_pnbuf); cnp->cn_pnbuf = cp; } else cnp->cn_pnbuf[linklen] = '\0'; ndp->ni_pathlen += linklen; /* * Cleanup refs for next loop and check if root directory * should replace current directory. Normally ni_dvp * becomes the new base directory and is cleaned up when * we loop. Explicitly null pointers after invalidation * to clarify operation. */ vput(ndp->ni_vp); ndp->ni_vp = NULL; if (cnp->cn_pnbuf[0] == '/') { vrele(ndp->ni_dvp); ndp->ni_dvp = ndp->ni_rootdir; VREF(ndp->ni_dvp); } ndp->ni_startdir = ndp->ni_dvp; ndp->ni_dvp = NULL; } out: zfree(namei_zone, cnp->cn_pnbuf); return (error); } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message