Commit 9767d74957450da6365c363d69e3d02d605d7375

Authored by Miklos Szeredi
Committed by Al Viro
1 parent 88b387824f

[patch 1/4] vfs: utimes: move owner check into inode_change_ok()

Add a new ia_valid flag: ATTR_TIMES_SET, to handle the
UTIMES_OMIT/UTIMES_NOW and UTIMES_NOW/UTIMES_OMIT cases.  In these
cases neither ATTR_MTIME_SET nor ATTR_ATIME_SET is in the flags, yet
the POSIX draft specifies that permission checking is performed the
same way as if one or both of the times was explicitly set to a
timestamp.

See the path "vfs: utimensat(): fix error checking for
{UTIME_NOW,UTIME_OMIT} case" by Michael Kerrisk for the patch
introducing this behavior.

This is a cleanup, as well as allowing filesystems (NFS/fuse/...) to
perform their own permission checking instead of the default.

CC: Ulrich Drepper <drepper@redhat.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Showing 3 changed files with 22 additions and 30 deletions Side-by-side Diff

... ... @@ -51,7 +51,7 @@
51 51 }
52 52  
53 53 /* Check for setting the inode time. */
54   - if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET)) {
  54 + if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)) {
55 55 if (!is_owner_or_cap(inode))
56 56 goto error;
57 57 }
... ... @@ -101,7 +101,6 @@
101 101 times[1].tv_nsec == UTIME_NOW)
102 102 times = NULL;
103 103  
104   - /* In most cases, the checks are done in inode_change_ok() */
105 104 newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
106 105 if (times) {
107 106 error = -EPERM;
108 107  
109 108  
110 109  
... ... @@ -123,21 +122,13 @@
123 122 newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
124 123 newattrs.ia_valid |= ATTR_MTIME_SET;
125 124 }
126   -
127 125 /*
128   - * For the UTIME_OMIT/UTIME_NOW and UTIME_NOW/UTIME_OMIT
129   - * cases, we need to make an extra check that is not done by
130   - * inode_change_ok().
  126 + * Tell inode_change_ok(), that this is an explicit time
  127 + * update, even if neither ATTR_ATIME_SET nor ATTR_MTIME_SET
  128 + * were used.
131 129 */
132   - if (((times[0].tv_nsec == UTIME_NOW &&
133   - times[1].tv_nsec == UTIME_OMIT)
134   - ||
135   - (times[0].tv_nsec == UTIME_OMIT &&
136   - times[1].tv_nsec == UTIME_NOW))
137   - && !is_owner_or_cap(inode))
138   - goto mnt_drop_write_and_out;
  130 + newattrs.ia_valid |= ATTR_TIMES_SET;
139 131 } else {
140   -
141 132 /*
142 133 * If times is NULL (or both times are UTIME_NOW),
143 134 * then we need to check permissions, because
... ... @@ -320,22 +320,23 @@
320 320 * Attribute flags. These should be or-ed together to figure out what
321 321 * has been changed!
322 322 */
323   -#define ATTR_MODE 1
324   -#define ATTR_UID 2
325   -#define ATTR_GID 4
326   -#define ATTR_SIZE 8
327   -#define ATTR_ATIME 16
328   -#define ATTR_MTIME 32
329   -#define ATTR_CTIME 64
330   -#define ATTR_ATIME_SET 128
331   -#define ATTR_MTIME_SET 256
332   -#define ATTR_FORCE 512 /* Not a change, but a change it */
333   -#define ATTR_ATTR_FLAG 1024
334   -#define ATTR_KILL_SUID 2048
335   -#define ATTR_KILL_SGID 4096
336   -#define ATTR_FILE 8192
337   -#define ATTR_KILL_PRIV 16384
338   -#define ATTR_OPEN 32768 /* Truncating from open(O_TRUNC) */
  323 +#define ATTR_MODE (1 << 0)
  324 +#define ATTR_UID (1 << 1)
  325 +#define ATTR_GID (1 << 2)
  326 +#define ATTR_SIZE (1 << 3)
  327 +#define ATTR_ATIME (1 << 4)
  328 +#define ATTR_MTIME (1 << 5)
  329 +#define ATTR_CTIME (1 << 6)
  330 +#define ATTR_ATIME_SET (1 << 7)
  331 +#define ATTR_MTIME_SET (1 << 8)
  332 +#define ATTR_FORCE (1 << 9) /* Not a change, but a change it */
  333 +#define ATTR_ATTR_FLAG (1 << 10)
  334 +#define ATTR_KILL_SUID (1 << 11)
  335 +#define ATTR_KILL_SGID (1 << 12)
  336 +#define ATTR_FILE (1 << 13)
  337 +#define ATTR_KILL_PRIV (1 << 14)
  338 +#define ATTR_OPEN (1 << 15) /* Truncating from open(O_TRUNC) */
  339 +#define ATTR_TIMES_SET (1 << 16)
339 340  
340 341 /*
341 342 * This is the Inode Attributes structure, used for notify_change(). It