* [patch 0/4] vfs: utimes cleanups
@ 2008-07-01 13:01 Miklos Szeredi
2008-07-01 13:01 ` [patch 1/4] vfs: utimes: move owner check into inode_change_ok() Miklos Szeredi
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Miklos Szeredi @ 2008-07-01 13:01 UTC (permalink / raw)
To: viro; +Cc: linux-kernel, linux-fsdevel, hch, akpm
Al,
Could you please take these (mostly) utimes cleanups for 2.6.27?
Thanks,
Miklos
--
^ permalink raw reply [flat|nested] 7+ messages in thread
* [patch 1/4] vfs: utimes: move owner check into inode_change_ok()
2008-07-01 13:01 [patch 0/4] vfs: utimes cleanups Miklos Szeredi
@ 2008-07-01 13:01 ` Miklos Szeredi
2008-07-01 14:09 ` Michael Kerrisk
2008-07-01 13:01 ` [patch 2/4] vfs: utimes cleanup Miklos Szeredi
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Miklos Szeredi @ 2008-07-01 13:01 UTC (permalink / raw)
To: viro
Cc: linux-kernel, linux-fsdevel, hch, akpm, Ulrich Drepper, Michael Kerrisk
[-- Attachment #1: vfs-utimes-move-owner-check-into-inode_change_ok.patch --]
[-- Type: text/plain, Size: 4411 bytes --]
From: Miklos Szeredi <mszeredi@suse.cz>
Add a new ia_valid flag: ATTR_TIMES_SET, to handle the
UTIMES_OMIT/UTIMES_NOW and UTIMES_NOW/UTIMES_OMIT cases. In these
cases neither ATTR_MTIME_SET nor ATTR_ATIME_SET is in the flags, yet
the POSIX draft specifies that permission checking is performed the
same way as if one or both of the times was explicitly set to a
timestamp.
See the path "vfs: utimensat(): fix error checking for
{UTIME_NOW,UTIME_OMIT} case" by Michael Kerrisk for the patch
introducing this behavior.
This is a cleanup, as well as allowing filesystems (NFS/fuse/...) to
perform their own permission checking instead of the default.
CC: Ulrich Drepper <drepper@redhat.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/attr.c | 2 +-
fs/utimes.c | 17 ++++-------------
include/linux/fs.h | 33 +++++++++++++++++----------------
3 files changed, 22 insertions(+), 30 deletions(-)
Index: linux-2.6/fs/attr.c
===================================================================
--- linux-2.6.orig/fs/attr.c 2008-06-27 22:09:08.000000000 +0200
+++ linux-2.6/fs/attr.c 2008-07-01 13:52:20.000000000 +0200
@@ -51,7 +51,7 @@ int inode_change_ok(struct inode *inode,
}
/* Check for setting the inode time. */
- if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET)) {
+ if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)) {
if (!is_owner_or_cap(inode))
goto error;
}
Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c 2008-07-01 08:10:12.000000000 +0200
+++ linux-2.6/fs/utimes.c 2008-07-01 13:52:20.000000000 +0200
@@ -101,7 +101,6 @@ long do_utimes(int dfd, char __user *fil
times[1].tv_nsec == UTIME_NOW)
times = NULL;
- /* In most cases, the checks are done in inode_change_ok() */
newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
if (times) {
error = -EPERM;
@@ -123,21 +122,13 @@ long do_utimes(int dfd, char __user *fil
newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
newattrs.ia_valid |= ATTR_MTIME_SET;
}
-
/*
- * For the UTIME_OMIT/UTIME_NOW and UTIME_NOW/UTIME_OMIT
- * cases, we need to make an extra check that is not done by
- * inode_change_ok().
+ * Tell inode_change_ok(), that this is an explicit time
+ * update, even if neither ATTR_ATIME_SET nor ATTR_MTIME_SET
+ * were used.
*/
- if (((times[0].tv_nsec == UTIME_NOW &&
- times[1].tv_nsec == UTIME_OMIT)
- ||
- (times[0].tv_nsec == UTIME_OMIT &&
- times[1].tv_nsec == UTIME_NOW))
- && !is_owner_or_cap(inode))
- goto mnt_drop_write_and_out;
+ newattrs.ia_valid |= ATTR_TIMES_SET;
} else {
-
/*
* If times is NULL (or both times are UTIME_NOW),
* then we need to check permissions, because
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h 2008-07-01 13:52:19.000000000 +0200
+++ linux-2.6/include/linux/fs.h 2008-07-01 13:52:20.000000000 +0200
@@ -317,22 +317,23 @@ typedef void (dio_iodone_t)(struct kiocb
* Attribute flags. These should be or-ed together to figure out what
* has been changed!
*/
-#define ATTR_MODE 1
-#define ATTR_UID 2
-#define ATTR_GID 4
-#define ATTR_SIZE 8
-#define ATTR_ATIME 16
-#define ATTR_MTIME 32
-#define ATTR_CTIME 64
-#define ATTR_ATIME_SET 128
-#define ATTR_MTIME_SET 256
-#define ATTR_FORCE 512 /* Not a change, but a change it */
-#define ATTR_ATTR_FLAG 1024
-#define ATTR_KILL_SUID 2048
-#define ATTR_KILL_SGID 4096
-#define ATTR_FILE 8192
-#define ATTR_KILL_PRIV 16384
-#define ATTR_OPEN 32768 /* Truncating from open(O_TRUNC) */
+#define ATTR_MODE (1 << 0)
+#define ATTR_UID (1 << 1)
+#define ATTR_GID (1 << 2)
+#define ATTR_SIZE (1 << 3)
+#define ATTR_ATIME (1 << 4)
+#define ATTR_MTIME (1 << 5)
+#define ATTR_CTIME (1 << 6)
+#define ATTR_ATIME_SET (1 << 7)
+#define ATTR_MTIME_SET (1 << 8)
+#define ATTR_FORCE (1 << 9) /* Not a change, but a change it */
+#define ATTR_ATTR_FLAG (1 << 10)
+#define ATTR_KILL_SUID (1 << 11)
+#define ATTR_KILL_SGID (1 << 12)
+#define ATTR_FILE (1 << 13)
+#define ATTR_KILL_PRIV (1 << 14)
+#define ATTR_OPEN (1 << 15) /* Truncating from open(O_TRUNC) */
+#define ATTR_TIMES_SET (1 << 16)
/*
* This is the Inode Attributes structure, used for notify_change(). It
--
^ permalink raw reply [flat|nested] 7+ messages in thread
* [patch 2/4] vfs: utimes cleanup
2008-07-01 13:01 [patch 0/4] vfs: utimes cleanups Miklos Szeredi
2008-07-01 13:01 ` [patch 1/4] vfs: utimes: move owner check into inode_change_ok() Miklos Szeredi
@ 2008-07-01 13:01 ` Miklos Szeredi
2008-07-01 13:01 ` [patch 3/4] fat: dont call notify_change Miklos Szeredi
2008-07-01 13:01 ` [patch 4/4] vfs: immutable inode checking cleanup Miklos Szeredi
3 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2008-07-01 13:01 UTC (permalink / raw)
To: viro
Cc: linux-kernel, linux-fsdevel, hch, akpm, Ulrich Drepper, Michael Kerrisk
[-- Attachment #1: vfs-utimes-cleanup.patch --]
[-- Type: text/plain, Size: 3905 bytes --]
From: Miklos Szeredi <mszeredi@suse.cz>
Untange the mess that is do_utimes(). Add kerneldoc comment to
do_utimes().
CC: Ulrich Drepper <drepper@redhat.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/utimes.c | 114 ++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 65 insertions(+), 49 deletions(-)
Index: linux/fs/utimes.c
===================================================================
--- linux.orig/fs/utimes.c 2008-06-13 11:35:24.000000000 +0200
+++ linux/fs/utimes.c 2008-06-13 11:49:37.000000000 +0200
@@ -48,54 +48,15 @@ static bool nsec_valid(long nsec)
return nsec >= 0 && nsec <= 999999999;
}
-/* 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.
- */
-long do_utimes(int dfd, char __user *filename, struct timespec *times, int flags)
+static int utimes_common(struct path *path, struct timespec *times)
{
int error;
- struct nameidata nd;
- struct dentry *dentry;
- struct inode *inode;
struct iattr newattrs;
- struct file *f = NULL;
- struct vfsmount *mnt;
-
- error = -EINVAL;
- if (times && (!nsec_valid(times[0].tv_nsec) ||
- !nsec_valid(times[1].tv_nsec))) {
- goto out;
- }
-
- if (flags & ~AT_SYMLINK_NOFOLLOW)
- goto out;
-
- if (filename == NULL && dfd != AT_FDCWD) {
- error = -EINVAL;
- if (flags & AT_SYMLINK_NOFOLLOW)
- goto out;
+ struct inode *inode = path->dentry->d_inode;
- error = -EBADF;
- f = fget(dfd);
- if (!f)
- goto out;
- dentry = f->f_path.dentry;
- mnt = f->f_path.mnt;
- } else {
- error = __user_walk_fd(dfd, filename, (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW, &nd);
- if (error)
- goto out;
-
- dentry = nd.path.dentry;
- mnt = nd.path.mnt;
- }
-
- inode = dentry->d_inode;
-
- error = mnt_want_write(mnt);
+ error = mnt_want_write(path->mnt);
if (error)
- goto dput_and_out;
+ goto out;
if (times && times[0].tv_nsec == UTIME_NOW &&
times[1].tv_nsec == UTIME_NOW)
@@ -145,15 +106,70 @@ long do_utimes(int dfd, char __user *fil
}
}
mutex_lock(&inode->i_mutex);
- error = notify_change(dentry, &newattrs);
+ error = notify_change(path->dentry, &newattrs);
mutex_unlock(&inode->i_mutex);
+
mnt_drop_write_and_out:
- mnt_drop_write(mnt);
-dput_and_out:
- if (f)
- fput(f);
- else
+ mnt_drop_write(path->mnt);
+out:
+ return error;
+}
+
+/*
+ * do_utimes - change times on filename or file descriptor
+ * @dfd: open file descriptor, -1 or AT_FDCWD
+ * @filename: path name or NULL
+ * @times: new times or NULL
+ * @flags: zero or more flags (only AT_SYMLINK_NOFOLLOW for the moment)
+ *
+ * If filename is NULL and dfd refers to an open file, then operate on
+ * the file. Otherwise look up filename, possibly using dfd as a
+ * starting point.
+ *
+ * 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.
+ */
+long do_utimes(int dfd, char __user *filename, struct timespec *times, int flags)
+{
+ int error = -EINVAL;
+
+ if (times && (!nsec_valid(times[0].tv_nsec) ||
+ !nsec_valid(times[1].tv_nsec))) {
+ goto out;
+ }
+
+ if (flags & ~AT_SYMLINK_NOFOLLOW)
+ goto out;
+
+ if (filename == NULL && dfd != AT_FDCWD) {
+ struct file *file;
+
+ if (flags & AT_SYMLINK_NOFOLLOW)
+ goto out;
+
+ file = fget(dfd);
+ error = -EBADF;
+ if (!file)
+ goto out;
+
+ error = utimes_common(&file->f_path, times);
+ fput(file);
+ } else {
+ struct nameidata nd;
+ int lookup_flags = 0;
+
+ if (!(flags & AT_SYMLINK_NOFOLLOW))
+ lookup_flags |= LOOKUP_FOLLOW;
+
+ error = __user_walk_fd(dfd, filename, lookup_flags, &nd);
+ if (error)
+ goto out;
+
+ error = utimes_common(&nd.path, times);
path_put(&nd.path);
+ }
+
out:
return error;
}
--
^ permalink raw reply [flat|nested] 7+ messages in thread
* [patch 3/4] fat: dont call notify_change
2008-07-01 13:01 [patch 0/4] vfs: utimes cleanups Miklos Szeredi
2008-07-01 13:01 ` [patch 1/4] vfs: utimes: move owner check into inode_change_ok() Miklos Szeredi
2008-07-01 13:01 ` [patch 2/4] vfs: utimes cleanup Miklos Szeredi
@ 2008-07-01 13:01 ` Miklos Szeredi
2008-07-01 13:01 ` [patch 4/4] vfs: immutable inode checking cleanup Miklos Szeredi
3 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2008-07-01 13:01 UTC (permalink / raw)
To: viro; +Cc: linux-kernel, linux-fsdevel, hch, akpm, OGAWA Hirofumi
[-- Attachment #1: fat-dont-call-notify_change.patch --]
[-- Type: text/plain, Size: 2816 bytes --]
From: Miklos Szeredi <mszeredi@suse.cz>
The FAT_IOCTL_SET_ATTRIBUTES ioctl() calls notify_change() to change
the file mode before changing the inode attributes. Replace with
explicit calls to security_inode_setattr(), fat_setattr() and
fsnotify_change().
This is equivalent to the original. The reason it is needed, is that
later in the series we move the immutable check into notify_change().
That would break the FAT_IOCTL_SET_ATTRIBUTES ioctl, as it needs to
perform the mode change regardless of the immutability of the file.
[Fix error if fat is built as a module. Thanks to OGAWA Hirofumi for
noticing.]
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
fs/fat/file.c | 15 ++++++++++++++-
security/security.c | 1 +
2 files changed, 15 insertions(+), 1 deletion(-)
Index: linux-2.6/fs/fat/file.c
===================================================================
--- linux-2.6.orig/fs/fat/file.c 2008-06-27 22:09:08.000000000 +0200
+++ linux-2.6/fs/fat/file.c 2008-07-01 13:52:26.000000000 +0200
@@ -16,6 +16,8 @@
#include <linux/writeback.h>
#include <linux/backing-dev.h>
#include <linux/blkdev.h>
+#include <linux/fsnotify.h>
+#include <linux/security.h>
int fat_generic_ioctl(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg)
@@ -65,6 +67,7 @@ int fat_generic_ioctl(struct inode *inod
/* Equivalent to a chmod() */
ia.ia_valid = ATTR_MODE | ATTR_CTIME;
+ ia.ia_ctime = current_fs_time(inode->i_sb);
if (is_dir) {
ia.ia_mode = MSDOS_MKMODE(attr,
S_IRWXUGO & ~sbi->options.fs_dmask)
@@ -91,11 +94,21 @@ int fat_generic_ioctl(struct inode *inod
}
}
+ /*
+ * The security check is questionable... We single
+ * out the RO attribute for checking by the security
+ * module, just because it maps to a file mode.
+ */
+ err = security_inode_setattr(filp->f_path.dentry, &ia);
+ if (err)
+ goto up;
+
/* This MUST be done before doing anything irreversible... */
- err = notify_change(filp->f_path.dentry, &ia);
+ err = fat_setattr(filp->f_path.dentry, &ia);
if (err)
goto up;
+ fsnotify_change(filp->f_path.dentry, ia.ia_valid);
if (sbi->options.sys_immutable) {
if (attr & ATTR_SYS)
inode->i_flags |= S_IMMUTABLE;
Index: linux-2.6/security/security.c
===================================================================
--- linux-2.6.orig/security/security.c 2008-06-27 22:09:08.000000000 +0200
+++ linux-2.6/security/security.c 2008-07-01 13:52:26.000000000 +0200
@@ -476,6 +476,7 @@ int security_inode_setattr(struct dentry
return 0;
return security_ops->inode_setattr(dentry, attr);
}
+EXPORT_SYMBOL_GPL(security_inode_setattr);
int security_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
{
--
^ permalink raw reply [flat|nested] 7+ messages in thread
* [patch 4/4] vfs: immutable inode checking cleanup
2008-07-01 13:01 [patch 0/4] vfs: utimes cleanups Miklos Szeredi
` (2 preceding siblings ...)
2008-07-01 13:01 ` [patch 3/4] fat: dont call notify_change Miklos Szeredi
@ 2008-07-01 13:01 ` Miklos Szeredi
3 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2008-07-01 13:01 UTC (permalink / raw)
To: viro
Cc: linux-kernel, linux-fsdevel, hch, akpm, Ulrich Drepper, Michael Kerrisk
[-- Attachment #1: vfs-immutable-inode-checking-cleanup.patch --]
[-- Type: text/plain, Size: 3952 bytes --]
From: Miklos Szeredi <mszeredi@suse.cz>
Move the immutable and append-only checks from chmod, chown and utimes
into notify_change(). Checks for immutable and append-only files are
always performed by the VFS and not by the filesystem (see
permission() and may_...() in namei.c), so these belong in
notify_change(), and not in inode_change_ok().
This should be completely equivalent.
CC: Ulrich Drepper <drepper@redhat.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/attr.c | 5 +++++
fs/open.c | 24 ++----------------------
fs/utimes.c | 4 ----
3 files changed, 7 insertions(+), 26 deletions(-)
Index: linux-2.6/fs/attr.c
===================================================================
--- linux-2.6.orig/fs/attr.c 2008-07-01 13:52:20.000000000 +0200
+++ linux-2.6/fs/attr.c 2008-07-01 13:52:29.000000000 +0200
@@ -108,6 +108,11 @@ int notify_change(struct dentry * dentry
struct timespec now;
unsigned int ia_valid = attr->ia_valid;
+ if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_TIMES_SET)) {
+ if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
+ return -EPERM;
+ }
+
now = current_fs_time(inode->i_sb);
attr->ia_ctime = now;
Index: linux-2.6/fs/open.c
===================================================================
--- linux-2.6.orig/fs/open.c 2008-07-01 13:52:17.000000000 +0200
+++ linux-2.6/fs/open.c 2008-07-01 13:52:29.000000000 +0200
@@ -598,9 +598,6 @@ asmlinkage long sys_fchmod(unsigned int
err = mnt_want_write(file->f_path.mnt);
if (err)
goto out_putf;
- err = -EPERM;
- if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
- goto out_drop_write;
mutex_lock(&inode->i_mutex);
if (mode == (mode_t) -1)
mode = inode->i_mode;
@@ -608,8 +605,6 @@ asmlinkage long sys_fchmod(unsigned int
newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
err = notify_change(dentry, &newattrs);
mutex_unlock(&inode->i_mutex);
-
-out_drop_write:
mnt_drop_write(file->f_path.mnt);
out_putf:
fput(file);
@@ -633,11 +628,6 @@ asmlinkage long sys_fchmodat(int dfd, co
error = mnt_want_write(nd.path.mnt);
if (error)
goto dput_and_out;
-
- error = -EPERM;
- if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
- goto out_drop_write;
-
mutex_lock(&inode->i_mutex);
if (mode == (mode_t) -1)
mode = inode->i_mode;
@@ -645,8 +635,6 @@ asmlinkage long sys_fchmodat(int dfd, co
newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
error = notify_change(nd.path.dentry, &newattrs);
mutex_unlock(&inode->i_mutex);
-
-out_drop_write:
mnt_drop_write(nd.path.mnt);
dput_and_out:
path_put(&nd.path);
@@ -661,18 +649,10 @@ asmlinkage long sys_chmod(const char __u
static int chown_common(struct dentry * dentry, uid_t user, gid_t group)
{
- struct inode * inode;
+ struct inode *inode = dentry->d_inode;
int error;
struct iattr newattrs;
- error = -ENOENT;
- if (!(inode = dentry->d_inode)) {
- printk(KERN_ERR "chown_common: NULL inode\n");
- goto out;
- }
- error = -EPERM;
- if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
- goto out;
newattrs.ia_valid = ATTR_CTIME;
if (user != (uid_t) -1) {
newattrs.ia_valid |= ATTR_UID;
@@ -688,7 +668,7 @@ static int chown_common(struct dentry *
mutex_lock(&inode->i_mutex);
error = notify_change(dentry, &newattrs);
mutex_unlock(&inode->i_mutex);
-out:
+
return error;
}
Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c 2008-07-01 13:52:23.000000000 +0200
+++ linux-2.6/fs/utimes.c 2008-07-01 13:52:29.000000000 +0200
@@ -64,10 +64,6 @@ static int utimes_common(struct path *pa
newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
if (times) {
- error = -EPERM;
- if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
- goto mnt_drop_write_and_out;
-
if (times[0].tv_nsec == UTIME_OMIT)
newattrs.ia_valid &= ~ATTR_ATIME;
else if (times[0].tv_nsec != UTIME_NOW) {
--
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch 1/4] vfs: utimes: move owner check into inode_change_ok()
2008-07-01 13:01 ` [patch 1/4] vfs: utimes: move owner check into inode_change_ok() Miklos Szeredi
@ 2008-07-01 14:09 ` Michael Kerrisk
2008-07-01 14:23 ` Miklos Szeredi
0 siblings, 1 reply; 7+ messages in thread
From: Michael Kerrisk @ 2008-07-01 14:09 UTC (permalink / raw)
To: Miklos Szeredi
Cc: viro, linux-kernel, linux-fsdevel, hch, akpm, Ulrich Drepper,
Michael Kerrisk
Hi Miklos,
On Tue, Jul 1, 2008 at 3:01 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
>
> Add a new ia_valid flag: ATTR_TIMES_SET, to handle the
> UTIMES_OMIT/UTIMES_NOW and UTIMES_NOW/UTIMES_OMIT cases. In these
> cases neither ATTR_MTIME_SET nor ATTR_ATIME_SET is in the flags, yet
> the POSIX draft specifies that permission checking is performed the
> same way as if one or both of the times was explicitly set to a
> timestamp.
>
> See the path "vfs: utimensat(): fix error checking for
> {UTIME_NOW,UTIME_OMIT} case" by Michael Kerrisk for the patch
> introducing this behavior.
>
> This is a cleanup, as well as allowing filesystems (NFS/fuse/...) to
> perform their own permission checking instead of the default.
What kernel version/tree is this patch against?
Cheers,
Michael
> CC: Ulrich Drepper <drepper@redhat.com>
> CC: Michael Kerrisk <mtk.manpages@gmail.com>
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> ---
> fs/attr.c | 2 +-
> fs/utimes.c | 17 ++++-------------
> include/linux/fs.h | 33 +++++++++++++++++----------------
> 3 files changed, 22 insertions(+), 30 deletions(-)
>
> Index: linux-2.6/fs/attr.c
> ===================================================================
> --- linux-2.6.orig/fs/attr.c 2008-06-27 22:09:08.000000000 +0200
> +++ linux-2.6/fs/attr.c 2008-07-01 13:52:20.000000000 +0200
> @@ -51,7 +51,7 @@ int inode_change_ok(struct inode *inode,
> }
>
> /* Check for setting the inode time. */
> - if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET)) {
> + if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)) {
> if (!is_owner_or_cap(inode))
> goto error;
> }
> Index: linux-2.6/fs/utimes.c
> ===================================================================
> --- linux-2.6.orig/fs/utimes.c 2008-07-01 08:10:12.000000000 +0200
> +++ linux-2.6/fs/utimes.c 2008-07-01 13:52:20.000000000 +0200
> @@ -101,7 +101,6 @@ long do_utimes(int dfd, char __user *fil
> times[1].tv_nsec == UTIME_NOW)
> times = NULL;
>
> - /* In most cases, the checks are done in inode_change_ok() */
> newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
> if (times) {
> error = -EPERM;
> @@ -123,21 +122,13 @@ long do_utimes(int dfd, char __user *fil
> newattrs.ia_mtime.tv_nsec = times[1].tv_nsec;
> newattrs.ia_valid |= ATTR_MTIME_SET;
> }
> -
> /*
> - * For the UTIME_OMIT/UTIME_NOW and UTIME_NOW/UTIME_OMIT
> - * cases, we need to make an extra check that is not done by
> - * inode_change_ok().
> + * Tell inode_change_ok(), that this is an explicit time
> + * update, even if neither ATTR_ATIME_SET nor ATTR_MTIME_SET
> + * were used.
> */
> - if (((times[0].tv_nsec == UTIME_NOW &&
> - times[1].tv_nsec == UTIME_OMIT)
> - ||
> - (times[0].tv_nsec == UTIME_OMIT &&
> - times[1].tv_nsec == UTIME_NOW))
> - && !is_owner_or_cap(inode))
> - goto mnt_drop_write_and_out;
> + newattrs.ia_valid |= ATTR_TIMES_SET;
> } else {
> -
> /*
> * If times is NULL (or both times are UTIME_NOW),
> * then we need to check permissions, because
> Index: linux-2.6/include/linux/fs.h
> ===================================================================
> --- linux-2.6.orig/include/linux/fs.h 2008-07-01 13:52:19.000000000 +0200
> +++ linux-2.6/include/linux/fs.h 2008-07-01 13:52:20.000000000 +0200
> @@ -317,22 +317,23 @@ typedef void (dio_iodone_t)(struct kiocb
> * Attribute flags. These should be or-ed together to figure out what
> * has been changed!
> */
> -#define ATTR_MODE 1
> -#define ATTR_UID 2
> -#define ATTR_GID 4
> -#define ATTR_SIZE 8
> -#define ATTR_ATIME 16
> -#define ATTR_MTIME 32
> -#define ATTR_CTIME 64
> -#define ATTR_ATIME_SET 128
> -#define ATTR_MTIME_SET 256
> -#define ATTR_FORCE 512 /* Not a change, but a change it */
> -#define ATTR_ATTR_FLAG 1024
> -#define ATTR_KILL_SUID 2048
> -#define ATTR_KILL_SGID 4096
> -#define ATTR_FILE 8192
> -#define ATTR_KILL_PRIV 16384
> -#define ATTR_OPEN 32768 /* Truncating from open(O_TRUNC) */
> +#define ATTR_MODE (1 << 0)
> +#define ATTR_UID (1 << 1)
> +#define ATTR_GID (1 << 2)
> +#define ATTR_SIZE (1 << 3)
> +#define ATTR_ATIME (1 << 4)
> +#define ATTR_MTIME (1 << 5)
> +#define ATTR_CTIME (1 << 6)
> +#define ATTR_ATIME_SET (1 << 7)
> +#define ATTR_MTIME_SET (1 << 8)
> +#define ATTR_FORCE (1 << 9) /* Not a change, but a change it */
> +#define ATTR_ATTR_FLAG (1 << 10)
> +#define ATTR_KILL_SUID (1 << 11)
> +#define ATTR_KILL_SGID (1 << 12)
> +#define ATTR_FILE (1 << 13)
> +#define ATTR_KILL_PRIV (1 << 14)
> +#define ATTR_OPEN (1 << 15) /* Truncating from open(O_TRUNC) */
> +#define ATTR_TIMES_SET (1 << 16)
>
> /*
> * This is the Inode Attributes structure, used for notify_change(). It
>
> --
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch 1/4] vfs: utimes: move owner check into inode_change_ok()
2008-07-01 14:09 ` Michael Kerrisk
@ 2008-07-01 14:23 ` Miklos Szeredi
0 siblings, 0 replies; 7+ messages in thread
From: Miklos Szeredi @ 2008-07-01 14:23 UTC (permalink / raw)
To: mtk.manpages
Cc: miklos, viro, linux-kernel, linux-fsdevel, hch, akpm, drepper,
mtk.manpages
Hi Michael,
On Tue, 1 Jul 2008, Michael Kerrisk wrote:
> On Tue, Jul 1, 2008 at 3:01 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > From: Miklos Szeredi <mszeredi@suse.cz>
> >
> > Add a new ia_valid flag: ATTR_TIMES_SET, to handle the
> > UTIMES_OMIT/UTIMES_NOW and UTIMES_NOW/UTIMES_OMIT cases. In these
> > cases neither ATTR_MTIME_SET nor ATTR_ATIME_SET is in the flags, yet
> > the POSIX draft specifies that permission checking is performed the
> > same way as if one or both of the times was explicitly set to a
> > timestamp.
> >
> > See the path "vfs: utimensat(): fix error checking for
> > {UTIME_NOW,UTIME_OMIT} case" by Michael Kerrisk for the patch
> > introducing this behavior.
> >
> > This is a cleanup, as well as allowing filesystems (NFS/fuse/...) to
> > perform their own permission checking instead of the default.
>
> What kernel version/tree is this patch against?
Against latest git. 2.6.26-rc8-git2 seems to be recent enough.
Miklos
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-07-01 14:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-07-01 13:01 [patch 0/4] vfs: utimes cleanups Miklos Szeredi
2008-07-01 13:01 ` [patch 1/4] vfs: utimes: move owner check into inode_change_ok() Miklos Szeredi
2008-07-01 14:09 ` Michael Kerrisk
2008-07-01 14:23 ` Miklos Szeredi
2008-07-01 13:01 ` [patch 2/4] vfs: utimes cleanup Miklos Szeredi
2008-07-01 13:01 ` [patch 3/4] fat: dont call notify_change Miklos Szeredi
2008-07-01 13:01 ` [patch 4/4] vfs: immutable inode checking cleanup Miklos Szeredi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®