From e6796b67d904cd5e64c1befaa6eb3200bb695a20 Mon Sep 17 00:00:00 2001 From: Kirk McKusick Date: Tue, 4 Jul 2000 03:34:11 +0000 Subject: [PATCH] Move the truncation code out of vn_open and into the open system call after the acquisition of any advisory locks. This fix corrects a case in which a process tries to open a file with a non-blocking exclusive lock. Even if it fails to get the lock it would still truncate the file even though its open failed. With this change, the truncation is done only after the lock is successfully acquired. Obtained from: BSD/OS --- sys/dev/ccd/ccd.c | 5 +++-- sys/dev/vn/vn.c | 4 ++-- sys/geom/geom_ccd.c | 5 +++-- sys/kern/kern_acct.c | 5 +++-- sys/kern/kern_ktrace.c | 5 +++-- sys/kern/kern_linker.c | 5 +++-- sys/kern/kern_sig.c | 5 +++-- sys/kern/link_aout.c | 5 +++-- sys/kern/link_elf.c | 5 +++-- sys/kern/link_elf_obj.c | 5 +++-- sys/kern/vfs_extattr.c | 36 +++++++++++++++++++++++------------- sys/kern/vfs_syscalls.c | 36 +++++++++++++++++++++++------------- sys/kern/vfs_vnops.c | 37 +++++++++++++------------------------ sys/sys/vnode.h | 2 +- sys/ufs/ufs/ufs_extattr.c | 5 +++-- sys/ufs/ufs/ufs_quota.c | 5 +++-- 16 files changed, 95 insertions(+), 75 deletions(-) diff --git a/sys/dev/ccd/ccd.c b/sys/dev/ccd/ccd.c index 3f62c1ad5138..5a478f71abec 100644 --- a/sys/dev/ccd/ccd.c +++ b/sys/dev/ccd/ccd.c @@ -1538,10 +1538,11 @@ ccdlookup(path, p, vpp) { struct nameidata nd; struct vnode *vp; - int error; + int error, flags; NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, path, p); - if ((error = vn_open(&nd, FREAD|FWRITE, 0)) != 0) { + flags = FREAD | FWRITE; + if ((error = vn_open(&nd, &flags, 0)) != 0) { #ifdef DEBUG if (ccddebug & CCDB_FOLLOW|CCDB_INIT) printf("ccdlookup: vn_open error = %d\n", error); diff --git a/sys/dev/vn/vn.c b/sys/dev/vn/vn.c index 29f4e7a54453..88e3801fdd95 100644 --- a/sys/dev/vn/vn.c +++ b/sys/dev/vn/vn.c @@ -539,13 +539,13 @@ vniocattach_file(vn, vio, dev, flag, p) flags = FREAD|FWRITE; NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, vio->vn_file, p); - error = vn_open(&nd, flags, 0); + error = vn_open(&nd, &flags, 0); if (error) { if (error != EACCES && error != EPERM && error != EROFS) return (error); flags &= ~FWRITE; NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, vio->vn_file, p); - error = vn_open(&nd, flags, 0); + error = vn_open(&nd, &flags, 0); if (error) return (error); } diff --git a/sys/geom/geom_ccd.c b/sys/geom/geom_ccd.c index 3f62c1ad5138..5a478f71abec 100644 --- a/sys/geom/geom_ccd.c +++ b/sys/geom/geom_ccd.c @@ -1538,10 +1538,11 @@ ccdlookup(path, p, vpp) { struct nameidata nd; struct vnode *vp; - int error; + int error, flags; NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, path, p); - if ((error = vn_open(&nd, FREAD|FWRITE, 0)) != 0) { + flags = FREAD | FWRITE; + if ((error = vn_open(&nd, &flags, 0)) != 0) { #ifdef DEBUG if (ccddebug & CCDB_FOLLOW|CCDB_INIT) printf("ccdlookup: vn_open error = %d\n", error); diff --git a/sys/kern/kern_acct.c b/sys/kern/kern_acct.c index 42969c6e2cf5..ff982e0849b9 100644 --- a/sys/kern/kern_acct.c +++ b/sys/kern/kern_acct.c @@ -117,7 +117,7 @@ acct(a1, uap) { struct proc *p = curproc; /* XXX */ struct nameidata nd; - int error; + int error, flags; /* Make sure that the caller is root. */ error = suser(p); @@ -131,7 +131,8 @@ acct(a1, uap) if (SCARG(uap, path) != NULL) { NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_USERSPACE, SCARG(uap, path), p); - error = vn_open(&nd, FWRITE, 0); + flags = FWRITE; + error = vn_open(&nd, &flags, 0); if (error) return (error); NDFREE(&nd, NDF_ONLY_PNBUF); diff --git a/sys/kern/kern_ktrace.c b/sys/kern/kern_ktrace.c index 22f04e0e4bd7..14cc719b8026 100644 --- a/sys/kern/kern_ktrace.c +++ b/sys/kern/kern_ktrace.c @@ -253,7 +253,7 @@ ktrace(curp, uap) int ops = KTROP(uap->ops); int descend = uap->ops & KTRFLAG_DESCEND; int ret = 0; - int error = 0; + int flags, error = 0; struct nameidata nd; curp->p_traceflag |= KTRFAC_ACTIVE; @@ -262,7 +262,8 @@ ktrace(curp, uap) * an operation which requires a file argument. */ NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_USERSPACE, uap->fname, curp); - error = vn_open(&nd, FREAD|FWRITE|O_NOFOLLOW, 0); + flags = FREAD | FWRITE | O_NOFOLLOW; + error = vn_open(&nd, &flags, 0); if (error) { curp->p_traceflag &= ~KTRFAC_ACTIVE; return (error); diff --git a/sys/kern/kern_linker.c b/sys/kern/kern_linker.c index f81e0007832d..e06692425a51 100644 --- a/sys/kern/kern_linker.c +++ b/sys/kern/kern_linker.c @@ -1175,7 +1175,7 @@ linker_search_path(const char *name) struct nameidata nd; struct proc *p = curproc; /* XXX */ char *cp, *ep, *result, **cpp; - int error, extlen, len; + int error, extlen, len, flags; enum vtype type; /* qualified at all? */ @@ -1211,7 +1211,8 @@ linker_search_path(const char *name) * and it's a regular file. */ NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, result, p); - error = vn_open(&nd, FREAD, 0); + flags = FREAD; + error = vn_open(&nd, &flags, 0); if (error == 0) { NDFREE(&nd, NDF_ONLY_PNBUF); type = nd.ni_vp->v_type; diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 7702aba5846d..e96f47113b3a 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -1598,7 +1598,7 @@ coredump(p) register struct ucred *cred = p->p_ucred; struct nameidata nd; struct vattr vattr; - int error, error1; + int error, error1, flags; char *name; /* name of corefile */ off_t limit; @@ -1621,7 +1621,8 @@ coredump(p) name = expand_name(p->p_comm, p->p_ucred->cr_uid, p->p_pid); NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, p); - error = vn_open(&nd, O_CREAT | FWRITE | O_NOFOLLOW, S_IRUSR | S_IWUSR); + flags = O_CREAT | FWRITE | O_NOFOLLOW; + error = vn_open(&nd, &flags, S_IRUSR | S_IWUSR); free(name, M_TEMP); if (error) return (error); diff --git a/sys/kern/link_aout.c b/sys/kern/link_aout.c index 8dd4efac3b91..ff691e74ae55 100644 --- a/sys/kern/link_aout.c +++ b/sys/kern/link_aout.c @@ -192,13 +192,14 @@ link_aout_load_file(linker_class_t lc, const char* filename, linker_file_t* resu struct nameidata nd; struct proc* p = curproc; /* XXX */ int error = 0; - int resid; + int resid, flags; struct exec header; aout_file_t af; linker_file_t lf = 0; NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, filename, p); - error = vn_open(&nd, FREAD, 0); + flags = FREAD; + error = vn_open(&nd, &flags, 0); if (error) return error; NDFREE(&nd, NDF_ONLY_PNBUF); diff --git a/sys/kern/link_elf.c b/sys/kern/link_elf.c index 833f12eabccb..18e32b2d06d4 100644 --- a/sys/kern/link_elf.c +++ b/sys/kern/link_elf.c @@ -504,7 +504,7 @@ link_elf_load_file(linker_class_t cls, const char* filename, linker_file_t* resu Elf_Addr base_vaddr; Elf_Addr base_vlimit; int error = 0; - int resid; + int resid, flags; elf_file_t ef; linker_file_t lf; Elf_Shdr *shdr; @@ -517,7 +517,8 @@ link_elf_load_file(linker_class_t cls, const char* filename, linker_file_t* resu lf = NULL; NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, filename, p); - error = vn_open(&nd, FREAD, 0); + flags = FREAD; + error = vn_open(&nd, &flags, 0); if (error) return error; NDFREE(&nd, NDF_ONLY_PNBUF); diff --git a/sys/kern/link_elf_obj.c b/sys/kern/link_elf_obj.c index 833f12eabccb..18e32b2d06d4 100644 --- a/sys/kern/link_elf_obj.c +++ b/sys/kern/link_elf_obj.c @@ -504,7 +504,7 @@ link_elf_load_file(linker_class_t cls, const char* filename, linker_file_t* resu Elf_Addr base_vaddr; Elf_Addr base_vlimit; int error = 0; - int resid; + int resid, flags; elf_file_t ef; linker_file_t lf; Elf_Shdr *shdr; @@ -517,7 +517,8 @@ link_elf_load_file(linker_class_t cls, const char* filename, linker_file_t* resu lf = NULL; NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, filename, p); - error = vn_open(&nd, FREAD, 0); + flags = FREAD; + error = vn_open(&nd, &flags, 0); if (error) return error; NDFREE(&nd, NDF_ONLY_PNBUF); diff --git a/sys/kern/vfs_extattr.c b/sys/kern/vfs_extattr.c index 37dc81d697cb..65a297ca4619 100644 --- a/sys/kern/vfs_extattr.c +++ b/sys/kern/vfs_extattr.c @@ -968,9 +968,10 @@ open(p, uap) syscallarg(int) mode; } */ *uap; { - register struct filedesc *fdp = p->p_fd; - register struct file *fp; - register struct vnode *vp; + struct filedesc *fdp = p->p_fd; + struct file *fp; + struct vnode *vp; + struct vattr vat; int cmode, flags, oflags; struct file *nfp; int type, indx, error; @@ -988,7 +989,7 @@ open(p, uap) cmode = ((SCARG(uap, mode) &~ fdp->fd_cmask) & ALLPERMS) &~ S_ISTXT; NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, SCARG(uap, path), p); p->p_dupfd = -indx - 1; /* XXX check for fdopen */ - error = vn_open(&nd, flags, cmode); + error = vn_open(&nd, &flags, cmode); if (error) { ffree(fp); if ((error == ENODEV || error == ENXIO) && @@ -1011,6 +1012,7 @@ open(p, uap) fp->f_flag = flags & FMASK; fp->f_ops = &vnops; fp->f_type = (vp->v_type == VFIFO ? DTYPE_FIFO : DTYPE_VNODE); + VOP_UNLOCK(vp, 0, p); if (flags & (O_EXLOCK | O_SHLOCK)) { lf.l_whence = SEEK_SET; lf.l_start = 0; @@ -1022,22 +1024,30 @@ open(p, uap) type = F_FLOCK; if ((flags & FNONBLOCK) == 0) type |= F_WAIT; - VOP_UNLOCK(vp, 0, p); - if ((error = VOP_ADVLOCK(vp, (caddr_t)fp, F_SETLK, &lf, type)) != 0) { - (void) vn_close(vp, fp->f_flag, fp->f_cred, p); - ffree(fp); - fdp->fd_ofiles[indx] = NULL; - return (error); - } - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p); + if ((error = VOP_ADVLOCK(vp, (caddr_t)fp, F_SETLK, &lf, type)) != 0) + goto bad; fp->f_flag |= FHASLOCK; } + if (flags & O_TRUNC) { + VOP_LEASE(vp, p, p->p_ucred, LEASE_WRITE); + VATTR_NULL(&vat); + vat.va_size = 0; + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p); + error = VOP_SETATTR(vp, &vat, p->p_ucred, p); + VOP_UNLOCK(vp, 0, p); + if (error) + goto bad; + } /* assert that vn_open created a backing object if one is needed */ KASSERT(!vn_canvmio(vp) || vp->v_object != NULL, ("open: vmio vnode has no backing object after vn_open")); - VOP_UNLOCK(vp, 0, p); p->p_retval[0] = indx; return (0); +bad: + (void) vn_close(vp, fp->f_flag, fp->f_cred, p); + ffree(fp); + fdp->fd_ofiles[indx] = NULL; + return (error); } #ifdef COMPAT_43 diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c index 37dc81d697cb..65a297ca4619 100644 --- a/sys/kern/vfs_syscalls.c +++ b/sys/kern/vfs_syscalls.c @@ -968,9 +968,10 @@ open(p, uap) syscallarg(int) mode; } */ *uap; { - register struct filedesc *fdp = p->p_fd; - register struct file *fp; - register struct vnode *vp; + struct filedesc *fdp = p->p_fd; + struct file *fp; + struct vnode *vp; + struct vattr vat; int cmode, flags, oflags; struct file *nfp; int type, indx, error; @@ -988,7 +989,7 @@ open(p, uap) cmode = ((SCARG(uap, mode) &~ fdp->fd_cmask) & ALLPERMS) &~ S_ISTXT; NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, SCARG(uap, path), p); p->p_dupfd = -indx - 1; /* XXX check for fdopen */ - error = vn_open(&nd, flags, cmode); + error = vn_open(&nd, &flags, cmode); if (error) { ffree(fp); if ((error == ENODEV || error == ENXIO) && @@ -1011,6 +1012,7 @@ open(p, uap) fp->f_flag = flags & FMASK; fp->f_ops = &vnops; fp->f_type = (vp->v_type == VFIFO ? DTYPE_FIFO : DTYPE_VNODE); + VOP_UNLOCK(vp, 0, p); if (flags & (O_EXLOCK | O_SHLOCK)) { lf.l_whence = SEEK_SET; lf.l_start = 0; @@ -1022,22 +1024,30 @@ open(p, uap) type = F_FLOCK; if ((flags & FNONBLOCK) == 0) type |= F_WAIT; - VOP_UNLOCK(vp, 0, p); - if ((error = VOP_ADVLOCK(vp, (caddr_t)fp, F_SETLK, &lf, type)) != 0) { - (void) vn_close(vp, fp->f_flag, fp->f_cred, p); - ffree(fp); - fdp->fd_ofiles[indx] = NULL; - return (error); - } - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p); + if ((error = VOP_ADVLOCK(vp, (caddr_t)fp, F_SETLK, &lf, type)) != 0) + goto bad; fp->f_flag |= FHASLOCK; } + if (flags & O_TRUNC) { + VOP_LEASE(vp, p, p->p_ucred, LEASE_WRITE); + VATTR_NULL(&vat); + vat.va_size = 0; + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p); + error = VOP_SETATTR(vp, &vat, p->p_ucred, p); + VOP_UNLOCK(vp, 0, p); + if (error) + goto bad; + } /* assert that vn_open created a backing object if one is needed */ KASSERT(!vn_canvmio(vp) || vp->v_object != NULL, ("open: vmio vnode has no backing object after vn_open")); - VOP_UNLOCK(vp, 0, p); p->p_retval[0] = indx; return (0); +bad: + (void) vn_close(vp, fp->f_flag, fp->f_cred, p); + ffree(fp); + fdp->fd_ofiles[indx] = NULL; + return (error); } #ifdef COMPAT_43 diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c index 9dedcbe2c2b4..0d0dc24dc574 100644 --- a/sys/kern/vfs_vnops.c +++ b/sys/kern/vfs_vnops.c @@ -98,25 +98,25 @@ struct filterops vn_rwfiltops[] = { * due to the NDINIT being done elsewhere. */ int -vn_open(ndp, fmode, cmode) +vn_open(ndp, flagp, cmode) register struct nameidata *ndp; - int fmode, cmode; + int *flagp, cmode; { - register struct vnode *vp; - register struct proc *p = ndp->ni_cnd.cn_proc; - register struct ucred *cred = p->p_ucred; + struct vnode *vp; + struct proc *p = ndp->ni_cnd.cn_proc; + struct ucred *cred = p->p_ucred; struct vattr vat; struct vattr *vap = &vat; - int mode, error; + int mode, fmode, error; + fmode = *flagp; if (fmode & O_CREAT) { ndp->ni_cnd.cn_nameiop = CREATE; ndp->ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF; if ((fmode & O_EXCL) == 0 && (fmode & O_NOFOLLOW) == 0) ndp->ni_cnd.cn_flags |= FOLLOW; bwillwrite(); - error = namei(ndp); - if (error) + if ((error = namei(ndp)) != 0) return (error); if (ndp->ni_vp == NULL) { VATTR_NULL(vap); @@ -127,12 +127,11 @@ vn_open(ndp, fmode, cmode) VOP_LEASE(ndp->ni_dvp, p, cred, LEASE_WRITE); error = VOP_CREATE(ndp->ni_dvp, &ndp->ni_vp, &ndp->ni_cnd, vap); + vput(ndp->ni_dvp); if (error) { NDFREE(ndp, NDF_ONLY_PNBUF); - vput(ndp->ni_dvp); return (error); } - vput(ndp->ni_dvp); ASSERT_VOP_UNLOCKED(ndp->ni_dvp, "create"); ASSERT_VOP_LOCKED(ndp->ni_vp, "create"); fmode &= ~O_TRUNC; @@ -154,8 +153,7 @@ vn_open(ndp, fmode, cmode) ndp->ni_cnd.cn_nameiop = LOOKUP; ndp->ni_cnd.cn_flags = ((fmode & O_NOFOLLOW) ? NOFOLLOW : FOLLOW) | LOCKLEAF; - error = namei(ndp); - if (error) + if ((error = namei(ndp)) != 0) return (error); vp = ndp->ni_vp; } @@ -187,18 +185,7 @@ vn_open(ndp, fmode, cmode) goto bad; } } - if (fmode & O_TRUNC) { - VOP_UNLOCK(vp, 0, p); /* XXX */ - VOP_LEASE(vp, p, cred, LEASE_WRITE); - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p); /* XXX */ - VATTR_NULL(vap); - vap->va_size = 0; - error = VOP_SETATTR(vp, vap, cred, p); - if (error) - goto bad; - } - error = VOP_OPEN(vp, fmode, cred, p); - if (error) + if ((error = VOP_OPEN(vp, fmode, cred, p)) != 0) goto bad; /* * Make sure that a VM object is created for VMIO support. @@ -210,10 +197,12 @@ vn_open(ndp, fmode, cmode) if (fmode & FWRITE) vp->v_writecount++; + *flagp = fmode; return (0); bad: NDFREE(ndp, NDF_ONLY_PNBUF); vput(vp); + *flagp = fmode; return (error); } diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h index 4938dcab8b77..b2ab76c636d3 100644 --- a/sys/sys/vnode.h +++ b/sys/sys/vnode.h @@ -579,7 +579,7 @@ int debug_vn_lock __P((struct vnode *vp, int flags, struct proc *p, const char *filename, int line)); #define vn_lock(vp,flags,p) debug_vn_lock(vp,flags,p,__FILE__,__LINE__) #endif -int vn_open __P((struct nameidata *ndp, int fmode, int cmode)); +int vn_open __P((struct nameidata *ndp, int *flagp, int cmode)); void vn_pollevent __P((struct vnode *vp, int events)); void vn_pollgone __P((struct vnode *vp)); int vn_pollrecord __P((struct vnode *vp, struct proc *p, int events)); diff --git a/sys/ufs/ufs/ufs_extattr.c b/sys/ufs/ufs/ufs_extattr.c index b7d57f03ebc2..13dc80307879 100644 --- a/sys/ufs/ufs/ufs_extattr.c +++ b/sys/ufs/ufs/ufs_extattr.c @@ -307,7 +307,7 @@ ufs_extattrctl(struct mount *mp, int cmd, char *attrname, struct vnode *vp; char local_attrname[UFS_EXTATTR_MAXEXTATTRNAME]; /* inc null */ char *filename; - int error, len; + int error, len, flags; if ((error = suser_xxx(p->p_cred->pc_ucred, p, 0))) return (error); @@ -329,7 +329,8 @@ ufs_extattrctl(struct mount *mp, int cmd, char *attrname, filename = (char *) arg; NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, filename, p); - error = vn_open(&nd, FREAD|FWRITE, 0); + flags = FREAD | FWRITE; + error = vn_open(&nd, &flags, 0); if (error) return (error); diff --git a/sys/ufs/ufs/ufs_quota.c b/sys/ufs/ufs/ufs_quota.c index 6203e45955c2..574a330a566c 100644 --- a/sys/ufs/ufs/ufs_quota.c +++ b/sys/ufs/ufs/ufs_quota.c @@ -393,12 +393,13 @@ quotaon(p, mp, type, fname) struct vnode *vp, **vpp; struct vnode *nextvp; struct dquot *dq; - int error; + int error, flags; struct nameidata nd; vpp = &ump->um_quotas[type]; NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, fname, p); - error = vn_open(&nd, FREAD|FWRITE, 0); + flags = FREAD | FWRITE; + error = vn_open(&nd, &flags, 0); if (error) return (error); NDFREE(&nd, NDF_ONLY_PNBUF);