From owner-svn-src-all@FreeBSD.ORG Tue Jun 24 08:21:45 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 1DAC7B09; Tue, 24 Jun 2014 08:21:45 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 092FB2C75; Tue, 24 Jun 2014 08:21:45 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.8/8.14.8) with ESMTP id s5O8LikM077804; Tue, 24 Jun 2014 08:21:44 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.8/8.14.8/Submit) id s5O8Lhjm077794; Tue, 24 Jun 2014 08:21:43 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201406240821.s5O8Lhjm077794@svn.freebsd.org> From: Konstantin Belousov Date: Tue, 24 Jun 2014 08:21:43 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r267816 - in stable/10/sys: fs/devfs fs/msdosfs fs/tmpfs kern sys ufs/ufs X-SVN-Group: stable-10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Jun 2014 08:21:45 -0000 Author: kib Date: Tue Jun 24 08:21:43 2014 New Revision: 267816 URL: http://svnweb.freebsd.org/changeset/base/267816 Log: MFC r267564: In msdosfs_setattr(), add a check for result of the utimes(2) permissions test. Refactor the permission checks for utimes(2). Modified: stable/10/sys/fs/devfs/devfs_vnops.c stable/10/sys/fs/msdosfs/msdosfs_vnops.c stable/10/sys/fs/tmpfs/tmpfs.h stable/10/sys/fs/tmpfs/tmpfs_subr.c stable/10/sys/fs/tmpfs/tmpfs_vnops.c stable/10/sys/kern/vfs_vnops.c stable/10/sys/sys/vnode.h stable/10/sys/ufs/ufs/ufs_vnops.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/fs/devfs/devfs_vnops.c ============================================================================== --- stable/10/sys/fs/devfs/devfs_vnops.c Tue Jun 24 06:55:49 2014 (r267815) +++ stable/10/sys/fs/devfs/devfs_vnops.c Tue Jun 24 08:21:43 2014 (r267816) @@ -1533,10 +1533,8 @@ devfs_setattr(struct vop_setattr_args *a } if (vap->va_atime.tv_sec != VNOVAL || vap->va_mtime.tv_sec != VNOVAL) { - /* See the comment in ufs_vnops::ufs_setattr(). */ - if ((error = VOP_ACCESS(vp, VADMIN, ap->a_cred, td)) && - ((vap->va_vaflags & VA_UTIMES_NULL) == 0 || - (error = VOP_ACCESS(vp, VWRITE, ap->a_cred, td)))) + error = vn_utimes_perm(vp, vap, ap->a_cred, td); + if (error != 0) return (error); if (vap->va_atime.tv_sec != VNOVAL) { if (vp->v_type == VCHR) Modified: stable/10/sys/fs/msdosfs/msdosfs_vnops.c ============================================================================== --- stable/10/sys/fs/msdosfs/msdosfs_vnops.c Tue Jun 24 06:55:49 2014 (r267815) +++ stable/10/sys/fs/msdosfs/msdosfs_vnops.c Tue Jun 24 08:21:43 2014 (r267816) @@ -501,12 +501,9 @@ msdosfs_setattr(ap) if (vap->va_atime.tv_sec != VNOVAL || vap->va_mtime.tv_sec != VNOVAL) { if (vp->v_mount->mnt_flag & MNT_RDONLY) return (EROFS); - if (vap->va_vaflags & VA_UTIMES_NULL) { - error = VOP_ACCESS(vp, VADMIN, cred, td); - if (error) - error = VOP_ACCESS(vp, VWRITE, cred, td); - } else - error = VOP_ACCESS(vp, VADMIN, cred, td); + error = vn_utimes_perm(vp, vap, cred, td); + if (error != 0) + return (error); if ((pmp->pm_flags & MSDOSFSMNT_NOWIN95) == 0 && vap->va_atime.tv_sec != VNOVAL) { dep->de_flag &= ~DE_ACCESS; Modified: stable/10/sys/fs/tmpfs/tmpfs.h ============================================================================== --- stable/10/sys/fs/tmpfs/tmpfs.h Tue Jun 24 06:55:49 2014 (r267815) +++ stable/10/sys/fs/tmpfs/tmpfs.h Tue Jun 24 08:21:43 2014 (r267816) @@ -425,8 +425,8 @@ int tmpfs_chmod(struct vnode *, mode_t, int tmpfs_chown(struct vnode *, uid_t, gid_t, struct ucred *, struct thread *); int tmpfs_chsize(struct vnode *, u_quad_t, struct ucred *, struct thread *); -int tmpfs_chtimes(struct vnode *, struct timespec *, struct timespec *, - struct timespec *, int, struct ucred *, struct thread *); +int tmpfs_chtimes(struct vnode *, struct vattr *, struct ucred *cred, + struct thread *); void tmpfs_itimes(struct vnode *, const struct timespec *, const struct timespec *); Modified: stable/10/sys/fs/tmpfs/tmpfs_subr.c ============================================================================== --- stable/10/sys/fs/tmpfs/tmpfs_subr.c Tue Jun 24 06:55:49 2014 (r267815) +++ stable/10/sys/fs/tmpfs/tmpfs_subr.c Tue Jun 24 08:21:43 2014 (r267816) @@ -1677,8 +1677,8 @@ tmpfs_chsize(struct vnode *vp, u_quad_t * The vnode must be locked on entry and remain locked on exit. */ int -tmpfs_chtimes(struct vnode *vp, struct timespec *atime, struct timespec *mtime, - struct timespec *birthtime, int vaflags, struct ucred *cred, struct thread *l) +tmpfs_chtimes(struct vnode *vp, struct vattr *vap, + struct ucred *cred, struct thread *l) { int error; struct tmpfs_node *node; @@ -1695,29 +1695,25 @@ tmpfs_chtimes(struct vnode *vp, struct t if (node->tn_flags & (IMMUTABLE | APPEND)) return EPERM; - /* Determine if the user have proper privilege to update time. */ - if (vaflags & VA_UTIMES_NULL) { - error = VOP_ACCESS(vp, VADMIN, cred, l); - if (error) - error = VOP_ACCESS(vp, VWRITE, cred, l); - } else - error = VOP_ACCESS(vp, VADMIN, cred, l); - if (error) + error = vn_utimes_perm(vp, vap, cred, l); + if (error != 0) return (error); - if (atime->tv_sec != VNOVAL && atime->tv_nsec != VNOVAL) + if (vap->va_atime.tv_sec != VNOVAL && vap->va_atime.tv_nsec != VNOVAL) node->tn_status |= TMPFS_NODE_ACCESSED; - if (mtime->tv_sec != VNOVAL && mtime->tv_nsec != VNOVAL) + if (vap->va_mtime.tv_sec != VNOVAL && vap->va_mtime.tv_nsec != VNOVAL) node->tn_status |= TMPFS_NODE_MODIFIED; - if (birthtime->tv_nsec != VNOVAL && birthtime->tv_nsec != VNOVAL) + if (vap->va_birthtime.tv_nsec != VNOVAL && + vap->va_birthtime.tv_nsec != VNOVAL) node->tn_status |= TMPFS_NODE_MODIFIED; - tmpfs_itimes(vp, atime, mtime); + tmpfs_itimes(vp, &vap->va_atime, &vap->va_mtime); - if (birthtime->tv_nsec != VNOVAL && birthtime->tv_nsec != VNOVAL) - node->tn_birthtime = *birthtime; + if (vap->va_birthtime.tv_nsec != VNOVAL && + vap->va_birthtime.tv_nsec != VNOVAL) + node->tn_birthtime = vap->va_birthtime; MPASS(VOP_ISLOCKED(vp)); return 0; Modified: stable/10/sys/fs/tmpfs/tmpfs_vnops.c ============================================================================== --- stable/10/sys/fs/tmpfs/tmpfs_vnops.c Tue Jun 24 06:55:49 2014 (r267815) +++ stable/10/sys/fs/tmpfs/tmpfs_vnops.c Tue Jun 24 08:21:43 2014 (r267816) @@ -378,10 +378,6 @@ tmpfs_getattr(struct vop_getattr_args *v return 0; } -/* --------------------------------------------------------------------- */ - -/* XXX Should this operation be atomic? I think it should, but code in - * XXX other places (e.g., ufs) doesn't seem to be... */ int tmpfs_setattr(struct vop_setattr_args *v) { @@ -425,8 +421,7 @@ tmpfs_setattr(struct vop_setattr_args *v vap->va_mtime.tv_nsec != VNOVAL) || (vap->va_birthtime.tv_sec != VNOVAL && vap->va_birthtime.tv_nsec != VNOVAL))) - error = tmpfs_chtimes(vp, &vap->va_atime, &vap->va_mtime, - &vap->va_birthtime, vap->va_vaflags, cred, td); + error = tmpfs_chtimes(vp, vap, cred, td); /* Update the node times. We give preference to the error codes * generated by this function rather than the ones that may arise Modified: stable/10/sys/kern/vfs_vnops.c ============================================================================== --- stable/10/sys/kern/vfs_vnops.c Tue Jun 24 06:55:49 2014 (r267815) +++ stable/10/sys/kern/vfs_vnops.c Tue Jun 24 08:21:43 2014 (r267816) @@ -2085,3 +2085,27 @@ drop: foffset_unlock(fp, offset, error != 0 ? FOF_NOUPDATE : 0); return (error); } + +int +vn_utimes_perm(struct vnode *vp, struct vattr *vap, struct ucred *cred, + struct thread *td) +{ + int error; + + error = VOP_ACCESSX(vp, VWRITE_ATTRIBUTES, cred, td); + + /* + * From utimes(2): + * Grant permission if the caller is the owner of the file or + * the super-user. If the time pointer is null, then write + * permission on the file is also sufficient. + * + * From NFSv4.1, draft 21, 6.2.1.3.1, Discussion of Mask Attributes: + * A user having ACL_WRITE_DATA or ACL_WRITE_ATTRIBUTES + * will be allowed to set the times [..] to the current + * server time. + */ + if (error != 0 && (vap->va_vaflags & VA_UTIMES_NULL) != 0) + error = VOP_ACCESS(vp, VWRITE, cred, td); + return (error); +} Modified: stable/10/sys/sys/vnode.h ============================================================================== --- stable/10/sys/sys/vnode.h Tue Jun 24 06:55:49 2014 (r267815) +++ stable/10/sys/sys/vnode.h Tue Jun 24 08:21:43 2014 (r267816) @@ -696,6 +696,8 @@ int vn_extattr_rm(struct vnode *vp, int const char *attrname, struct thread *td); int vn_vget_ino(struct vnode *vp, ino_t ino, int lkflags, struct vnode **rvp); +int vn_utimes_perm(struct vnode *vp, struct vattr *vap, + struct ucred *cred, struct thread *td); int vn_io_fault_uiomove(char *data, int xfersize, struct uio *uio); int vn_io_fault_pgmove(vm_page_t ma[], vm_offset_t offset, int xfersize, Modified: stable/10/sys/ufs/ufs/ufs_vnops.c ============================================================================== --- stable/10/sys/ufs/ufs/ufs_vnops.c Tue Jun 24 06:55:49 2014 (r267815) +++ stable/10/sys/ufs/ufs/ufs_vnops.c Tue Jun 24 08:21:43 2014 (r267816) @@ -635,35 +635,8 @@ ufs_setattr(ap) return (EROFS); if ((ip->i_flags & SF_SNAPSHOT) != 0) return (EPERM); - /* - * From utimes(2): - * If times is NULL, ... The caller must be the owner of - * the file, have permission to write the file, or be the - * super-user. - * If times is non-NULL, ... The caller must be the owner of - * the file or be the super-user. - * - * Possibly for historical reasons, try to use VADMIN in - * preference to VWRITE for a NULL timestamp. This means we - * will return EACCES in preference to EPERM if neither - * check succeeds. - */ - if (vap->va_vaflags & VA_UTIMES_NULL) { - /* - * NFSv4.1, draft 21, 6.2.1.3.1, Discussion of Mask Attributes - * - * "A user having ACL_WRITE_DATA or ACL_WRITE_ATTRIBUTES - * will be allowed to set the times [..] to the current - * server time." - * - * XXX: Calling it four times seems a little excessive. - */ - error = VOP_ACCESSX(vp, VWRITE_ATTRIBUTES, cred, td); - if (error) - error = VOP_ACCESS(vp, VWRITE, cred, td); - } else - error = VOP_ACCESSX(vp, VWRITE_ATTRIBUTES, cred, td); - if (error) + error = vn_utimes_perm(vp, vap, cred, td); + if (error != 0) return (error); if (vap->va_atime.tv_sec != VNOVAL) ip->i_flag |= IN_ACCESS;