Re: [PATCH] utimensat() non-conformances and fixes -- version 2
From: Miklos Szeredi
Date: Mon May 19 2008 - 06:13:12 EST
> It's not only fragile, it's ugly as sin. I'd rather do it this way:
>
> - initialize error to zero
> - if no write access then set error, and the ATTR_TIMES_UPDATE(*) flag
> - set error2 from result of notify_change()
> - if error is zero then return error2, otherwise return error
Something like this (haven't thought it through, totally untested,
etc...)
Miklos
---
fs/utimes.c | 48 ++++++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 26 deletions(-)
Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c 2008-05-17 08:50:01.000000000 +0200
+++ linux-2.6/fs/utimes.c 2008-05-19 12:08:18.000000000 +0200
@@ -53,7 +53,8 @@ static bool nsec_valid(long nsec)
return nsec >= 0 && nsec <= 999999999;
}
-static int utimes_common(struct path *path, struct timespec *times)
+static int utimes_common(struct path *path, struct timespec *times,
+ int write_error)
{
int error;
struct iattr newattrs;
@@ -76,11 +77,18 @@ static int utimes_common(struct path *pa
newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
newattrs.ia_valid |= ATTR_MTIME_SET;
}
+ newattrs.ia_valid |= ATTR_OWNER_CHECK;
+ } else if (write_error) {
+ newattrs.ia_valid |= ATTR_OWNER_CHECK;
}
+
mutex_lock(&path->dentry->d_inode->i_mutex);
error = path_setattr(path, &newattrs);
mutex_unlock(&path->dentry->d_inode->i_mutex);
+ if (write_error && error)
+ error = write_error;
+
return error;
}
@@ -97,21 +105,16 @@ static bool utimes_need_permission(struc
static int do_futimes(int fd, struct timespec *times)
{
int error;
+ int write_error = 0;
struct file *file = fget(fd);
if (!file)
return -EBADF;
- if (utimes_need_permission(times)) {
- struct inode *inode = file->f_path.dentry->d_inode;
+ if (!times && !(file->f_mode & FMODE_WRITE))
+ write_error = -EACCES;
- error = -EACCES;
- if (!is_owner_or_cap(inode) && !(file->f_mode & FMODE_WRITE))
- goto out_fput;
- }
- error = utimes_common(&file->f_path, times);
-
- out_fput:
+ error = utimes_common(&file->f_path, times, write_error);
fput(file);
return error;
@@ -121,6 +124,7 @@ static int do_utimes_name(int dfd, char
struct timespec *times, int flags)
{
int error;
+ int write_error = 0;
struct nameidata nd;
int lookup_flags;
@@ -132,23 +136,10 @@ static int do_utimes_name(int dfd, char
if (error)
return error;
+ if (!times)
+ write_error = vfs_permission(&nd, MAY_WRITE);
- if (utimes_need_permission(times)) {
- struct inode *inode = nd.path.dentry->d_inode;
-
- error = -EACCES;
- if (IS_IMMUTABLE(inode))
- goto out_path_put;
-
- if (!is_owner_or_cap(inode)) {
- error = vfs_permission(&nd, MAY_WRITE);
- if (error)
- goto out_path_put;
- }
- }
- error = utimes_common(&nd.path, times);
-
- out_path_put:
+ error = utimes_common(&nd.path, times, write_error);
path_put(&nd.path);
return error;
@@ -177,6 +168,11 @@ int do_utimes(int dfd, char __user *file
return -EINVAL;
}
+ if (times && times[0].tv_nsec == UTIME_NOW &&
+ times[1].tv_nsec == UTIME_NOW) {
+ times = NULL;
+ }
+
if (filename == NULL && dfd != AT_FDCWD) {
if (flags)
return -EINVAL;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/