lkml.org 
[lkml]   [2008]   [Jun]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [parch 4/4] vfs: utimensat(): fix write access check for futimens()
    On Wed, Jun 4, 2008 at 6:41 AM, Miklos Szeredi <miklos@szeredi.hu> 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 <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>
    >>
    >> --- 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


    \
     
     \ /
      Last update: 2008-06-04 10:43    [W:0.028 / U:0.048 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site