The POSIX.1 draft spec for futimens()/utimensat() 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 a library
function 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.
Thanks to Miklos for pointing out how to do this check.
CC: Miklos Szeredi <[email protected]>
CC: Al Viro <[email protected]>
CC: Ulrich Drepper <[email protected]>
Signed-off-by: Michael Kerrisk <[email protected]>
--- linux-2.6.26-rc4/fs/utimes.c 2008-06-03 23:13:31.000000000 +0200
+++ linux-2.6.26-rc4-utimensat-fix-v4/fs/utimes.c 2008-06-03 23:15:12.000000000 +0200
@@ -137,7 +137,8 @@
if (!is_owner_or_cap(inode)) {
if (f) {
- if (!(f->f_mode & FMODE_WRITE))
+ error = permission(inode, MAY_WRITE, NULL);
+ if (error)
goto mnt_drop_write_and_out;
} else {
error = vfs_permission(&nd, MAY_WRITE);
> The POSIX.1 draft spec for futimens()/utimensat() 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 a library
> function 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.
>
> Thanks to Miklos for pointing out how to do this check.
>
> CC: Miklos Szeredi <[email protected]>
> CC: Al Viro <[email protected]>
> CC: Ulrich Drepper <[email protected]>
> Signed-off-by: Michael Kerrisk <[email protected]>
>
> --- linux-2.6.26-rc4/fs/utimes.c 2008-06-03 23:13:31.000000000 +0200
> +++ linux-2.6.26-rc4-utimensat-fix-v4/fs/utimes.c 2008-06-03 23:15:12.000000000 +0200
> @@ -137,7 +137,8 @@
>
> if (!is_owner_or_cap(inode)) {
> if (f) {
> - if (!(f->f_mode & FMODE_WRITE))
> + error = permission(inode, MAY_WRITE, NULL);
> + if (error)
> goto mnt_drop_write_and_out;
> } else {
> error = vfs_permission(&nd, MAY_WRITE);
At which point the "if (f)" and the "else" branches become equivalent
(the nameidata isn't interesting in the other case either). So that
could be written as:
if (!is_owner_or_cap(inode)) {
error = permission(inode, MAY_WRITE, NULL);
if (error)
goto mnt_drop_write_and_out;
}
Miklos
> At which point the "if (f)" and the "else" branches become equivalent
> (the nameidata isn't interesting in the other case either). So that
> could be written as:
>
> if (!is_owner_or_cap(inode)) {
> error = permission(inode, MAY_WRITE, NULL);
> if (error)
> goto mnt_drop_write_and_out;
> }
And also the IS_IMMUTABLE() check can be removed, since it's checked
by permission(MAY_WRITE) anyway.
Miklos
On Wed, Jun 4, 2008 at 6:41 AM, Miklos Szeredi <[email protected]> wrote:
>> The POSIX.1 draft spec for futimens()/utimensat() 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 a library
>> function 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.
>>
>> Thanks to Miklos for pointing out how to do this check.
>>
>> CC: Miklos Szeredi <[email protected]>
>> CC: Al Viro <[email protected]>
>> CC: Ulrich Drepper <[email protected]>
>> Signed-off-by: Michael Kerrisk <[email protected]>
>>
>> --- linux-2.6.26-rc4/fs/utimes.c 2008-06-03 23:13:31.000000000 +0200
>> +++ linux-2.6.26-rc4-utimensat-fix-v4/fs/utimes.c 2008-06-03 23:15:12.000000000 +0200
>> @@ -137,7 +137,8 @@
>>
>> if (!is_owner_or_cap(inode)) {
>> if (f) {
>> - if (!(f->f_mode & FMODE_WRITE))
>> + error = permission(inode, MAY_WRITE, NULL);
>> + if (error)
>> goto mnt_drop_write_and_out;
>> } else {
>> error = vfs_permission(&nd, MAY_WRITE);
>
> At which point the "if (f)" and the "else" branches become equivalent
> (the nameidata isn't interesting in the other case either). So that
> could be written as:
>
> if (!is_owner_or_cap(inode)) {
> error = permission(inode, MAY_WRITE, NULL);
> if (error)
> goto mnt_drop_write_and_out;
> }
Okay -- thanks Miklos. I'll change that, and test.
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
On Wed, Jun 4, 2008 at 7:54 AM, Miklos Szeredi <[email protected]> wrote:
>> At which point the "if (f)" and the "else" branches become equivalent
>> (the nameidata isn't interesting in the other case either). So that
>> could be written as:
>>
>> if (!is_owner_or_cap(inode)) {
>> error = permission(inode, MAY_WRITE, NULL);
>> if (error)
>> goto mnt_drop_write_and_out;
>> }
>
> And also the IS_IMMUTABLE() check can be removed, since it's checked
> by permission(MAY_WRITE) anyway.
I'm not sure that that is true, because immutability applies
regardless of capabilities or ownership, right?
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
> On Wed, Jun 4, 2008 at 7:54 AM, Miklos Szeredi <[email protected]> wrote:
> >> At which point the "if (f)" and the "else" branches become equivalent
> >> (the nameidata isn't interesting in the other case either). So that
> >> could be written as:
> >>
> >> if (!is_owner_or_cap(inode)) {
> >> error = permission(inode, MAY_WRITE, NULL);
> >> if (error)
> >> goto mnt_drop_write_and_out;
> >> }
> >
> > And also the IS_IMMUTABLE() check can be removed, since it's checked
> > by permission(MAY_WRITE) anyway.
>
> I'm not sure that that is true, because immutability applies
> regardless of capabilities or ownership, right?
Yeah, you're right. I misread that.
Miklos