Re: Minor optimizations in open.

Eric Brunet (ebrunet@clipper.ens.fr)
Thu, 29 Jul 1999 18:15:21 +0200

On Thu, Jul 29, 1999 at 08:24:08AM -0400, Gregory Maxwell wrote:
>
> Thanks for this patch, but in the future could you please only post
> patches created with diff -u? Thanks!

Here it is at the end of this mail. (Slightly different, 100% equivalent)

Today, looking at the sources, I have noticed something strange:
In open.c, there is:
/* If times==NULL, set access and modification to current time,
* must be owner or have write permission.
* Else, update from *times, must be owner or super user.
*/
asmlinkage int sys_utime(char * filename, struct utimbuf * times)
{
[....]
/* Don't worry, the checks are done in inode_change_ok() */
newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
if (times) {
error = get_user(newattrs.ia_atime, &times->actime);
if (!error)
error = get_user(newattrs.ia_mtime, &times->modtime);
if (error)
goto dput_and_out;

newattrs.ia_valid |= ATTR_ATIME_SET | ATTR_MTIME_SET;
} else {
if (current->fsuid != inode->i_uid &&
(error = permission(inode,MAY_WRITE)) != 0)
goto dput_and_out;
}
error = notify_change(dentry, &newattrs);
[...]
}

What interests me is the ``else'' part: if I am not the owner and if I
can't write to the file, deny permission. So far so good, as announced in
the comment. The problem is that notify_change calls inode_change_ok
where permissions are rechecked: extract from attr.c
int inode_change_ok(struct inode *inode, struct iattr *attr)
{
[...]
/* Check for setting the inode time. */
if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET)) {
if (current->fsuid != inode->i_uid && !capable(CAP_FOWNER))
goto error;
}
There, I raise an error if I am not the owner of the file, whether I have
write permissions or not.

So can I touch a file I don't own if I have write permissions on it ? If
yes, the code is incorrect, if no, the comment in the source is incorrect.

Éric Brunet

--- linux-2.3.11/fs/namei.c.orig Wed Jul 28 21:54:10 1999
+++ linux-2.3.11/fs/namei.c Thu Jul 29 17:49:03 1999
@@ -686,8 +686,10 @@
struct dentry *dir;

if (dentry->d_inode) {
- if (!(flag & O_EXCL))
+ if (!(flag & O_EXCL)) {
+ inode = dentry->d_inode;
goto nocreate;
+ }
error = -EEXIST;
goto exit;
}
@@ -728,12 +730,12 @@
goto exit;
}

-nocreate:
error = -ENOENT;
inode = dentry->d_inode;
if (!inode)
goto exit;

+nocreate:
error = -ELOOP;
if (S_ISLNK(inode->i_mode))
goto exit;
@@ -747,6 +749,8 @@
goto exit;

/*
+ * permission() has already tested that we are not trying to
+ * write to a regular file on a read-only filesystem.
* FIFO's, sockets and device files are special: they don't
* actually live on the filesystem itself, and as such you
* can write to them even if the filesystem is read-only.
@@ -759,11 +763,8 @@
goto exit;

flag &= ~O_TRUNC;
- } else {
- error = -EROFS;
- if (IS_RDONLY(inode) && (flag & 2))
- goto exit;
}
+
/*
* An append-only file must be opened in append mode for writing.
*/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/