Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Jun 2014 07:11:01 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r267564 - in head/sys: fs/devfs fs/msdosfs fs/tmpfs kern sys ufs/ufs
Message-ID:  <201406170711.s5H7B1G2006079@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Tue Jun 17 07:11:00 2014
New Revision: 267564
URL: http://svnweb.freebsd.org/changeset/base/267564

Log:
  In msdosfs_setattr(), add a check for result of the utimes(2)
  permissions test, forgotten in r164033.
  
  Refactor the permission checks for utimes(2) into vnode helper
  function vn_utimes_perm(9), and simplify its code comparing with the
  UFS origin, by writing the call to VOP_ACCESSX only once.  Use the
  helper for UFS(5), tmpfs(5), devfs(5) and msdosfs(5).
  
  Reported by:	bde
  Reviewed by:	bde, trasz
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Modified:
  head/sys/fs/devfs/devfs_vnops.c
  head/sys/fs/msdosfs/msdosfs_vnops.c
  head/sys/fs/tmpfs/tmpfs.h
  head/sys/fs/tmpfs/tmpfs_subr.c
  head/sys/fs/tmpfs/tmpfs_vnops.c
  head/sys/kern/vfs_vnops.c
  head/sys/sys/vnode.h
  head/sys/ufs/ufs/ufs_vnops.c

Modified: head/sys/fs/devfs/devfs_vnops.c
==============================================================================
--- head/sys/fs/devfs/devfs_vnops.c	Tue Jun 17 05:29:18 2014	(r267563)
+++ head/sys/fs/devfs/devfs_vnops.c	Tue Jun 17 07:11:00 2014	(r267564)
@@ -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: head/sys/fs/msdosfs/msdosfs_vnops.c
==============================================================================
--- head/sys/fs/msdosfs/msdosfs_vnops.c	Tue Jun 17 05:29:18 2014	(r267563)
+++ head/sys/fs/msdosfs/msdosfs_vnops.c	Tue Jun 17 07:11:00 2014	(r267564)
@@ -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: head/sys/fs/tmpfs/tmpfs.h
==============================================================================
--- head/sys/fs/tmpfs/tmpfs.h	Tue Jun 17 05:29:18 2014	(r267563)
+++ head/sys/fs/tmpfs/tmpfs.h	Tue Jun 17 07:11:00 2014	(r267564)
@@ -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: head/sys/fs/tmpfs/tmpfs_subr.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_subr.c	Tue Jun 17 05:29:18 2014	(r267563)
+++ head/sys/fs/tmpfs/tmpfs_subr.c	Tue Jun 17 07:11:00 2014	(r267564)
@@ -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: head/sys/fs/tmpfs/tmpfs_vnops.c
==============================================================================
--- head/sys/fs/tmpfs/tmpfs_vnops.c	Tue Jun 17 05:29:18 2014	(r267563)
+++ head/sys/fs/tmpfs/tmpfs_vnops.c	Tue Jun 17 07:11:00 2014	(r267564)
@@ -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: head/sys/kern/vfs_vnops.c
==============================================================================
--- head/sys/kern/vfs_vnops.c	Tue Jun 17 05:29:18 2014	(r267563)
+++ head/sys/kern/vfs_vnops.c	Tue Jun 17 07:11:00 2014	(r267564)
@@ -2170,3 +2170,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: head/sys/sys/vnode.h
==============================================================================
--- head/sys/sys/vnode.h	Tue Jun 17 05:29:18 2014	(r267563)
+++ head/sys/sys/vnode.h	Tue Jun 17 07:11:00 2014	(r267564)
@@ -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: head/sys/ufs/ufs/ufs_vnops.c
==============================================================================
--- head/sys/ufs/ufs/ufs_vnops.c	Tue Jun 17 05:29:18 2014	(r267563)
+++ head/sys/ufs/ufs/ufs_vnops.c	Tue Jun 17 07:11:00 2014	(r267564)
@@ -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;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201406170711.s5H7B1G2006079>