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;
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"
