From owner-svn-src-head@FreeBSD.ORG Mon Apr 20 14:36:01 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6B0C61065672; Mon, 20 Apr 2009 14:36:01 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 596218FC0C; Mon, 20 Apr 2009 14:36:01 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n3KEa11L048906; Mon, 20 Apr 2009 14:36:01 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n3KEa1UU048903; Mon, 20 Apr 2009 14:36:01 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <200904201436.n3KEa1UU048903@svn.freebsd.org> From: Konstantin Belousov Date: Mon, 20 Apr 2009 14:36:01 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r191315 - head/sys/ufs/ufs X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Apr 2009 14:36:01 -0000 Author: kib Date: Mon Apr 20 14:36:01 2009 New Revision: 191315 URL: http://svn.freebsd.org/changeset/base/191315 Log: In ufs_checkpath(), recheck that '..' still points to the inode with the same inode number after VFS_VGET() and relock of the vp. If '..' changed, redo the lookup. To reduce code duplication, move the code to read '..' dirent into the static helper function ufs_dir_dd_ino(). Supply the source inode number as an argument to ufs_checkpath() instead of the source inode itself. The inode is unlocked, thus it might be reclaimed, causing accesses to the freed memory. Use vn_vget_ino() to get the '..' vnode by its inode number, instead of directly code VFS_VGET() and relock, to properly busy the mount point while vp lock is dropped. Noted and reviewed by: tegge Tested by: pho MFC after: 1 month Modified: head/sys/ufs/ufs/ufs_extern.h head/sys/ufs/ufs/ufs_lookup.c head/sys/ufs/ufs/ufs_vnops.c Modified: head/sys/ufs/ufs/ufs_extern.h ============================================================================== --- head/sys/ufs/ufs/ufs_extern.h Mon Apr 20 14:35:42 2009 (r191314) +++ head/sys/ufs/ufs/ufs_extern.h Mon Apr 20 14:36:01 2009 (r191315) @@ -58,7 +58,7 @@ int ufs_bmap(struct vop_bmap_args *); int ufs_bmaparray(struct vnode *, ufs2_daddr_t, ufs2_daddr_t *, struct buf *, int *, int *); int ufs_fhtovp(struct mount *, struct ufid *, struct vnode **); -int ufs_checkpath(struct inode *, struct inode *, struct ucred *); +int ufs_checkpath(ino_t, struct inode *, struct ucred *); void ufs_dirbad(struct inode *, doff_t, char *); int ufs_dirbadentry(struct vnode *, struct direct *, int); int ufs_dirempty(struct inode *, ino_t, struct ucred *); Modified: head/sys/ufs/ufs/ufs_lookup.c ============================================================================== --- head/sys/ufs/ufs/ufs_lookup.c Mon Apr 20 14:35:42 2009 (r191314) +++ head/sys/ufs/ufs/ufs_lookup.c Mon Apr 20 14:36:01 2009 (r191315) @@ -1237,69 +1237,81 @@ ufs_dirempty(ip, parentino, cred) return (1); } +static int +ufs_dir_dd_ino(struct vnode *vp, struct ucred *cred, ino_t *dd_ino) +{ + struct dirtemplate dirbuf; + int error, namlen; + + if (vp->v_type != VDIR) + return (ENOTDIR); + error = vn_rdwr(UIO_READ, vp, (caddr_t)&dirbuf, + sizeof (struct dirtemplate), (off_t)0, UIO_SYSSPACE, + IO_NODELOCKED | IO_NOMACCHECK, cred, NOCRED, (int *)0, NULL); + if (error != 0) + return (error); +#if (BYTE_ORDER == LITTLE_ENDIAN) + if (OFSFMT(vp)) + namlen = dirbuf.dotdot_type; + else + namlen = dirbuf.dotdot_namlen; +#else + namlen = dirbuf.dotdot_namlen; +#endif + if (namlen != 2 || dirbuf.dotdot_name[0] != '.' || + dirbuf.dotdot_name[1] != '.') + return (ENOTDIR); + *dd_ino = dirbuf.dotdot_ino; + return (0); +} + /* * Check if source directory is in the path of the target directory. * Target is supplied locked, source is unlocked. * The target is always vput before returning. */ int -ufs_checkpath(source, target, cred) - struct inode *source, *target; - struct ucred *cred; +ufs_checkpath(ino_t source_ino, struct inode *target, struct ucred *cred) { - struct vnode *vp; - int error, namlen; - ino_t rootino; - struct dirtemplate dirbuf; + struct vnode *vp, *vp1; + int error; + ino_t dd_ino; vp = ITOV(target); - if (target->i_number == source->i_number) { + if (target->i_number == source_ino) { error = EEXIST; goto out; } - rootino = ROOTINO; error = 0; - if (target->i_number == rootino) + if (target->i_number == ROOTINO) goto out; for (;;) { - if (vp->v_type != VDIR) { - error = ENOTDIR; - break; - } - error = vn_rdwr(UIO_READ, vp, (caddr_t)&dirbuf, - sizeof (struct dirtemplate), (off_t)0, UIO_SYSSPACE, - IO_NODELOCKED | IO_NOMACCHECK, cred, NOCRED, (int *)0, - (struct thread *)0); + error = ufs_dir_dd_ino(vp, cred, &dd_ino); if (error != 0) break; -# if (BYTE_ORDER == LITTLE_ENDIAN) - if (OFSFMT(vp)) - namlen = dirbuf.dotdot_type; - else - namlen = dirbuf.dotdot_namlen; -# else - namlen = dirbuf.dotdot_namlen; -# endif - if (namlen != 2 || - dirbuf.dotdot_name[0] != '.' || - dirbuf.dotdot_name[1] != '.') { - error = ENOTDIR; - break; - } - if (dirbuf.dotdot_ino == source->i_number) { + if (dd_ino == source_ino) { error = EINVAL; break; } - if (dirbuf.dotdot_ino == rootino) + if (dd_ino == ROOTINO) break; - vput(vp); - error = VFS_VGET(vp->v_mount, dirbuf.dotdot_ino, - LK_EXCLUSIVE, &vp); - if (error) { - vp = NULL; + error = vn_vget_ino(vp, dd_ino, LK_EXCLUSIVE, &vp1); + if (error != 0) + break; + /* Recheck that ".." still points to vp1 after relock of vp */ + error = ufs_dir_dd_ino(vp, cred, &dd_ino); + if (error != 0) { + vput(vp1); break; } + /* Redo the check of ".." if directory was reparented */ + if (dd_ino != VTOI(vp1)->i_number) { + vput(vp1); + continue; + } + vput(vp); + vp = vp1; } out: Modified: head/sys/ufs/ufs/ufs_vnops.c ============================================================================== --- head/sys/ufs/ufs/ufs_vnops.c Mon Apr 20 14:35:42 2009 (r191314) +++ head/sys/ufs/ufs/ufs_vnops.c Mon Apr 20 14:36:01 2009 (r191315) @@ -1036,6 +1036,7 @@ ufs_rename(ap) struct direct newdir; int doingdirectory = 0, oldparent = 0, newparent = 0; int error = 0, ioflag; + ino_t fvp_ino; #ifdef INVARIANTS if ((tcnp->cn_flags & HASBUF) == 0 || @@ -1146,6 +1147,7 @@ abortit: * call to checkpath(). */ error = VOP_ACCESS(fvp, VWRITE, tcnp->cn_cred, tcnp->cn_thread); + fvp_ino = ip->i_number; VOP_UNLOCK(fvp, 0); if (oldparent != dp->i_number) newparent = dp->i_number; @@ -1154,7 +1156,7 @@ abortit: goto bad; if (xp != NULL) vput(tvp); - error = ufs_checkpath(ip, dp, tcnp->cn_cred); + error = ufs_checkpath(fvp_ino, dp, tcnp->cn_cred); if (error) goto out; if ((tcnp->cn_flags & SAVESTART) == 0)