Andrew,
This patch series is a revised version of the patch series
that I sent yesterday to fix various utimensat()
non-conformances. Patches 1 and 2 are unchanged (and were
Acked by Miklos); patches 3 and 4 are revised following
comments by Miklos.
As requested I've split the original patch
("utimensat() non-conformances and fixes [v4] (patch)";
http://article.gmane.org/gmane.linux.file-systems/24325 )
into four parts. Ideally, these should be applied for
2.6.26-rc, for the reasons outlined in my earlier mail
"utimensat() non-conformances and fixes [v4] (test results)",
http://article.gmane.org/gmane.linux.kernel/689411/ .
The four patches can be applied independently, except that
patch 3 needs patch 2 to be applied first.
I've run my test suite
(http://article.gmane.org/gmane.linux.file-systems/24327 )
against this version of the patches, and all tests pass.
Cheers,
Michael
PS Just for reference, I'll include my earlier "overview"
message here:
-------- Original Message --------
Subject: utimensat() non-conformances and fixes [v4] (overview)
Date: Tue, 03 Jun 2008 22:13:32 +0200
From: Michael Kerrisk <mtk.manpages@xxxxxxxxx>
To: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
CC: lkml <linux-kernel@xxxxxxxxxxxxxxx>, Christoph Hellwig <hch@xxxxxx>, Miklos Szeredi <miklos@xxxxxxxxxx>, Al Viro
<viro@xxxxxxxxxxxxxxxxxx>, jamie@xxxxxxxxxxxxx, Ulrich Drepper <drepper@xxxxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx,
Subrata Modak <subrata@xxxxxxxxxxxxxxxxxx>
Andrew,
A follow-on to this mail is a patch (against 2.6.26-rc4) for -mm. The
patch fixes several problems in the utimensat() system call.
I would like to see this patch get into mainline ASAP. If you don't want
to push it for .26, I can understand that, since we are late in the cycle.
Nevertheless, I will provide some arguments in favor of doing so, in a
follow-on mail. (I'll also provide a fairly comprehensive test suite, and
test results, which may make you feel fairly confident of the patch.)
==
In an earlier mail I described four problems with the utimensat()
implementation, and attempted a clumsy fix,
http://thread.gmane.org/gmane.linux.man/129
Miklos pointed out a number of problems in my attempted fix, and pushed a
fix into .26-rc for one of the problems (number 3 in the mail), which was a
security-related issue (http://thread.gmane.org/gmane.linux.kernel/678130).
He also gave me a couple of clues along the way about how to fix the
remaining problems.
In this mail, I will explain the remaining problems from the beginning.
(Although Miklos pushed a fix for one of the problems, there still remain
four problems, because I found another one in the meantime.)
==
While writing a man page for utimensat(2) and futimens(3), I found a few
bugs in the utimensat() system call (i.e., non-conformances with either
the specification in the draft POSIX.1-200x revision or traditional Linux
behavior).
int futimens(inf fd, const struct timespec times[2]);
int utimensat(int dirfd, const char *pathname,
const struct timespec times[2], int flags);
1. The POSIX.1 draft says that to do anything other than setting both
timestamps to a time other than the current time (i.e., times is
not NULL, and both tv_nsec fields are not UTIME_NOW and both
tv_nsec fields are not UTIME_OMIT), either:
a) the caller's effective user ID must match the file owner; or
b) the caller must have appropriate privileges.
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 TIME_OMIT -- it
should give this error for that case.
2. The POSIX.1 draft says:
Only a process with the effective user ID equal to the
user ID of the file, *or with write access to the file*,
or with appropriate privileges may use futimens() or
utimensat() with a null pointer as the times argument
or with both tv_nsec fields set to the special value
UTIME_NOW.
The important piece here is "with write access to the file", and
this matters for futimens(), which deals with an argument that
is a file descriptor referring to the file whose timestamps are
being updated, The standard is saying that the "writability"
check is based on the file permissions, not the access mode with
which the file is opened. (This behavior is consistent with the
semantics of FreeBSD's futimes().) However, Linux is currently
doing the latter -- futimens(fd, times) is implemented as
utimensat(fd, NULL, times, 0)
and within the utimensat() implementation we have the code:
f = fget(dfd); // dfd is 'fd'
...
if (f) {
if (!(f->f_mode & FMODE_WRITE))
goto mnt_drop_write_and_out;
The check should instead be based on the file permissions.
3. The POSIX.1 draft says that if a times[n].tv_nsec field is
UTIME_OMIT or UTIME_NOW, then the value in the corresponding tv_sec
field is ignored. However the current Linux implementation
requires the tv_sec value to be zero (or the EINVAL error results).
This requirement should be removed.
4. A further bug relates to traditional Linux behavior. Traditionally
(i.e., utime(2) and utimes(2)), the EPERM error could also occur if
the 'times' argument was non-NULL (i.e., we are setting the
timestamps to a value other than the current time) and the file is
marked as immutable or append-only. The current implementation
also returns this error if 'times' is non-NULL and the tv_nsec
fields are both UTIME_NOW. For consistency, the
(times != NULL && times[0].tv_nsec == UTIME_NOW &&
times[1].tv_nsec == UTIME_NOW)
case should be treated like the traditional utimes() case where
'times' is NULL. That is, the call should succeed for a file
marked append-only and should give the error EACCES if the file
is marked as immutable.
Cheers,
Michael
PS For completeness, here are the expected results from various cases of
calls to utime() and utimensat().
| utime[s]() | utimensat() 'times' argument
| NULL non- | NULL . non-NULL arg
| arg NULL | arg . 2*NOW 2*OMIT OMIT {x,y}
| arg | . +NOW
----------------+--------------+-------------------------------------
file owned by/ | | .
permissions | | .
| | .
caller 400 | succ succ | succ . succ succ succ succ
| | .
not-caller 644 | EACCES EPERM | EACCES . EACCES succ EPERM EPERM
| | .
not-caller 666 | succ EPERM | succ . succ succ EPERM EPERM
----------------+--------------+-------------------------------------
ext2 attributes | | .
append-only | succ EPERM | succ . succ succ EPERM EPERM
| | .
immutable | EACCES EPERM | EACCES . EACCES succ EPERM EPERM
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
