[patch for 2.6.26 3/4] vfs: utimensat(): fix error checking for {UTIME_NOW,UTIME_OMIT} case (4cca9226) · Commits · 戴 / test

The POSIX.1 draft spec for utimensat() says: Only a process with the effective user ID equal to the user ID of the file or with appropriate privileges may use futimens() or utimensat() with a non-null times argument that does not have both tv_nsec fields set to UTIME_NOW and does not have both tv_nsec fields set to UTIME_OMIT. If this condition is violated, then the error EPERM should result. However, the current implementation does not generate EPERM if one tv_nsec field is UTIME_NOW while the other is UTIME_OMIT. It should give this error for that case. This patch: a) Repairs that problem. b) Removes the now unneeded nsec_special() helper function. c) Adds some comments to explain the checks that are being performed. Thanks to Miklos, who provided comments on the previous iteration of this patch. As a result, this version is a little simpler and and its logic is better structured. Miklos suggested an alternative idea, migrating the is_owner_or_cap() checks into fs/attr.c:inode_change_ok() via the use of an ATTR_OWNER_CHECK flag. Maybe we could do that later, but for now I've gone with this version, which is IMO simpler, and can be more easily read as being correct. Acked-by:

Miklos Szeredi <miklos@szeredi.hu> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Ulrich Drepper <drepper@redhat.com> Signed-off-by:

Michael Kerrisk <mtk.manpages@gmail.com> Signed-off-by:

Andrew Morton <akpm@linux-foundation.org> Signed-off-by:

Al Viro <viro@zeniv.linux.org.uk>

fs/utimes.c

+21 −15

Original line number	Diff line number	Diff line
		@@ -40,14 +40,9 @@ asmlinkage long sys_utime(char __user filename, struct utimbuf __user times)

		#endif

		static bool nsec_special(long nsec)
		{
		return nsec == UTIME_OMIT \|\| nsec == UTIME_NOW;
		}

		static bool nsec_valid(long nsec)
		{
		if (nsec_special(nsec))
		if (nsec == UTIME_OMIT \|\| nsec == UTIME_NOW)
		return true;

		return nsec >= 0 && nsec <= 999999999;
		@@ -106,7 +101,7 @@ long do_utimes(int dfd, char __user filename, struct timespec times, int flags
		times[1].tv_nsec == UTIME_NOW)
		times = NULL;

		/* Don't worry, the checks are done in inode_change_ok() */
		/* In most cases, the checks are done in inode_change_ok() */
		newattrs.ia_valid = ATTR_CTIME \| ATTR_MTIME \| ATTR_ATIME;
		if (times) {
		error = -EPERM;
		@@ -128,15 +123,26 @@ long do_utimes(int dfd, char __user filename, struct timespec times, int flags
		newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
		newattrs.ia_valid \|= ATTR_MTIME_SET;
		}
		}

		/*
		* If times is NULL or both times are either UTIME_OMIT or
		* UTIME_NOW, then need to check permissions, because
		* For the UTIME_OMIT/UTIME_NOW and UTIME_NOW/UTIME_OMIT
		* cases, we need to make an extra check that is not done by
		* inode_change_ok().
		*/
		if (((times[0].tv_nsec == UTIME_NOW &&
		times[1].tv_nsec == UTIME_OMIT)
		\|\|
		(times[0].tv_nsec == UTIME_OMIT &&
		times[1].tv_nsec == UTIME_NOW))
		&& !is_owner_or_cap(inode))
		goto mnt_drop_write_and_out;
		} else {

		/*
		* If times is NULL (or both times are UTIME_NOW),
		* then we need to check permissions, because
		* inode_change_ok() won't do it.
		*/
		if (!times \|\| (nsec_special(times[0].tv_nsec) &&
		nsec_special(times[1].tv_nsec))) {
		error = -EACCES;
		if (IS_IMMUTABLE(inode))
		goto mnt_drop_write_and_out;

Admin message