From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Kerrisk <mtk.manpages@googlemail.com>
Subject: [parch 4/4] vfs: utimensat(): fix write access check for futimens()
Date: Wed, 04 Jun 2008 00:25:22 +0200
Message-ID: <4845C4D2.8050408@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
To: Andrew Morton <akpm@linux-foundation.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Al Viro <viro@zeniv.linux.org.uk>,
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mu-out-0910.google.com ([209.85.134.184]:19854 "EHLO
	mu-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1758806AbYFCWZx (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Tue, 3 Jun 2008 18:25:53 -0400
Received: by mu-out-0910.google.com with SMTP id w8so1701663mue.1
        for <linux-fsdevel@vger.kernel.org>; Tue, 03 Jun 2008 15:25:52 -0700 (PDT)
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

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);