linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts
@ 2008-05-06  9:13 Miklos Szeredi
  2008-05-06  9:13 ` [patch 01/24] nfsd: clean up mnt_want_write calls Miklos Szeredi
                   ` (25 more replies)
  0 siblings, 26 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

Here it is again as a single series (without the ecryptfs one already
merged).

I've addressed Christoph's comments.

Thanks,
Miklos

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 01/24] nfsd: clean up mnt_want_write calls
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 02/24] cgroup: dont call vfs_mkdir Miklos Szeredi
                   ` (24 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: nfsd_create_cleanup.patch --]
[-- Type: text/plain, Size: 1982 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Multiple mnt_want_write() calls in the switch statement looks really
ugly.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/nfsd/vfs.c |   25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:32.000000000 +0200
@@ -1248,36 +1248,34 @@ nfsd_create(struct svc_rqst *rqstp, stru
 		iap->ia_mode = 0;
 	iap->ia_mode = (iap->ia_mode & S_IALLUGO) | type;
 
+	err = nfserr_inval;
+	if (!S_ISREG(type) && !S_ISDIR(type) && !special_file(type)) {
+		printk(KERN_WARNING "nfsd: bad file type %o in nfsd_create\n",
+		       type);
+		goto out;
+	}
+
+	host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
+	if (host_err)
+		goto out_nfserr;
+
 	/*
 	 * Get the dir op function pointer.
 	 */
 	err = 0;
 	switch (type) {
 	case S_IFREG:
-		host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
-		if (host_err)
-			goto out_nfserr;
 		host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
 		break;
 	case S_IFDIR:
-		host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
-		if (host_err)
-			goto out_nfserr;
 		host_err = vfs_mkdir(dirp, dchild, iap->ia_mode);
 		break;
 	case S_IFCHR:
 	case S_IFBLK:
 	case S_IFIFO:
 	case S_IFSOCK:
-		host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
-		if (host_err)
-			goto out_nfserr;
 		host_err = vfs_mknod(dirp, dchild, iap->ia_mode, rdev);
 		break;
-	default:
-	        printk("nfsd: bad file type %o in nfsd_create\n", type);
-		host_err = -EINVAL;
-		goto out_nfserr;
 	}
 	if (host_err < 0) {
 		mnt_drop_write(fhp->fh_export->ex_path.mnt);
@@ -1289,7 +1287,6 @@ nfsd_create(struct svc_rqst *rqstp, stru
 		write_inode_now(dchild->d_inode, 1);
 	}
 
-
 	err2 = nfsd_create_setattr(rqstp, resfhp, iap);
 	if (err2)
 		err = err2;

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 02/24] cgroup: dont call vfs_mkdir
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
  2008-05-06  9:13 ` [patch 01/24] nfsd: clean up mnt_want_write calls Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 03/24] reiserfs: dont call vfs_rmdir Miklos Szeredi
                   ` (23 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, Paul Menage

[-- Attachment #1: cgroup_mkdir_cleanup.patch --]
[-- Type: text/plain, Size: 1561 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

cgroup_clone() calls vfs_mkdir() to create a directory in the cgroup
filesystem.  Replace with explicit call to cgroup_mkdir() and
fsnotify_mkdir().

This is equivalent, except that the following functions are not called
before cgroup_mkdir():

 - may_create()
 - security_inode_mkdir()
 - DQUOT_INIT()

Permission to clone the cgroup has already been checked in
copy_namespaces() (requiring CAP_SYS_ADMIN).  Additional file system
related capability checks are inappropriate and confusing.

The quota check is unnecessary, as quotas don't make any sense for
this filesystem.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Paul Menage <menage@google.com>
---
 kernel/cgroup.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-2.6/kernel/cgroup.c
===================================================================
--- linux-2.6.orig/kernel/cgroup.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/kernel/cgroup.c	2008-05-06 11:04:32.000000000 +0200
@@ -25,6 +25,7 @@
 #include <linux/cgroup.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
+#include <linux/fsnotify.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/mm.h>
@@ -2928,7 +2929,9 @@ int cgroup_clone(struct task_struct *tsk
 	}
 
 	/* Create the cgroup directory, which also creates the cgroup */
-	ret = vfs_mkdir(inode, dentry, S_IFDIR | 0755);
+	ret = cgroup_mkdir(inode, dentry, S_IFDIR);
+	if (!ret)
+		fsnotify_mkdir(inode, dentry);
 	child = __d_cgrp(dentry);
 	dput(dentry);
 	if (ret) {

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 03/24] reiserfs: dont call vfs_rmdir
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
  2008-05-06  9:13 ` [patch 01/24] nfsd: clean up mnt_want_write calls Miklos Szeredi
  2008-05-06  9:13 ` [patch 02/24] cgroup: dont call vfs_mkdir Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 04/24] reiserfs: dont call notify_change Miklos Szeredi
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, Jeff Mahoney

[-- Attachment #1: reiserfs_rmdir_cleanup.patch --]
[-- Type: text/plain, Size: 3375 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

reiserfs_delete_xattrs() calls vfs_rmdir() to remove the private
directory associated with the extended attributes for an inode.
Replace with explicit call to reiserfs_rmdir().

The extended attribute files reside in a private directory, completely
inaccessible from userspace.  This means, that checking against mounts
on the dentry is not necessary.

The calls to may_delete() and security_inode_rmdir(), as performed by
vfs_rmdir() are also unecessary for this case.

Based on patch by Jeff Mahoney.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Acked-by: Jeff Mahoney <jeffm@suse.de>
---
 fs/reiserfs/namei.c         |    2 +-
 fs/reiserfs/xattr.c         |   22 +++++++++++++++++++++-
 include/linux/reiserfs_fs.h |    1 +
 3 files changed, 23 insertions(+), 2 deletions(-)

Index: linux-2.6/fs/reiserfs/xattr.c
===================================================================
--- linux-2.6.orig/fs/reiserfs/xattr.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/reiserfs/xattr.c	2008-05-06 11:04:33.000000000 +0200
@@ -35,6 +35,7 @@
 #include <linux/namei.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
+#include <linux/quotaops.h>
 #include <linux/file.h>
 #include <linux/pagemap.h>
 #include <linux/xattr.h>
@@ -712,6 +713,25 @@ reiserfs_delete_xattrs_filler(void *buf,
 
 }
 
+static int xattr_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	int error;
+
+	DQUOT_INIT(dir);
+
+	mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD);
+	dentry_unhash(dentry);
+	error = reiserfs_rmdir(dir, dentry);
+	if (!error)
+		dentry->d_inode->i_flags |= S_DEAD;
+	mutex_unlock(&dentry->d_inode->i_mutex);
+	if (!error)
+		d_delete(dentry);
+	dput(dentry);
+
+	return error;
+}
+
 /* This is called w/ inode->i_mutex downed */
 int reiserfs_delete_xattrs(struct inode *inode)
 {
@@ -746,7 +766,7 @@ int reiserfs_delete_xattrs(struct inode 
 	if (dir->d_inode->i_nlink <= 2) {
 		root = get_xa_root(inode->i_sb, XATTR_REPLACE);
 		reiserfs_write_lock_xattrs(inode->i_sb);
-		err = vfs_rmdir(root->d_inode, dir);
+		err = xattr_rmdir(root->d_inode, dir);
 		reiserfs_write_unlock_xattrs(inode->i_sb);
 		dput(root);
 	} else {
Index: linux-2.6/fs/reiserfs/namei.c
===================================================================
--- linux-2.6.orig/fs/reiserfs/namei.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/reiserfs/namei.c	2008-05-06 11:04:33.000000000 +0200
@@ -850,7 +850,7 @@ static inline int reiserfs_empty_dir(str
 	return 1;
 }
 
-static int reiserfs_rmdir(struct inode *dir, struct dentry *dentry)
+int reiserfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	int retval, err;
 	struct inode *inode;
Index: linux-2.6/include/linux/reiserfs_fs.h
===================================================================
--- linux-2.6.orig/include/linux/reiserfs_fs.h	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/include/linux/reiserfs_fs.h	2008-05-06 11:04:33.000000000 +0200
@@ -1911,6 +1911,7 @@ static inline void reiserfs_update_sd(st
 void sd_attrs_to_i_attrs(__u16 sd_attrs, struct inode *inode);
 void i_attrs_to_sd_attrs(struct inode *inode, __u16 * sd_attrs);
 int reiserfs_setattr(struct dentry *dentry, struct iattr *attr);
+int reiserfs_rmdir(struct inode *dir, struct dentry *dentry);
 
 /* namei.c */
 void set_de_name_and_namelen(struct reiserfs_dir_entry *de);

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 04/24] reiserfs: dont call notify_change
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (2 preceding siblings ...)
  2008-05-06  9:13 ` [patch 03/24] reiserfs: dont call vfs_rmdir Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-08 16:49   ` Christoph Hellwig
  2008-05-06  9:13 ` [patch 05/24] sysfs: " Miklos Szeredi
                   ` (21 subsequent siblings)
  25 siblings, 1 reply; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, Jeff Mahoney

[-- Attachment #1: reiserfs_notify_change_cleanup.patch --]
[-- Type: text/plain, Size: 2128 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Reiserfs calls notify_change() on it's private extended attribute
files to resize and change ownership of these files.  Replace with
reiserfs_setattr().

This is equivalent, except that:

 - i_alloc_sem locking is not performed around reiserfs_setattr() when
   the size is changed
 - security_inode_setattr() is not called before reiserfs_setattr()
 - fsnotify_change() is not called after reiserfs_setattr()

None of the above is necessary, since the files are private to
reiserfs and inaccessible from userspace.

Also remove setting of ctime on the xattr files.

Based on patch by Jeff Mahoney.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Acked-by: Jeff Mahoney <jeffm@suse.de>
---
 fs/reiserfs/xattr.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux-2.6/fs/reiserfs/xattr.c
===================================================================
--- linux-2.6.orig/fs/reiserfs/xattr.c	2008-05-06 11:04:33.000000000 +0200
+++ linux-2.6/fs/reiserfs/xattr.c	2008-05-06 11:04:33.000000000 +0200
@@ -458,9 +458,9 @@ reiserfs_xattr_set(struct inode *inode, 
 
 	/* Resize it so we're ok to write there */
 	newattrs.ia_size = buffer_size;
-	newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
+	newattrs.ia_valid = ATTR_SIZE;
 	mutex_lock_nested(&xinode->i_mutex, I_MUTEX_XATTR);
-	err = notify_change(dentry, &newattrs);
+	err = reiserfs_setattr(dentry, &newattrs);
 	if (err)
 		goto out_filp;
 
@@ -810,7 +810,7 @@ reiserfs_chown_xattrs_filler(void *buf, 
 	}
 
 	if (!S_ISDIR(xafile->d_inode->i_mode))
-		err = notify_change(xafile, attrs);
+		err = reiserfs_setattr(xafile, attrs);
 	dput(xafile);
 
 	return err;
@@ -843,7 +843,7 @@ int reiserfs_chown_xattrs(struct inode *
 
 	lock_kernel();
 
-	attrs->ia_valid &= (ATTR_UID | ATTR_GID | ATTR_CTIME);
+	attrs->ia_valid &= (ATTR_UID | ATTR_GID);
 	buf.xadir = dir;
 	buf.attrs = attrs;
 	buf.inode = inode;
@@ -854,7 +854,7 @@ int reiserfs_chown_xattrs(struct inode *
 		goto out_dir;
 	}
 
-	err = notify_change(dir, attrs);
+	err = reiserfs_setattr(dir, attrs);
 	unlock_kernel();
 
       out_dir:

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 05/24] sysfs: dont call notify_change
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (3 preceding siblings ...)
  2008-05-06  9:13 ` [patch 04/24] reiserfs: dont call notify_change Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-08 16:50   ` Christoph Hellwig
  2008-05-06  9:13 ` [patch 06/24] hpfs: " Miklos Szeredi
                   ` (20 subsequent siblings)
  25 siblings, 1 reply; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, Greg Kroah-Hartman

[-- Attachment #1: sysfs_notify_change_cleanup.patch --]
[-- Type: text/plain, Size: 1430 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

sysfs_chmod_file() calls notify_change() to change the permission bits
on a sysfs file.  Replace with explicit call to sysfs_setattr() and
fsnotify_change().

This is equivalent, except that security_inode_setattr() is not
called.  This function is called by drivers, so the security checks do
not make any sense.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 fs/sysfs/file.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-2.6/fs/sysfs/file.c
===================================================================
--- linux-2.6.orig/fs/sysfs/file.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/sysfs/file.c	2008-05-06 11:04:34.000000000 +0200
@@ -18,6 +18,7 @@
 #include <linux/poll.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
+#include <linux/fsnotify.h>
 #include <asm/uaccess.h>
 
 #include "sysfs.h"
@@ -585,9 +586,11 @@ int sysfs_chmod_file(struct kobject *kob
 
 	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
 	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
-	rc = notify_change(victim, &newattrs);
+	newattrs.ia_ctime = current_fs_time(inode->i_sb);
+	rc = sysfs_setattr(victim, &newattrs);
 
 	if (rc == 0) {
+		fsnotify_change(victim, newattrs.ia_valid);
 		mutex_lock(&sysfs_mutex);
 		victim_sd->s_mode = newattrs.ia_mode;
 		mutex_unlock(&sysfs_mutex);

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 06/24] hpfs: dont call notify_change
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (4 preceding siblings ...)
  2008-05-06  9:13 ` [patch 05/24] sysfs: " Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-08  0:41   ` Mikulas Patocka
  2008-05-08 16:52   ` Christoph Hellwig
  2008-05-06  9:13 ` [patch 07/24] fat: " Miklos Szeredi
                   ` (19 subsequent siblings)
  25 siblings, 2 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, Mikulas Patocka

[-- Attachment #1: hpfs_notify_change_cleanup.patch --]
[-- Type: text/plain, Size: 1291 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

hpfs_unlink() calls notify_change() to truncate the file before
deleting.  Replace with explicit call to hpfs_notify_change().

This is equivalent, except that:
 - security_inode_setattr() is not called before hpfs_notify_change()
 - fsnotify_change() is not called after hpfs_notify_change()

The truncation is just an implementation detail, so both the security
check and the notification are unnecessary.

Possibly even the ctime modification is wrong?

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
---
 fs/hpfs/namei.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6/fs/hpfs/namei.c
===================================================================
--- linux-2.6.orig/fs/hpfs/namei.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/hpfs/namei.c	2008-05-06 11:04:34.000000000 +0200
@@ -426,7 +426,8 @@ again:
 			/*printk("HPFS: truncating file before delete.\n");*/
 			newattrs.ia_size = 0;
 			newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
-			err = notify_change(dentry, &newattrs);
+			newattrs.ia_ctime = current_fs_time(inode->i_sb);
+			err = hpfs_notify_change(dentry, &newattrs);
 			put_write_access(inode);
 			if (!err)
 				goto again;

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 07/24] fat: dont call notify_change
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (5 preceding siblings ...)
  2008-05-06  9:13 ` [patch 06/24] hpfs: " Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-08 16:57   ` Christoph Hellwig
  2008-05-06  9:13 ` [patch 08/24] vfs: immutable inode checking cleanup Miklos Szeredi
                   ` (18 subsequent siblings)
  25 siblings, 1 reply; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, OGAWA Hirofumi

[-- Attachment #1: fat_notify_change_cleanup.patch --]
[-- Type: text/plain, Size: 1749 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 call to fat_setattr().

This is equivalent, except that security_inode_setattr() is not called
before fat_setattr().  I think this is not needed, since the mode
change is just a side effect of the attribute change.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
 fs/fat/file.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-2.6/fs/fat/file.c
===================================================================
--- linux-2.6.orig/fs/fat/file.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/fat/file.c	2008-05-06 11:04:34.000000000 +0200
@@ -16,6 +16,7 @@
 #include <linux/writeback.h>
 #include <linux/backing-dev.h>
 #include <linux/blkdev.h>
+#include <linux/fsnotify.h>
 
 int fat_generic_ioctl(struct inode *inode, struct file *filp,
 		      unsigned int cmd, unsigned long arg)
@@ -65,6 +66,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)
@@ -92,10 +94,11 @@ int fat_generic_ioctl(struct inode *inod
 		}
 
 		/* 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;

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 08/24] vfs: immutable inode checking cleanup
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (6 preceding siblings ...)
  2008-05-06  9:13 ` [patch 07/24] fat: " Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-15  7:23   ` Al Viro
  2008-05-06  9:13 ` [patch 09/24] vfs: truncate: dont check immutable twice Miklos Szeredi
                   ` (17 subsequent siblings)
  25 siblings, 1 reply; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: immutable_cleanup.patch --]
[-- Type: text/plain, Size: 3912 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().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/attr.c   |    6 ++++++
 fs/open.c   |   24 ++----------------------
 fs/utimes.c |    4 ----
 3 files changed, 8 insertions(+), 26 deletions(-)

Index: linux-2.6/fs/open.c
===================================================================
--- linux-2.6.orig/fs/open.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/open.c	2008-05-06 11:04:35.000000000 +0200
@@ -582,9 +582,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;
@@ -592,8 +589,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);
@@ -617,11 +612,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;
@@ -629,8 +619,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);
@@ -645,18 +633,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;
@@ -672,7 +652,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/attr.c
===================================================================
--- linux-2.6.orig/fs/attr.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/attr.c	2008-05-06 11:04:35.000000000 +0200
@@ -108,6 +108,12 @@ 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_ATIME_SET | ATTR_MTIME_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/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/utimes.c	2008-05-06 11:04:35.000000000 +0200
@@ -105,10 +105,6 @@ long do_utimes(int dfd, char __user *fil
 	/* Don't worry, the checks are done in inode_change_ok() */
 	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] 45+ messages in thread

* [patch 09/24] vfs: truncate: dont check immutable twice
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (7 preceding siblings ...)
  2008-05-06  9:13 ` [patch 08/24] vfs: immutable inode checking cleanup Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 10/24] vfs: truncate: append-only checking cleanup Miklos Szeredi
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: truncate_immutable_cleanup.patch --]
[-- Type: text/plain, Size: 739 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

vfs_permission(MAY_WRITE) already checked for the inode being
immutable, so no need to repeat it.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/open.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/fs/open.c
===================================================================
--- linux-2.6.orig/fs/open.c	2008-05-06 11:04:35.000000000 +0200
+++ linux-2.6/fs/open.c	2008-05-06 11:04:35.000000000 +0200
@@ -254,7 +254,7 @@ static long do_sys_truncate(const char _
 		goto mnt_drop_write_and_out;
 
 	error = -EPERM;
-	if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
+	if (IS_APPEND(inode))
 		goto mnt_drop_write_and_out;
 
 	error = get_write_access(inode);

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 10/24] vfs: truncate: append-only checking cleanup
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (8 preceding siblings ...)
  2008-05-06  9:13 ` [patch 09/24] vfs: truncate: dont check immutable twice Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-15  7:49   ` Al Viro
  2008-05-06  9:13 ` [patch 11/24] vfs: create file_truncate() helper Miklos Szeredi
                   ` (15 subsequent siblings)
  25 siblings, 1 reply; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: truncate_append_cleanup.patch --]
[-- Type: text/plain, Size: 2245 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Move the append-only checks from truncate to notify_change().  Checks
for append-only files are always performed by the VFS and not by the
filesystem so this belongs in notify_change(), and not in
inode_change_ok().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/attr.c  |    6 ++++++
 fs/namei.c |    2 --
 fs/open.c  |    8 --------
 3 files changed, 6 insertions(+), 10 deletions(-)

Index: linux-2.6/fs/open.c
===================================================================
--- linux-2.6.orig/fs/open.c	2008-05-06 11:04:35.000000000 +0200
+++ linux-2.6/fs/open.c	2008-05-06 11:04:36.000000000 +0200
@@ -253,10 +253,6 @@ static long do_sys_truncate(const char _
 	if (error)
 		goto mnt_drop_write_and_out;
 
-	error = -EPERM;
-	if (IS_APPEND(inode))
-		goto mnt_drop_write_and_out;
-
 	error = get_write_access(inode);
 	if (error)
 		goto mnt_drop_write_and_out;
@@ -321,10 +317,6 @@ static long do_sys_ftruncate(unsigned in
 	if (small && length > MAX_NON_LFS)
 		goto out_putf;
 
-	error = -EPERM;
-	if (IS_APPEND(inode))
-		goto out_putf;
-
 	error = locks_verify_truncate(inode, file, length);
 	if (!error)
 		error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
Index: linux-2.6/fs/attr.c
===================================================================
--- linux-2.6.orig/fs/attr.c	2008-05-06 11:04:35.000000000 +0200
+++ linux-2.6/fs/attr.c	2008-05-06 11:04:36.000000000 +0200
@@ -114,6 +114,12 @@ int notify_change(struct dentry * dentry
 			return -EPERM;
 	}
 
+	/*
+	 * IS_IMMUTABLE() is checked by caller for truncation
+	 */
+	if ((ia_valid & ATTR_SIZE) && IS_APPEND(inode))
+		return -EPERM;
+
 	now = current_fs_time(inode->i_sb);
 
 	attr->ia_ctime = now;
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-06 11:04:36.000000000 +0200
@@ -1639,8 +1639,6 @@ int may_open(struct nameidata *nd, int a
 	if (IS_APPEND(inode)) {
 		if  ((flag & FMODE_WRITE) && !(flag & O_APPEND))
 			return -EPERM;
-		if (flag & O_TRUNC)
-			return -EPERM;
 	}
 
 	/* O_NOATIME can only be set by the owner or superuser */

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 11/24] vfs: create file_truncate() helper
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (9 preceding siblings ...)
  2008-05-06  9:13 ` [patch 10/24] vfs: truncate: append-only checking cleanup Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 12/24] vfs: utimes immutable fix Miklos Szeredi
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: file_truncate.patch --]
[-- Type: text/plain, Size: 4123 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Clean up do_truncate() API:

  - create a new function file_truncate()
  - remove the 'struct file *' argument from do_truncate()

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/exec.c          |    2 +-
 fs/open.c          |   24 ++++++++++++++++++++----
 include/linux/fs.h |    2 ++
 mm/tiny-shmem.c    |    2 +-
 4 files changed, 24 insertions(+), 6 deletions(-)

Index: linux-2.6/fs/exec.c
===================================================================
--- linux-2.6.orig/fs/exec.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/fs/exec.c	2008-05-06 11:04:36.000000000 +0200
@@ -1763,7 +1763,7 @@ int do_coredump(long signr, int exit_cod
 		goto close_fail;
 	if (!file->f_op->write)
 		goto close_fail;
-	if (!ispipe && do_truncate(file->f_path.dentry, 0, 0, file) != 0)
+	if (!ispipe && file_truncate(file, 0, 0) != 0)
 		goto close_fail;
 
 	retval = binfmt->core_dump(signr, regs, file, core_limit);
Index: linux-2.6/fs/open.c
===================================================================
--- linux-2.6.orig/fs/open.c	2008-05-06 11:04:36.000000000 +0200
+++ linux-2.6/fs/open.c	2008-05-06 11:04:36.000000000 +0200
@@ -195,6 +195,13 @@ out:
 	return error;
 }
 
+/*
+ * do_truncate - truncate (or extend) an inode
+ * @dentry: the dentry to truncate
+ * @length: the new length
+ * @time_attrs: file times to be updated (e.g. ATTR_MTIME|ATTR_CTIME)
+ * @filp: an open file or NULL (see file_truncate() as well)
+ */
 int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs,
 	struct file *filp)
 {
@@ -221,6 +228,17 @@ int do_truncate(struct dentry *dentry, l
 	return err;
 }
 
+/*
+ * file_truncate - truncate (or extend) an open file
+ * @filp: the open file
+ * @length: the new length
+ * @time_attrs: file times to be updated (e.g. ATTR_MTIME|ATTR_CTIME)
+ */
+int file_truncate(struct file *filp, loff_t length, unsigned int time_attrs)
+{
+	return do_truncate(filp->f_path.dentry, length, time_attrs, filp);
+}
+
 static long do_sys_truncate(const char __user * path, loff_t length)
 {
 	struct nameidata nd;
@@ -290,7 +308,6 @@ asmlinkage long sys_truncate(const char 
 static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
 {
 	struct inode * inode;
-	struct dentry *dentry;
 	struct file * file;
 	int error;
 
@@ -306,8 +323,7 @@ static long do_sys_ftruncate(unsigned in
 	if (file->f_flags & O_LARGEFILE)
 		small = 0;
 
-	dentry = file->f_path.dentry;
-	inode = dentry->d_inode;
+	inode = file->f_path.dentry->d_inode;
 	error = -EINVAL;
 	if (!S_ISREG(inode->i_mode) || !(file->f_mode & FMODE_WRITE))
 		goto out_putf;
@@ -319,7 +335,7 @@ static long do_sys_ftruncate(unsigned in
 
 	error = locks_verify_truncate(inode, file, length);
 	if (!error)
-		error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
+		error = file_truncate(file, length, ATTR_MTIME|ATTR_CTIME);
 out_putf:
 	fput(file);
 out:
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:36.000000000 +0200
@@ -1605,6 +1605,8 @@ static inline int break_lease(struct ino
 
 extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,
 		       struct file *filp);
+extern int file_truncate(struct file *filp, loff_t start,
+			 unsigned int time_attrs);
 extern long do_sys_open(int dfd, const char __user *filename, int flags,
 			int mode);
 extern struct file *filp_open(const char *, int, int);
Index: linux-2.6/mm/tiny-shmem.c
===================================================================
--- linux-2.6.orig/mm/tiny-shmem.c	2008-05-06 11:04:29.000000000 +0200
+++ linux-2.6/mm/tiny-shmem.c	2008-05-06 11:04:36.000000000 +0200
@@ -80,7 +80,7 @@ struct file *shmem_file_setup(char *name
 	inode->i_nlink = 0;	/* It is unlinked */
 
 	/* notify everyone as to the change of file size */
-	error = do_truncate(dentry, size, 0, file);
+	error = file_truncate(file, size, 0);
 	if (error < 0)
 		goto close_file;
 

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 12/24] vfs: utimes immutable fix
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (10 preceding siblings ...)
  2008-05-06  9:13 ` [patch 11/24] vfs: create file_truncate() helper Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 13/24] vfs: utimes cleanup Miklos Szeredi
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, Ulrich Drepper

[-- Attachment #1: utimes_immutable_fix.patch --]
[-- Type: text/plain, Size: 1026 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

If updating file times to the current time and using a file
descriptor, then don't check for immutable inode, only if the file is
opened for write.  In this case immutability has been checked at open
time.  This is the same as how write() and ftruncate() are handled.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Ulrich Drepper <drepper@redhat.com>
---
 fs/utimes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c	2008-05-06 11:04:35.000000000 +0200
+++ linux-2.6/fs/utimes.c	2008-05-06 11:04:36.000000000 +0200
@@ -130,7 +130,7 @@ long do_utimes(int dfd, char __user *fil
 	if (!times || (nsec_special(times[0].tv_nsec) &&
 		       nsec_special(times[1].tv_nsec))) {
 		error = -EACCES;
-                if (IS_IMMUTABLE(inode))
+		if (!f && IS_IMMUTABLE(inode))
 			goto mnt_drop_write_and_out;
 
 		if (!is_owner_or_cap(inode)) {

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 13/24] vfs: utimes cleanup
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (11 preceding siblings ...)
  2008-05-06  9:13 ` [patch 12/24] vfs: utimes immutable fix Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06 10:03   ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 14/24] vfs: splice remove_suid() cleanup Miklos Szeredi
                   ` (12 subsequent siblings)
  25 siblings, 1 reply; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, Ulrich Drepper

[-- Attachment #1: utimes_cleanup.patch --]
[-- Type: text/plain, Size: 7745 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Untange the mess that is do_utimes()

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Ulrich Drepper <drepper@redhat.com>
---
 fs/compat.c |    4 -
 fs/utimes.c |  197 +++++++++++++++++++++++++++++++++++++-----------------------
 2 files changed, 124 insertions(+), 77 deletions(-)

Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c	2008-05-06 11:04:36.000000000 +0200
+++ linux-2.6/fs/utimes.c	2008-05-06 11:04:37.000000000 +0200
@@ -53,54 +53,10 @@ 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;
-
-		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);
-	if (error)
-		goto dput_and_out;
 
 	/* Don't worry, the checks are done in inode_change_ok() */
 	newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
@@ -121,41 +77,132 @@ long do_utimes(int dfd, char __user *fil
 			newattrs.ia_valid |= ATTR_MTIME_SET;
 		}
 	}
+	mutex_lock(&path->dentry->d_inode->i_mutex);
+	error = mnt_want_write(path->mnt);
+	if (!error) {
+		error = notify_change(path->dentry, &newattrs);
+		mnt_drop_write(path->mnt);
+	}
+	mutex_unlock(&path->dentry->d_inode->i_mutex);
+
+	return error;
+}
+
+/*
+ * If times is NULL or both times are either UTIME_OMIT or UTIME_NOW, then
+ * need to check permissions, because inode_change_ok() won't do it.
+ */
+static bool utimes_need_permission(struct timespec *times)
+{
+	return !times || (nsec_special(times[0].tv_nsec) &&
+			  nsec_special(times[1].tv_nsec));
+}
+
+static int do_futimes(int fd, struct timespec *times)
+{
+	int error;
+	struct file *file = fget(fd);
+
+	if (!file)
+		return -EBADF;
+
+	if (utimes_need_permission(times)) {
+		struct inode *inode = file->f_path.dentry->d_inode;
+
+		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:
+	fput(file);
+
+	return error;
+}
+
+
+/*
+ * do_futimesat - change times on filename
+ * @dfd: open file descriptor, -1 or AT_FDCWD
+ * @filename: path name
+ * @times: new times or NULL
+ * @flags: zero or more flags (only AT_SYMLINK_NOFOLLOW for the moment)
+ *
+ * Caller must verify the values in times (if not NULL)
+ *
+ * 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.
+ */
+int do_utimes(int dfd, char __user *filename, struct timespec *times, int flags)
+{
+	int error;
+	struct nameidata nd;
+	int lookup_flags;
+
+	if (flags & ~AT_SYMLINK_NOFOLLOW)
+		return -EINVAL;
+
+	lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+	error = __user_walk_fd(dfd, filename, lookup_flags, &nd);
+	if (error)
+		return error;
+
+
+	if (utimes_need_permission(times)) {
+		struct inode *inode = nd.path.dentry->d_inode;
 
-	/*
-	 * If times is NULL or both times are either UTIME_OMIT or
-	 * UTIME_NOW, then need to check permissions, because
-	 * inode_change_ok() won't do it.
-	 */
-	if (!times || (nsec_special(times[0].tv_nsec) &&
-		       nsec_special(times[1].tv_nsec))) {
 		error = -EACCES;
-		if (!f && IS_IMMUTABLE(inode))
-			goto mnt_drop_write_and_out;
+		if (IS_IMMUTABLE(inode))
+			goto out_path_put;
 
 		if (!is_owner_or_cap(inode)) {
-			if (f) {
-				if (!(f->f_mode & FMODE_WRITE))
-					goto mnt_drop_write_and_out;
-			} else {
-				error = vfs_permission(&nd, MAY_WRITE);
-				if (error)
-					goto mnt_drop_write_and_out;
-			}
+			error = vfs_permission(&nd, MAY_WRITE);
+			if (error)
+				goto out_path_put;
 		}
 	}
-	mutex_lock(&inode->i_mutex);
-	error = notify_change(dentry, &newattrs);
-	mutex_unlock(&inode->i_mutex);
-mnt_drop_write_and_out:
-	mnt_drop_write(mnt);
-dput_and_out:
-	if (f)
-		fput(f);
-	else
-		path_put(&nd.path);
-out:
+	error = utimes_common(&nd.path, times);
+
+ out_path_put:
+	path_put(&nd.path);
+
 	return error;
+
+}
+
+/*
+ * do_futimesat - 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 defers 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.
+ */
+int do_futimesat(int dfd, char __user *filename, struct timespec *times,
+		 int flags)
+{
+	if (times && (!nsec_valid(times[0].tv_nsec) ||
+		      !nsec_valid(times[1].tv_nsec))) {
+		return -EINVAL;
+	}
+
+	if (filename == NULL && dfd != AT_FDCWD) {
+		if (flags)
+			return -EINVAL;
+
+		return do_futimes(dfd, times);
+	} else {
+		return do_utimes(dfd, filename, times, flags);
+	}
 }
 
 asmlinkage long sys_utimensat(int dfd, char __user *filename, struct timespec __user *utimes, int flags)
@@ -180,7 +227,7 @@ asmlinkage long sys_utimensat(int dfd, c
 			return 0;
 	}
 
-	return do_utimes(dfd, filename, utimes ? tstimes : NULL, flags);
+	return do_futimesat(dfd, filename, utimes ? tstimes : NULL, flags);
 }
 
 asmlinkage long sys_futimesat(int dfd, char __user *filename, struct timeval __user *utimes)
@@ -207,7 +254,7 @@ asmlinkage long sys_futimesat(int dfd, c
 		tstimes[1].tv_nsec = 1000 * times[1].tv_usec;
 	}
 
-	return do_utimes(dfd, filename, utimes ? tstimes : NULL, 0);
+	return do_futimesat(dfd, filename, utimes ? tstimes : NULL, 0);
 }
 
 asmlinkage long sys_utimes(char __user *filename, struct timeval __user *utimes)
Index: linux-2.6/fs/compat.c
===================================================================
--- linux-2.6.orig/fs/compat.c	2008-05-06 11:04:28.000000000 +0200
+++ linux-2.6/fs/compat.c	2008-05-06 11:04:37.000000000 +0200
@@ -110,7 +110,7 @@ asmlinkage long compat_sys_utimensat(uns
 		if (tv[0].tv_nsec == UTIME_OMIT && tv[1].tv_nsec == UTIME_OMIT)
 			return 0;
 	}
-	return do_utimes(dfd, filename, t ? tv : NULL, flags);
+	return do_futimesat(dfd, filename, t ? tv : NULL, flags);
 }
 
 asmlinkage long compat_sys_futimesat(unsigned int dfd, char __user *filename, struct compat_timeval __user *t)
@@ -129,7 +129,7 @@ asmlinkage long compat_sys_futimesat(uns
 		tv[0].tv_nsec *= 1000;
 		tv[1].tv_nsec *= 1000;
 	}
-	return do_utimes(dfd, filename, t ? tv : NULL, 0);
+	return do_futimesat(dfd, filename, t ? tv : NULL, 0);
 }
 
 asmlinkage long compat_sys_utimes(char __user *filename, struct compat_timeval __user *t)

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 14/24] vfs: splice remove_suid() cleanup
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (12 preceding siblings ...)
  2008-05-06  9:13 ` [patch 13/24] vfs: utimes cleanup Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 15/24] vfs: add path_create() and path_mknod() Miklos Szeredi
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, Jens Axboe

[-- Attachment #1: splice_remove_suid_cleanup.patch --]
[-- Type: text/plain, Size: 3197 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

generic_file_splice_write() duplicates remove_suid() just because it
doesn't hold i_mutex.  But it grabs i_mutex inside splice_from_pipe()
anyway, so this is rather pointless.

Move locking to generic_file_splice_write() and call remove_suid() and
__splice_from_pipe() instead.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Jens Axboe <jens.axboe@oracle.com>
---
 fs/splice.c        |   29 +++++++++++++----------------
 include/linux/fs.h |    1 -
 mm/filemap.c       |    2 +-
 3 files changed, 14 insertions(+), 18 deletions(-)

Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c	2008-05-06 11:04:28.000000000 +0200
+++ linux-2.6/fs/splice.c	2008-05-06 11:04:37.000000000 +0200
@@ -811,24 +811,19 @@ generic_file_splice_write(struct pipe_in
 {
 	struct address_space *mapping = out->f_mapping;
 	struct inode *inode = mapping->host;
-	int killsuid, killpriv;
+	struct splice_desc sd = {
+		.total_len = len,
+		.flags = flags,
+		.pos = *ppos,
+		.u.file = out,
+	};
 	ssize_t ret;
-	int err = 0;
 
-	killpriv = security_inode_need_killpriv(out->f_path.dentry);
-	killsuid = should_remove_suid(out->f_path.dentry);
-	if (unlikely(killsuid || killpriv)) {
-		mutex_lock(&inode->i_mutex);
-		if (killpriv)
-			err = security_inode_killpriv(out->f_path.dentry);
-		if (!err && killsuid)
-			err = __remove_suid(out->f_path.dentry, killsuid);
-		mutex_unlock(&inode->i_mutex);
-		if (err)
-			return err;
-	}
-
-	ret = splice_from_pipe(pipe, out, ppos, len, flags, pipe_to_file);
+	inode_double_lock(inode, pipe->inode);
+	ret = remove_suid(out->f_path.dentry);
+	if (likely(!ret))
+		ret = __splice_from_pipe(pipe, &sd, pipe_to_file);
+	inode_double_unlock(inode, pipe->inode);
 	if (ret > 0) {
 		unsigned long nr_pages;
 
@@ -840,6 +835,8 @@ generic_file_splice_write(struct pipe_in
 		 * sync it.
 		 */
 		if (unlikely((out->f_flags & O_SYNC) || IS_SYNC(inode))) {
+			int err;
+
 			mutex_lock(&inode->i_mutex);
 			err = generic_osync_inode(inode, mapping,
 						  OSYNC_METADATA|OSYNC_DATA);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:36.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:37.000000000 +0200
@@ -1823,7 +1823,6 @@ extern void iget_failed(struct inode *);
 extern void clear_inode(struct inode *);
 extern void destroy_inode(struct inode *);
 extern struct inode *new_inode(struct super_block *);
-extern int __remove_suid(struct dentry *, int);
 extern int should_remove_suid(struct dentry *);
 extern int remove_suid(struct dentry *);
 
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c	2008-05-06 11:04:28.000000000 +0200
+++ linux-2.6/mm/filemap.c	2008-05-06 11:04:37.000000000 +0200
@@ -1655,7 +1655,7 @@ int should_remove_suid(struct dentry *de
 }
 EXPORT_SYMBOL(should_remove_suid);
 
-int __remove_suid(struct dentry *dentry, int kill)
+static int __remove_suid(struct dentry *dentry, int kill)
 {
 	struct iattr newattrs;
 

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 15/24] vfs: add path_create() and path_mknod()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (13 preceding siblings ...)
  2008-05-06  9:13 ` [patch 14/24] vfs: splice remove_suid() cleanup Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 16/24] vfs: add path_mkdir() Miklos Szeredi
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_create.patch --]
[-- Type: text/plain, Size: 13269 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

R/O bind mounts require operations which modify the filesystem to be
wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
this, so callers won't need to bother, and more importantly, cannot
forget!  Call these path_*, analogous to vfs_*.  Since there are no
callers of vfs_* left, make them static.

Overall this patchset is just 23 lines in the red, but at the same
time it fixes several places in nfsd and the whole of ecryptfs, where
the mnt_want_write/drop_write() calls were missing.

It will also help with merging certain security modules, which need to
know the path within the namespace, and not just within the
filesystem.  These helpers will allow the security hooks to be in a
common place, and need not be repeated in all callers.

Note, that the mnt_want_write/drop_write() bracketing provided by the
path_* functions is not strictly necessary in all cases, since the
caller may do it's own bracketing to span multiple VFS calls.
However, this does not make the checks in path_* incorrect, just
unnecessary.

The advantages of the path_* API are:

  - it's consistent

  - it provides some (not all) guarantees, i.e. it's easier to prove
    that all callers play by the rules

  - for the syscall case it has zero cost

  - for all the other cases it has either zero, or minimal cost

It does require the caller to have a vfsmount available, but it's hard
to imagine that the caller does not have it:

  - most userspace calls do have it, as they are either operating on a
    path or a file descriptor.  There are some exceptions like sync(2)
    and ustat(2), the latter not being a very exemplary interface, and
    neither of them being relevant to this discussion.

  - all kernel callers (nfs export, stacking) should have it, as they
    need it for open() anyway

And even if some theoretical caller didn't have the vfsmount, it still
should be easy to allocate one, providing a clean way to do the r/o
bracketing, and not requiring another mechanism to be exported that
provides the same functionality on the superblock.


This patch:

Introduce path_create() and path_mknod().  Make vfs_create() and
vfs_mknod() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   33 ++++++++++------------
 fs/namei.c          |   75 +++++++++++++++++++++++++++-------------------------
 fs/nfsd/vfs.c       |   19 +++++++++----
 include/linux/fs.h  |    4 +-
 ipc/mqueue.c        |    6 +++-
 net/unix/af_unix.c  |    6 ----
 6 files changed, 76 insertions(+), 67 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-06 11:04:36.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-06 11:04:38.000000000 +0200
@@ -1579,7 +1579,7 @@ void unlock_rename(struct dentry *p1, st
 	}
 }
 
-int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
+static int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
 		struct nameidata *nd)
 {
 	int error = may_create(dir, dentry, nd);
@@ -1601,6 +1601,20 @@ int vfs_create(struct inode *dir, struct
 	return error;
 }
 
+int path_create(struct path *dir_path, struct dentry *dentry, int mode,
+		struct nameidata *nd)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_create(dir_path->dentry->d_inode, dentry, mode, nd);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_create);
+
 int may_open(struct nameidata *nd, int acc_mode, int flag)
 {
 	struct dentry *dentry = nd->path.dentry;
@@ -2018,7 +2032,7 @@ fail:
 }
 EXPORT_SYMBOL_GPL(lookup_create);
 
-int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
+static int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
 {
 	int error = may_create(dir, dentry, NULL);
 
@@ -2046,22 +2060,19 @@ int vfs_mknod(struct inode *dir, struct 
 	return error;
 }
 
-static int may_mknod(mode_t mode)
+int path_mknod(struct path *dir_path, struct dentry *dentry, int mode,
+	       dev_t dev)
 {
-	switch (mode & S_IFMT) {
-	case S_IFREG:
-	case S_IFCHR:
-	case S_IFBLK:
-	case S_IFIFO:
-	case S_IFSOCK:
-	case 0: /* zero mode translates to S_IFREG */
-		return 0;
-	case S_IFDIR:
-		return -EPERM;
-	default:
-		return -EINVAL;
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_mknod(dir_path->dentry->d_inode, dentry, mode, dev);
+		mnt_drop_write(dir_path->mnt);
 	}
+
+	return error;
 }
+EXPORT_SYMBOL(path_mknod);
 
 asmlinkage long sys_mknodat(int dfd, const char __user *filename, int mode,
 				unsigned dev)
@@ -2087,26 +2098,22 @@ asmlinkage long sys_mknodat(int dfd, con
 	}
 	if (!IS_POSIXACL(nd.path.dentry->d_inode))
 		mode &= ~current->fs->umask;
-	error = may_mknod(mode);
-	if (error)
-		goto out_dput;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
 	switch (mode & S_IFMT) {
-		case 0: case S_IFREG:
-			error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
-			break;
-		case S_IFCHR: case S_IFBLK:
-			error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,
-					new_decode_dev(dev));
-			break;
-		case S_IFIFO: case S_IFSOCK:
-			error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
-			break;
+	case 0: case S_IFREG:
+		error = path_create(&nd.path, dentry, mode, &nd);
+		break;
+	case S_IFCHR: case S_IFBLK:
+		error = path_mknod(&nd.path, dentry, mode, new_decode_dev(dev));
+		break;
+	case S_IFIFO: case S_IFSOCK:
+		error = path_mknod(&nd.path, dentry, mode, 0);
+		break;
+	case S_IFDIR:
+		error = -EPERM;
+		break;
+	default:
+		error = -EINVAL;
 	}
-	mnt_drop_write(nd.path.mnt);
-out_dput:
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -2973,11 +2980,9 @@ EXPORT_SYMBOL(permission);
 EXPORT_SYMBOL(vfs_permission);
 EXPORT_SYMBOL(file_permission);
 EXPORT_SYMBOL(unlock_rename);
-EXPORT_SYMBOL(vfs_create);
 EXPORT_SYMBOL(vfs_follow_link);
 EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(vfs_mkdir);
-EXPORT_SYMBOL(vfs_mknod);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:37.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:38.000000000 +0200
@@ -1124,9 +1124,9 @@ extern void unlock_super(struct super_bl
  * VFS helper functions..
  */
 extern int vfs_permission(struct nameidata *, int);
-extern int vfs_create(struct inode *, struct dentry *, int, struct nameidata *);
+extern int path_create(struct path *, struct dentry *, int, struct nameidata *);
 extern int vfs_mkdir(struct inode *, struct dentry *, int);
-extern int vfs_mknod(struct inode *, struct dentry *, int, dev_t);
+extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
 extern int vfs_rmdir(struct inode *, struct dentry *);
Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-06 11:04:32.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-06 11:04:38.000000000 +0200
@@ -61,23 +61,19 @@ static void unlock_dir(struct dentry *di
  * Returns zero on success; non-zero on error condition
  */
 static int
-ecryptfs_create_underlying_file(struct inode *lower_dir_inode,
+ecryptfs_create_underlying_file(struct dentry *lower_dir_dentry,
 				struct dentry *dentry, int mode,
 				struct nameidata *nd)
 {
 	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	struct vfsmount *lower_mnt = ecryptfs_dentry_to_lower_mnt(dentry);
-	struct dentry *dentry_save;
-	struct vfsmount *vfsmount_save;
+	struct path save;
 	int rc;
 
-	dentry_save = nd->path.dentry;
-	vfsmount_save = nd->path.mnt;
-	nd->path.dentry = lower_dentry;
-	nd->path.mnt = lower_mnt;
-	rc = vfs_create(lower_dir_inode, lower_dentry, mode, nd);
-	nd->path.dentry = dentry_save;
-	nd->path.mnt = vfsmount_save;
+	save = nd->path;
+	nd->path.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	nd->path.dentry = lower_dir_dentry;
+	rc = path_create(&nd->path, lower_dentry, mode, nd);
+	nd->path = save;
 	return rc;
 }
 
@@ -111,7 +107,7 @@ ecryptfs_do_create(struct inode *directo
 		rc = PTR_ERR(lower_dir_dentry);
 		goto out;
 	}
-	rc = ecryptfs_create_underlying_file(lower_dir_dentry->d_inode,
+	rc = ecryptfs_create_underlying_file(lower_dir_dentry,
 					     ecryptfs_dentry, mode, nd);
 	if (rc) {
 		printk(KERN_ERR "%s: Failure to create dentry in lower fs; "
@@ -530,20 +526,21 @@ ecryptfs_mknod(struct inode *dir, struct
 {
 	int rc;
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
-	rc = vfs_mknod(lower_dir_dentry->d_inode, lower_dentry, mode, dev);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
+	rc = path_mknod(&lower_dir, lower_dentry, mode, dev);
 	if (rc || !lower_dentry->d_inode)
 		goto out;
 	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
 	if (rc)
 		goto out;
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	fsstack_copy_inode_size(dir, lower_dir.dentry->d_inode);
 out:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	if (!dentry->d_inode)
 		d_drop(dentry);
 	return rc;
Index: linux-2.6/ipc/mqueue.c
===================================================================
--- linux-2.6.orig/ipc/mqueue.c	2008-05-06 11:04:28.000000000 +0200
+++ linux-2.6/ipc/mqueue.c	2008-05-06 11:04:38.000000000 +0200
@@ -599,6 +599,7 @@ static struct file *do_create(struct den
 {
 	struct mq_attr attr;
 	struct file *result;
+	struct path dir_path;
 	int ret;
 
 	if (u_attr) {
@@ -616,7 +617,10 @@ static struct file *do_create(struct den
 	ret = mnt_want_write(mqueue_mnt);
 	if (ret)
 		goto out;
-	ret = vfs_create(dir->d_inode, dentry, mode, NULL);
+
+	dir_path.mnt = mqueue_mnt;
+	dir_path.dentry = dir;
+	ret = path_create(&dir_path, dentry, mode, NULL);
 	dentry->d_fsdata = NULL;
 	if (ret)
 		goto out_drop_write;
Index: linux-2.6/net/unix/af_unix.c
===================================================================
--- linux-2.6.orig/net/unix/af_unix.c	2008-05-06 11:04:28.000000000 +0200
+++ linux-2.6/net/unix/af_unix.c	2008-05-06 11:04:38.000000000 +0200
@@ -819,11 +819,7 @@ static int unix_bind(struct socket *sock
 		 */
 		mode = S_IFSOCK |
 		       (SOCK_INODE(sock)->i_mode & ~current->fs->umask);
-		err = mnt_want_write(nd.path.mnt);
-		if (err)
-			goto out_mknod_dput;
-		err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
-		mnt_drop_write(nd.path.mnt);
+		err = path_mknod(&nd.path, dentry, mode, 0);
 		if (err)
 			goto out_mknod_dput;
 		mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:32.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:38.000000000 +0200
@@ -130,6 +130,12 @@ out:
 	return err;
 }
 
+static void fh_to_path(struct svc_fh *fhp, struct path *path)
+{
+	path->dentry = fhp->fh_dentry;
+	path->mnt = fhp->fh_export->ex_path.mnt;
+}
+
 __be32
 nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		   const char *name, unsigned int len,
@@ -1184,6 +1190,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 		char *fname, int flen, struct iattr *iap,
 		int type, dev_t rdev, struct svc_fh *resfhp)
 {
+	struct path	dir_path;
 	struct dentry	*dentry, *dchild = NULL;
 	struct inode	*dirp;
 	__be32		err;
@@ -1259,13 +1266,11 @@ nfsd_create(struct svc_rqst *rqstp, stru
 	if (host_err)
 		goto out_nfserr;
 
-	/*
-	 * Get the dir op function pointer.
-	 */
+	fh_to_path(fhp, &dir_path);
 	err = 0;
 	switch (type) {
 	case S_IFREG:
-		host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
+		host_err = path_create(&dir_path, dchild, iap->ia_mode, NULL);
 		break;
 	case S_IFDIR:
 		host_err = vfs_mkdir(dirp, dchild, iap->ia_mode);
@@ -1274,7 +1279,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 	case S_IFBLK:
 	case S_IFIFO:
 	case S_IFSOCK:
-		host_err = vfs_mknod(dirp, dchild, iap->ia_mode, rdev);
+		host_err = path_mknod(&dir_path, dchild, iap->ia_mode, rdev);
 		break;
 	}
 	if (host_err < 0) {
@@ -1316,6 +1321,7 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 		struct svc_fh *resfhp, int createmode, u32 *verifier,
 	        int *truncp, int *created)
 {
+	struct path 	dir_path;
 	struct dentry	*dentry, *dchild = NULL;
 	struct inode	*dirp;
 	__be32		err;
@@ -1406,7 +1412,8 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 		goto out;
 	}
 
-	host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
+	fh_to_path(fhp, &dir_path);
+	host_err = path_create(&dir_path, dchild, iap->ia_mode, NULL);
 	if (host_err < 0) {
 		mnt_drop_write(fhp->fh_export->ex_path.mnt);
 		goto out_nfserr;

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 16/24] vfs: add path_mkdir()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (14 preceding siblings ...)
  2008-05-06  9:13 ` [patch 15/24] vfs: add path_create() and path_mknod() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 17/24] vfs: add path_rmdir() Miklos Szeredi
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_mkdir.patch --]
[-- Type: text/plain, Size: 5466 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_mkdir().  Make vfs_mkdir() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c   |   15 ++++++++-------
 fs/namei.c            |   23 +++++++++++++++--------
 fs/nfsd/nfs4recover.c |    6 +-----
 fs/nfsd/vfs.c         |    2 +-
 include/linux/fs.h    |    2 +-
 5 files changed, 26 insertions(+), 22 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-06 11:04:38.000000000 +0200
@@ -478,21 +478,22 @@ static int ecryptfs_mkdir(struct inode *
 {
 	int rc;
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
-	rc = vfs_mkdir(lower_dir_dentry->d_inode, lower_dentry, mode);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
+	rc = path_mkdir(&lower_dir, lower_dentry, mode);
 	if (rc || !lower_dentry->d_inode)
 		goto out;
 	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
 	if (rc)
 		goto out;
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
-	dir->i_nlink = lower_dir_dentry->d_inode->i_nlink;
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	fsstack_copy_inode_size(dir, lower_dir.dentry->d_inode);
+	dir->i_nlink = lower_dir.dentry->d_inode->i_nlink;
 out:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	if (!dentry->d_inode)
 		d_drop(dentry);
 	return rc;
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-06 11:04:38.000000000 +0200
@@ -2129,7 +2129,7 @@ asmlinkage long sys_mknod(const char __u
 	return sys_mknodat(AT_FDCWD, filename, mode, dev);
 }
 
-int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
+static int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 {
 	int error = may_create(dir, dentry, NULL);
 
@@ -2151,6 +2151,19 @@ int vfs_mkdir(struct inode *dir, struct 
 	return error;
 }
 
+int path_mkdir(struct path *dir_path, struct dentry *dentry, int mode)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_mkdir(dir_path->dentry->d_inode, dentry, mode);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_mkdir);
+
 asmlinkage long sys_mkdirat(int dfd, const char __user *pathname, int mode)
 {
 	int error = 0;
@@ -2173,12 +2186,7 @@ asmlinkage long sys_mkdirat(int dfd, con
 
 	if (!IS_POSIXACL(nd.path.dentry->d_inode))
 		mode &= ~current->fs->umask;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
-	error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
-	mnt_drop_write(nd.path.mnt);
-out_dput:
+	error = path_mkdir(&nd.path, dentry, mode);
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -2982,7 +2990,6 @@ EXPORT_SYMBOL(file_permission);
 EXPORT_SYMBOL(unlock_rename);
 EXPORT_SYMBOL(vfs_follow_link);
 EXPORT_SYMBOL(vfs_link);
-EXPORT_SYMBOL(vfs_mkdir);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
Index: linux-2.6/fs/nfsd/nfs4recover.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4recover.c	2008-05-06 11:04:28.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4recover.c	2008-05-06 11:04:38.000000000 +0200
@@ -155,11 +155,7 @@ nfsd4_create_clid_dir(struct nfs4_client
 		dprintk("NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS\n");
 		goto out_put;
 	}
-	status = mnt_want_write(rec_dir.path.mnt);
-	if (status)
-		goto out_put;
-	status = vfs_mkdir(rec_dir.path.dentry->d_inode, dentry, S_IRWXU);
-	mnt_drop_write(rec_dir.path.mnt);
+	status = path_mkdir(&rec_dir.path, dentry, S_IRWXU);
 out_put:
 	dput(dentry);
 out_unlock:
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:38.000000000 +0200
@@ -1273,7 +1273,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 		host_err = path_create(&dir_path, dchild, iap->ia_mode, NULL);
 		break;
 	case S_IFDIR:
-		host_err = vfs_mkdir(dirp, dchild, iap->ia_mode);
+		host_err = path_mkdir(&dir_path, dchild, iap->ia_mode);
 		break;
 	case S_IFCHR:
 	case S_IFBLK:
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:38.000000000 +0200
@@ -1125,7 +1125,7 @@ extern void unlock_super(struct super_bl
  */
 extern int vfs_permission(struct nameidata *, int);
 extern int path_create(struct path *, struct dentry *, int, struct nameidata *);
-extern int vfs_mkdir(struct inode *, struct dentry *, int);
+extern int path_mkdir(struct path *, struct dentry *, int);
 extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 17/24] vfs: add path_rmdir()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (15 preceding siblings ...)
  2008-05-06  9:13 ` [patch 16/24] vfs: add path_mkdir() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 18/24] vfs: add path_unlink() Miklos Szeredi
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_rmdir.patch --]
[-- Type: text/plain, Size: 5868 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_rmdir().

The only user of vfs_rmdir() remaining is reiserfs_delete_xattrs().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c   |   13 +++++++------
 fs/namei.c            |   23 +++++++++++++++--------
 fs/nfsd/nfs4recover.c |    3 ++-
 fs/nfsd/vfs.c         |    4 +++-
 include/linux/fs.h    |    2 +-
 5 files changed, 28 insertions(+), 17 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-06 11:04:39.000000000 +0200
@@ -2229,7 +2229,7 @@ void dentry_unhash(struct dentry *dentry
 	spin_unlock(&dcache_lock);
 }
 
-int vfs_rmdir(struct inode *dir, struct dentry *dentry)
+static int vfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	int error = may_delete(dir, dentry, 1);
 
@@ -2262,6 +2262,19 @@ int vfs_rmdir(struct inode *dir, struct 
 	return error;
 }
 
+int path_rmdir(struct path *dir_path, struct dentry *dentry)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_rmdir(dir_path->dentry->d_inode, dentry);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_rmdir);
+
 static long do_rmdir(int dfd, const char __user *pathname)
 {
 	int error = 0;
@@ -2293,12 +2306,7 @@ static long do_rmdir(int dfd, const char
 	error = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		goto exit2;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto exit3;
-	error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
-	mnt_drop_write(nd.path.mnt);
-exit3:
+	error = path_rmdir(&nd.path, dentry);
 	dput(dentry);
 exit2:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -2993,7 +3001,6 @@ EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
-EXPORT_SYMBOL(vfs_rmdir);
 EXPORT_SYMBOL(vfs_symlink);
 EXPORT_SYMBOL(vfs_unlink);
 EXPORT_SYMBOL(dentry_unhash);
Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-06 11:04:39.000000000 +0200
@@ -502,20 +502,21 @@ out:
 static int ecryptfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 	int rc;
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
 	dget(dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
 	dget(lower_dentry);
-	rc = vfs_rmdir(lower_dir_dentry->d_inode, lower_dentry);
+	rc = path_rmdir(&lower_dir, lower_dentry);
 	dput(lower_dentry);
 	if (!rc)
 		d_delete(lower_dentry);
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	dir->i_nlink = lower_dir_dentry->d_inode->i_nlink;
-	unlock_dir(lower_dir_dentry);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	dir->i_nlink = lower_dir.dentry->d_inode->i_nlink;
+	unlock_dir(lower_dir.dentry);
 	if (!rc)
 		d_drop(dentry);
 	dput(dentry);
Index: linux-2.6/fs/nfsd/nfs4recover.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4recover.c	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4recover.c	2008-05-06 11:04:39.000000000 +0200
@@ -267,6 +267,7 @@ nfsd4_remove_clid_file(struct dentry *di
 static int
 nfsd4_clear_clid_dir(struct dentry *dir, struct dentry *dentry)
 {
+	struct path dir_path = { .dentry = dir, .mnt = rec_dir.path.mnt };
 	int status;
 
 	/* For now this directory should already be empty, but we empty it of
@@ -274,7 +275,7 @@ nfsd4_clear_clid_dir(struct dentry *dir,
 	 * a kernel from the future.... */
 	nfsd4_list_rec_dir(dentry, nfsd4_remove_clid_file);
 	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
-	status = vfs_rmdir(dir->d_inode, dentry);
+	status = path_rmdir(&dir_path, dentry);
 	mutex_unlock(&dir->d_inode->i_mutex);
 	return status;
 }
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:39.000000000 +0200
@@ -1764,6 +1764,7 @@ __be32
 nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
 				char *fname, int flen)
 {
+	struct path 	dir_path;
 	struct dentry	*dentry, *rdentry;
 	struct inode	*dirp;
 	__be32		err;
@@ -1798,6 +1799,7 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 	if (host_err)
 		goto out_nfserr;
 
+	fh_to_path(fhp, &dir_path);
 	if (type != S_IFDIR) { /* It's UNLINK */
 #ifdef MSNFS
 		if ((fhp->fh_export->ex_flags & NFSEXP_MSNFS) &&
@@ -1807,7 +1809,7 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 #endif
 		host_err = vfs_unlink(dirp, rdentry);
 	} else { /* It's RMDIR */
-		host_err = vfs_rmdir(dirp, rdentry);
+		host_err = path_rmdir(&dir_path, rdentry);
 	}
 
 	dput(rdentry);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:39.000000000 +0200
@@ -1129,7 +1129,7 @@ extern int path_mkdir(struct path *, str
 extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
-extern int vfs_rmdir(struct inode *, struct dentry *);
+extern int path_rmdir(struct path *, struct dentry *);
 extern int vfs_unlink(struct inode *, struct dentry *);
 extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
 

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 18/24] vfs: add path_unlink()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (16 preceding siblings ...)
  2008-05-06  9:13 ` [patch 17/24] vfs: add path_rmdir() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 19/24] vfs: add path_symlink() Miklos Szeredi
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_unlink.patch --]
[-- Type: text/plain, Size: 7192 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_unlink().  Make vfs_unlink() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c   |   12 ++++++------
 fs/namei.c            |   22 +++++++++++++++-------
 fs/nfsd/nfs4recover.c |    3 ++-
 fs/nfsd/vfs.c         |   12 ++----------
 include/linux/fs.h    |    2 +-
 ipc/mqueue.c          |   10 +++++-----
 6 files changed, 31 insertions(+), 30 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-06 11:04:39.000000000 +0200
@@ -415,22 +415,22 @@ static int ecryptfs_unlink(struct inode 
 {
 	int rc = 0;
 	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	struct inode *lower_dir_inode = ecryptfs_inode_to_lower(dir);
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 
-	lower_dir_dentry = lock_parent(lower_dentry);
-	rc = vfs_unlink(lower_dir_inode, lower_dentry);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
+	rc = path_unlink(&lower_dir, lower_dentry);
 	if (rc) {
 		printk(KERN_ERR "Error in vfs_unlink; rc = [%d]\n", rc);
 		goto out_unlock;
 	}
-	fsstack_copy_attr_times(dir, lower_dir_inode);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
 	dentry->d_inode->i_nlink =
 		ecryptfs_inode_to_lower(dentry->d_inode)->i_nlink;
 	dentry->d_inode->i_ctime = dir->i_ctime;
 	d_drop(dentry);
 out_unlock:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	return rc;
 }
 
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-06 11:04:39.000000000 +0200
@@ -2322,7 +2322,7 @@ asmlinkage long sys_rmdir(const char __u
 	return do_rmdir(AT_FDCWD, pathname);
 }
 
-int vfs_unlink(struct inode *dir, struct dentry *dentry)
+static int vfs_unlink(struct inode *dir, struct dentry *dentry)
 {
 	int error = may_delete(dir, dentry, 0);
 
@@ -2353,6 +2353,19 @@ int vfs_unlink(struct inode *dir, struct
 	return error;
 }
 
+int path_unlink(struct path *dir_path, struct dentry *dentry)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_unlink(dir_path->dentry->d_inode, dentry);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_unlink);
+
 /*
  * Make sure that the actual truncation of the file will occur outside its
  * directory's i_mutex.  Truncate can take a long time if there is a lot of
@@ -2387,11 +2400,7 @@ static long do_unlinkat(int dfd, const c
 		inode = dentry->d_inode;
 		if (inode)
 			atomic_inc(&inode->i_count);
-		error = mnt_want_write(nd.path.mnt);
-		if (error)
-			goto exit2;
-		error = vfs_unlink(nd.path.dentry->d_inode, dentry);
-		mnt_drop_write(nd.path.mnt);
+		error = path_unlink(&nd.path, dentry);
 	exit2:
 		dput(dentry);
 	}
@@ -3002,6 +3011,5 @@ EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
 EXPORT_SYMBOL(vfs_symlink);
-EXPORT_SYMBOL(vfs_unlink);
 EXPORT_SYMBOL(dentry_unhash);
 EXPORT_SYMBOL(generic_readlink);
Index: linux-2.6/fs/nfsd/nfs4recover.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4recover.c	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4recover.c	2008-05-06 11:04:39.000000000 +0200
@@ -252,6 +252,7 @@ out:
 static int
 nfsd4_remove_clid_file(struct dentry *dir, struct dentry *dentry)
 {
+	struct path dir_path = { .dentry = dir, .mnt = rec_dir.path.mnt };
 	int status;
 
 	if (!S_ISREG(dir->d_inode->i_mode)) {
@@ -259,7 +260,7 @@ nfsd4_remove_clid_file(struct dentry *di
 		return -EINVAL;
 	}
 	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
-	status = vfs_unlink(dir->d_inode, dentry);
+	status = path_unlink(&dir_path, dentry);
 	mutex_unlock(&dir->d_inode->i_mutex);
 	return status;
 }
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:39.000000000 +0200
@@ -1766,7 +1766,6 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 {
 	struct path 	dir_path;
 	struct dentry	*dentry, *rdentry;
-	struct inode	*dirp;
 	__be32		err;
 	int		host_err;
 
@@ -1779,7 +1778,6 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 
 	fh_lock_nested(fhp, I_MUTEX_PARENT);
 	dentry = fhp->fh_dentry;
-	dirp = dentry->d_inode;
 
 	rdentry = lookup_one_len(fname, dentry, flen);
 	host_err = PTR_ERR(rdentry);
@@ -1795,10 +1793,6 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 	if (!type)
 		type = rdentry->d_inode->i_mode & S_IFMT;
 
-	host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
-	if (host_err)
-		goto out_nfserr;
-
 	fh_to_path(fhp, &dir_path);
 	if (type != S_IFDIR) { /* It's UNLINK */
 #ifdef MSNFS
@@ -1807,7 +1801,7 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 			host_err = -EPERM;
 		} else
 #endif
-		host_err = vfs_unlink(dirp, rdentry);
+		host_err = path_unlink(&dir_path, rdentry);
 	} else { /* It's RMDIR */
 		host_err = path_rmdir(&dir_path, rdentry);
 	}
@@ -1815,12 +1809,10 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 	dput(rdentry);
 
 	if (host_err)
-		goto out_drop;
+		goto out_nfserr;
 	if (EX_ISSYNC(fhp->fh_export))
 		host_err = nfsd_sync_dir(dentry);
 
-out_drop:
-	mnt_drop_write(fhp->fh_export->ex_path.mnt);
 out_nfserr:
 	err = nfserrno(host_err);
 out:
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:39.000000000 +0200
@@ -1130,7 +1130,7 @@ extern int path_mknod(struct path *, str
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
 extern int path_rmdir(struct path *, struct dentry *);
-extern int vfs_unlink(struct inode *, struct dentry *);
+extern int path_unlink(struct path *, struct dentry *);
 extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
 
 /*
Index: linux-2.6/ipc/mqueue.c
===================================================================
--- linux-2.6.orig/ipc/mqueue.c	2008-05-06 11:04:38.000000000 +0200
+++ linux-2.6/ipc/mqueue.c	2008-05-06 11:04:39.000000000 +0200
@@ -736,6 +736,7 @@ asmlinkage long sys_mq_unlink(const char
 	char *name;
 	struct dentry *dentry;
 	struct inode *inode = NULL;
+	struct path dir_path;
 
 	name = getname(u_name);
 	if (IS_ERR(name))
@@ -757,11 +758,10 @@ asmlinkage long sys_mq_unlink(const char
 	inode = dentry->d_inode;
 	if (inode)
 		atomic_inc(&inode->i_count);
-	err = mnt_want_write(mqueue_mnt);
-	if (err)
-		goto out_err;
-	err = vfs_unlink(dentry->d_parent->d_inode, dentry);
-	mnt_drop_write(mqueue_mnt);
+
+	dir_path.mnt = mqueue_mnt;
+	dir_path.dentry = dentry->d_parent;
+	err = path_unlink(&dir_path, dentry);
 out_err:
 	dput(dentry);
 

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 19/24] vfs: add path_symlink()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (17 preceding siblings ...)
  2008-05-06  9:13 ` [patch 18/24] vfs: add path_unlink() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 20/24] vfs: add path_link() Miklos Szeredi
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_symlink.patch --]
[-- Type: text/plain, Size: 6302 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_symlink().  Make vfs_symlink() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   14 +++++++-------
 fs/namei.c          |   26 ++++++++++++++++++--------
 fs/nfsd/vfs.c       |   12 ++++--------
 include/linux/fs.h  |    2 +-
 4 files changed, 30 insertions(+), 24 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-06 11:04:40.000000000 +0200
@@ -439,7 +439,7 @@ static int ecryptfs_symlink(struct inode
 {
 	int rc;
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 	umode_t mode;
 	char *encoded_symname;
 	int encoded_symlen;
@@ -447,7 +447,8 @@ static int ecryptfs_symlink(struct inode
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
 	dget(lower_dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
 	mode = S_IALLUGO;
 	encoded_symlen = ecryptfs_encode_filename(crypt_stat, symname,
 						  strlen(symname),
@@ -456,18 +457,17 @@ static int ecryptfs_symlink(struct inode
 		rc = encoded_symlen;
 		goto out_lock;
 	}
-	rc = vfs_symlink(lower_dir_dentry->d_inode, lower_dentry,
-			 encoded_symname, mode);
+	rc = path_symlink(&lower_dir, lower_dentry, encoded_symname, mode);
 	kfree(encoded_symname);
 	if (rc || !lower_dentry->d_inode)
 		goto out_lock;
 	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
 	if (rc)
 		goto out_lock;
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	fsstack_copy_inode_size(dir, lower_dir.dentry->d_inode);
 out_lock:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	dput(lower_dentry);
 	if (!dentry->d_inode)
 		d_drop(dentry);
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-06 11:04:40.000000000 +0200
@@ -2435,7 +2435,7 @@ asmlinkage long sys_unlink(const char __
 	return do_unlinkat(AT_FDCWD, pathname);
 }
 
-int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname, int mode)
+static int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname, int mode)
 {
 	int error = may_create(dir, dentry, NULL);
 
@@ -2456,6 +2456,22 @@ int vfs_symlink(struct inode *dir, struc
 	return error;
 }
 
+int path_symlink(struct path *dir_path, struct dentry *dentry,
+		 const char *oldname, int mode)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		struct inode *dir = dir_path->dentry->d_inode;
+
+		error = vfs_symlink(dir, dentry, oldname, mode);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_symlink);
+
 asmlinkage long sys_symlinkat(const char __user *oldname,
 			      int newdfd, const char __user *newname)
 {
@@ -2481,12 +2497,7 @@ asmlinkage long sys_symlinkat(const char
 	if (IS_ERR(dentry))
 		goto out_unlock;
 
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
-	error = vfs_symlink(nd.path.dentry->d_inode, dentry, from, S_IALLUGO);
-	mnt_drop_write(nd.path.mnt);
-out_dput:
+	error = path_symlink(&nd.path, dentry, from, S_IALLUGO);
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -3010,6 +3021,5 @@ EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
-EXPORT_SYMBOL(vfs_symlink);
 EXPORT_SYMBOL(dentry_unhash);
 EXPORT_SYMBOL(generic_readlink);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:40.000000000 +0200
@@ -1518,6 +1518,7 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 				struct svc_fh *resfhp,
 				struct iattr *iap)
 {
+	struct path 	dir_path;
 	struct dentry	*dentry, *dnew;
 	__be32		err, cerr;
 	int		host_err;
@@ -1545,10 +1546,7 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 	if (iap && (iap->ia_valid & ATTR_MODE))
 		mode = iap->ia_mode & S_IALLUGO;
 
-	host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
-	if (host_err)
-		goto out_nfserr;
-
+	fh_to_path(fhp, &dir_path);
 	if (unlikely(path[plen] != 0)) {
 		char *path_alloced = kmalloc(plen+1, GFP_KERNEL);
 		if (path_alloced == NULL)
@@ -1556,11 +1554,11 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 		else {
 			strncpy(path_alloced, path, plen);
 			path_alloced[plen] = 0;
-			host_err = vfs_symlink(dentry->d_inode, dnew, path_alloced, mode);
+			host_err = path_symlink(&dir_path, dnew, path_alloced, mode);
 			kfree(path_alloced);
 		}
 	} else
-		host_err = vfs_symlink(dentry->d_inode, dnew, path, mode);
+		host_err = path_symlink(&dir_path, dnew, path, mode);
 
 	if (!host_err) {
 		if (EX_ISSYNC(fhp->fh_export))
@@ -1569,8 +1567,6 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 	err = nfserrno(host_err);
 	fh_unlock(fhp);
 
-	mnt_drop_write(fhp->fh_export->ex_path.mnt);
-
 	cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp);
 	dput(dnew);
 	if (err==0) err = cerr;
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:39.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:40.000000000 +0200
@@ -1127,7 +1127,7 @@ extern int vfs_permission(struct nameida
 extern int path_create(struct path *, struct dentry *, int, struct nameidata *);
 extern int path_mkdir(struct path *, struct dentry *, int);
 extern int path_mknod(struct path *, struct dentry *, int, dev_t);
-extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
+extern int path_symlink(struct path *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
 extern int path_rmdir(struct path *, struct dentry *);
 extern int path_unlink(struct path *, struct dentry *);

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 20/24] vfs: add path_link()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (18 preceding siblings ...)
  2008-05-06  9:13 ` [patch 19/24] vfs: add path_symlink() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 21/24] vfs: add path_rename() Miklos Szeredi
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_link.patch --]
[-- Type: text/plain, Size: 5971 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_link().  Make vfs_link() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   10 +++++-----
 fs/namei.c          |   26 ++++++++++++++++++--------
 fs/nfsd/vfs.c       |   14 ++++----------
 include/linux/fs.h  |    2 +-
 4 files changed, 28 insertions(+), 24 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-06 11:04:40.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-06 11:04:40.000000000 +0200
@@ -379,7 +379,7 @@ static int ecryptfs_link(struct dentry *
 {
 	struct dentry *lower_old_dentry;
 	struct dentry *lower_new_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 	u64 file_size_save;
 	int rc;
 
@@ -388,9 +388,9 @@ static int ecryptfs_link(struct dentry *
 	lower_new_dentry = ecryptfs_dentry_to_lower(new_dentry);
 	dget(lower_old_dentry);
 	dget(lower_new_dentry);
-	lower_dir_dentry = lock_parent(lower_new_dentry);
-	rc = vfs_link(lower_old_dentry, lower_dir_dentry->d_inode,
-		      lower_new_dentry);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(new_dentry);
+	lower_dir.dentry = lock_parent(lower_new_dentry);
+	rc = path_link(lower_old_dentry, &lower_dir, lower_new_dentry);
 	if (rc || !lower_new_dentry->d_inode)
 		goto out_lock;
 	rc = ecryptfs_interpose(lower_new_dentry, new_dentry, dir->i_sb, 0);
@@ -402,7 +402,7 @@ static int ecryptfs_link(struct dentry *
 		ecryptfs_inode_to_lower(old_dentry->d_inode)->i_nlink;
 	i_size_write(new_dentry->d_inode, file_size_save);
 out_lock:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	dput(lower_new_dentry);
 	dput(lower_old_dentry);
 	d_drop(lower_old_dentry);
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-06 11:04:40.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-06 11:04:40.000000000 +0200
@@ -2514,7 +2514,7 @@ asmlinkage long sys_symlink(const char _
 	return sys_symlinkat(oldname, AT_FDCWD, newname);
 }
 
-int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_dentry)
+static int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_dentry)
 {
 	struct inode *inode = old_dentry->d_inode;
 	int error;
@@ -2552,6 +2552,22 @@ int vfs_link(struct dentry *old_dentry, 
 	return error;
 }
 
+int path_link(struct dentry *old_dentry, struct path *dir_path,
+	      struct dentry *new_dentry)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		struct inode *dir = dir_path->dentry->d_inode;
+
+		error = vfs_link(old_dentry, dir, new_dentry);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_link);
+
 /*
  * Hardlinks are often used in delicate situations.  We avoid
  * security-related surprises by not following symlinks on the
@@ -2592,12 +2608,7 @@ asmlinkage long sys_linkat(int olddfd, c
 	error = PTR_ERR(new_dentry);
 	if (IS_ERR(new_dentry))
 		goto out_unlock;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
-	error = vfs_link(old_nd.path.dentry, nd.path.dentry->d_inode, new_dentry);
-	mnt_drop_write(nd.path.mnt);
-out_dput:
+	error = path_link(old_nd.path.dentry, &nd.path, new_dentry);
 	dput(new_dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -3017,7 +3028,6 @@ EXPORT_SYMBOL(vfs_permission);
 EXPORT_SYMBOL(file_permission);
 EXPORT_SYMBOL(unlock_rename);
 EXPORT_SYMBOL(vfs_follow_link);
-EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:40.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:40.000000000 +0200
@@ -1586,8 +1586,9 @@ __be32
 nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
 				char *name, int len, struct svc_fh *tfhp)
 {
+	struct path 	dir_path;
 	struct dentry	*ddir, *dnew, *dold;
-	struct inode	*dirp, *dest;
+	struct inode	*dest;
 	__be32		err;
 	int		host_err;
 
@@ -1607,7 +1608,6 @@ nfsd_link(struct svc_rqst *rqstp, struct
 
 	fh_lock_nested(ffhp, I_MUTEX_PARENT);
 	ddir = ffhp->fh_dentry;
-	dirp = ddir->d_inode;
 
 	dnew = lookup_one_len(name, ddir, len);
 	host_err = PTR_ERR(dnew);
@@ -1617,12 +1617,8 @@ nfsd_link(struct svc_rqst *rqstp, struct
 	dold = tfhp->fh_dentry;
 	dest = dold->d_inode;
 
-	host_err = mnt_want_write(tfhp->fh_export->ex_path.mnt);
-	if (host_err) {
-		err = nfserrno(host_err);
-		goto out_dput;
-	}
-	host_err = vfs_link(dold, dirp, dnew);
+	fh_to_path(ffhp, &dir_path);
+	host_err = path_link(dold, &dir_path, dnew);
 	if (!host_err) {
 		if (EX_ISSYNC(ffhp->fh_export)) {
 			err = nfserrno(nfsd_sync_dir(ddir));
@@ -1635,8 +1631,6 @@ nfsd_link(struct svc_rqst *rqstp, struct
 		else
 			err = nfserrno(host_err);
 	}
-	mnt_drop_write(tfhp->fh_export->ex_path.mnt);
-out_dput:
 	dput(dnew);
 out_unlock:
 	fh_unlock(ffhp);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:40.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:40.000000000 +0200
@@ -1128,7 +1128,7 @@ extern int path_create(struct path *, st
 extern int path_mkdir(struct path *, struct dentry *, int);
 extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int path_symlink(struct path *, struct dentry *, const char *, int);
-extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
+extern int path_link(struct dentry *, struct path *, struct dentry *);
 extern int path_rmdir(struct path *, struct dentry *);
 extern int path_unlink(struct path *, struct dentry *);
 extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 21/24] vfs: add path_rename()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (19 preceding siblings ...)
  2008-05-06  9:13 ` [patch 20/24] vfs: add path_link() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 22/24] vfs: add path_setattr() Miklos Szeredi
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_rename.patch --]
[-- Type: text/plain, Size: 6710 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_rename().  Make vfs_rename() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   22 ++++++++++++----------
 fs/namei.c          |   33 ++++++++++++++++++++++++---------
 fs/nfsd/vfs.c       |   19 +++++--------------
 include/linux/fs.h  |    3 ++-
 4 files changed, 43 insertions(+), 34 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-06 11:04:40.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-06 11:04:41.000000000 +0200
@@ -555,25 +555,27 @@ ecryptfs_rename(struct inode *old_dir, s
 	int rc;
 	struct dentry *lower_old_dentry;
 	struct dentry *lower_new_dentry;
-	struct dentry *lower_old_dir_dentry;
-	struct dentry *lower_new_dir_dentry;
+	struct path lower_old_dir;
+	struct path lower_new_dir;
 
 	lower_old_dentry = ecryptfs_dentry_to_lower(old_dentry);
 	lower_new_dentry = ecryptfs_dentry_to_lower(new_dentry);
 	dget(lower_old_dentry);
 	dget(lower_new_dentry);
-	lower_old_dir_dentry = dget_parent(lower_old_dentry);
-	lower_new_dir_dentry = dget_parent(lower_new_dentry);
-	lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
-	rc = vfs_rename(lower_old_dir_dentry->d_inode, lower_old_dentry,
-			lower_new_dir_dentry->d_inode, lower_new_dentry);
+	lower_old_dir.mnt = ecryptfs_dentry_to_lower_mnt(old_dentry);
+	lower_old_dir.dentry = dget_parent(lower_old_dentry);
+	lower_new_dir.mnt = ecryptfs_dentry_to_lower_mnt(new_dentry);
+	lower_new_dir.dentry = dget_parent(lower_new_dentry);
+	lock_rename(lower_old_dir.dentry, lower_new_dir.dentry);
+	rc = path_rename(&lower_old_dir, lower_old_dentry,
+			 &lower_new_dir, lower_new_dentry);
 	if (rc)
 		goto out_lock;
-	fsstack_copy_attr_all(new_dir, lower_new_dir_dentry->d_inode, NULL);
+	fsstack_copy_attr_all(new_dir, lower_new_dir.dentry->d_inode, NULL);
 	if (new_dir != old_dir)
-		fsstack_copy_attr_all(old_dir, lower_old_dir_dentry->d_inode, NULL);
+		fsstack_copy_attr_all(old_dir, lower_old_dir.dentry->d_inode, NULL);
 out_lock:
-	unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
+	unlock_rename(lower_old_dir.dentry, lower_new_dir.dentry);
 	dput(lower_new_dentry->d_parent);
 	dput(lower_old_dentry->d_parent);
 	dput(lower_new_dentry);
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-06 11:04:40.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-06 11:04:41.000000000 +0200
@@ -2730,8 +2730,8 @@ static int vfs_rename_other(struct inode
 	return error;
 }
 
-int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
-	       struct inode *new_dir, struct dentry *new_dentry)
+static int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
+		      struct inode *new_dir, struct dentry *new_dentry)
 {
 	int error;
 	int is_dir = S_ISDIR(old_dentry->d_inode->i_mode);
@@ -2773,6 +2773,27 @@ int vfs_rename(struct inode *old_dir, st
 	return error;
 }
 
+int path_rename(struct path *old_dir_path, struct dentry *old_dentry,
+		struct path *new_dir_path, struct dentry *new_dentry)
+{
+	int error;
+	struct vfsmount *mnt = old_dir_path->mnt;
+
+	BUG_ON(mnt != new_dir_path->mnt);
+
+	error = mnt_want_write(mnt);
+	if (!error) {
+		struct inode *old_dir = old_dir_path->dentry->d_inode;
+		struct inode *new_dir = new_dir_path->dentry->d_inode;
+
+		error = vfs_rename(old_dir, old_dentry, new_dir, new_dentry);
+		mnt_drop_write(mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_rename);
+
 static int do_rename(int olddfd, const char *oldname,
 			int newdfd, const char *newname)
 {
@@ -2834,12 +2855,7 @@ static int do_rename(int olddfd, const c
 	if (new_dentry == trap)
 		goto exit5;
 
-	error = mnt_want_write(oldnd.path.mnt);
-	if (error)
-		goto exit5;
-	error = vfs_rename(old_dir->d_inode, old_dentry,
-				   new_dir->d_inode, new_dentry);
-	mnt_drop_write(oldnd.path.mnt);
+	error = path_rename(&oldnd.path, old_dentry, &newnd.path, new_dentry);
 exit5:
 	dput(new_dentry);
 exit4:
@@ -3030,6 +3046,5 @@ EXPORT_SYMBOL(unlock_rename);
 EXPORT_SYMBOL(vfs_follow_link);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
-EXPORT_SYMBOL(vfs_rename);
 EXPORT_SYMBOL(dentry_unhash);
 EXPORT_SYMBOL(generic_readlink);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:40.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:41.000000000 +0200
@@ -1650,8 +1650,9 @@ __be32
 nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
 			    struct svc_fh *tfhp, char *tname, int tlen)
 {
+	struct path 	old_dir_path;
+	struct path	new_dir_path;
 	struct dentry	*fdentry, *tdentry, *odentry, *ndentry, *trap;
-	struct inode	*fdir, *tdir;
 	__be32		err;
 	int		host_err;
 
@@ -1663,10 +1664,7 @@ nfsd_rename(struct svc_rqst *rqstp, stru
 		goto out;
 
 	fdentry = ffhp->fh_dentry;
-	fdir = fdentry->d_inode;
-
 	tdentry = tfhp->fh_dentry;
-	tdir = tdentry->d_inode;
 
 	err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
 	if (ffhp->fh_export != tfhp->fh_export)
@@ -1710,22 +1708,15 @@ nfsd_rename(struct svc_rqst *rqstp, stru
 			goto out_dput_new;
 	}
 
-	host_err = -EXDEV;
-	if (ffhp->fh_export->ex_path.mnt != tfhp->fh_export->ex_path.mnt)
-		goto out_dput_new;
-	host_err = mnt_want_write(ffhp->fh_export->ex_path.mnt);
-	if (host_err)
-		goto out_dput_new;
-
-	host_err = vfs_rename(fdir, odentry, tdir, ndentry);
+	fh_to_path(ffhp, &old_dir_path);
+	fh_to_path(tfhp, &new_dir_path);
+	host_err = path_rename(&old_dir_path, odentry, &new_dir_path, ndentry);
 	if (!host_err && EX_ISSYNC(tfhp->fh_export)) {
 		host_err = nfsd_sync_dir(tdentry);
 		if (!host_err)
 			host_err = nfsd_sync_dir(fdentry);
 	}
 
-	mnt_drop_write(ffhp->fh_export->ex_path.mnt);
-
  out_dput_new:
 	dput(ndentry);
  out_dput_old:
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:40.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:41.000000000 +0200
@@ -1131,7 +1131,8 @@ extern int path_symlink(struct path *, s
 extern int path_link(struct dentry *, struct path *, struct dentry *);
 extern int path_rmdir(struct path *, struct dentry *);
 extern int path_unlink(struct path *, struct dentry *);
-extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
+extern int path_rename(struct path *, struct dentry *,
+		       struct path *, struct dentry *);
 
 /*
  * VFS dentry helper functions.

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 22/24] vfs: add path_setattr()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (20 preceding siblings ...)
  2008-05-06  9:13 ` [patch 21/24] vfs: add path_rename() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 23/24] vfs: add path_setxattr() Miklos Szeredi
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_setattr.patch --]
[-- Type: text/plain, Size: 9209 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_setattr().  Stop exporting notify_change() to modules.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/attr.c           |   14 +++++++++++++-
 fs/ecryptfs/inode.c |   11 ++++++-----
 fs/nfsd/vfs.c       |   17 +++++++++++------
 fs/open.c           |   52 ++++++++++------------------------------------------
 fs/utimes.c         |    6 +-----
 include/linux/fs.h  |    1 +
 6 files changed, 42 insertions(+), 59 deletions(-)

Index: linux-2.6/fs/open.c
===================================================================
--- linux-2.6.orig/fs/open.c	2008-05-06 11:04:36.000000000 +0200
+++ linux-2.6/fs/open.c	2008-05-06 11:04:41.000000000 +0200
@@ -587,18 +587,13 @@ asmlinkage long sys_fchmod(unsigned int 
 
 	audit_inode(NULL, dentry);
 
-	err = mnt_want_write(file->f_path.mnt);
-	if (err)
-		goto out_putf;
 	mutex_lock(&inode->i_mutex);
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;
 	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
 	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
-	err = notify_change(dentry, &newattrs);
+	err = path_setattr(&file->f_path, &newattrs);
 	mutex_unlock(&inode->i_mutex);
-	mnt_drop_write(file->f_path.mnt);
-out_putf:
 	fput(file);
 out:
 	return err;
@@ -617,18 +612,13 @@ asmlinkage long sys_fchmodat(int dfd, co
 		goto out;
 	inode = nd.path.dentry->d_inode;
 
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto dput_and_out;
 	mutex_lock(&inode->i_mutex);
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;
 	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
 	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
-	error = notify_change(nd.path.dentry, &newattrs);
+	error = path_setattr(&nd.path, &newattrs);
 	mutex_unlock(&inode->i_mutex);
-	mnt_drop_write(nd.path.mnt);
-dput_and_out:
 	path_put(&nd.path);
 out:
 	return error;
@@ -639,9 +629,9 @@ asmlinkage long sys_chmod(const char __u
 	return sys_fchmodat(AT_FDCWD, filename, mode);
 }
 
-static int chown_common(struct dentry * dentry, uid_t user, gid_t group)
+static int chown_common(struct path *path, uid_t user, gid_t group)
 {
-	struct inode *inode = dentry->d_inode;
+	struct inode *inode = path->dentry->d_inode;
 	int error;
 	struct iattr newattrs;
 
@@ -658,7 +648,7 @@ static int chown_common(struct dentry * 
 		newattrs.ia_valid |=
 			ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
 	mutex_lock(&inode->i_mutex);
-	error = notify_change(dentry, &newattrs);
+	error = path_setattr(path, &newattrs);
 	mutex_unlock(&inode->i_mutex);
 
 	return error;
@@ -672,12 +662,7 @@ asmlinkage long sys_chown(const char __u
 	error = user_path_walk(filename, &nd);
 	if (error)
 		goto out;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_release;
-	error = chown_common(nd.path.dentry, user, group);
-	mnt_drop_write(nd.path.mnt);
-out_release:
+	error = chown_common(&nd.path, user, group);
 	path_put(&nd.path);
 out:
 	return error;
@@ -697,12 +682,7 @@ asmlinkage long sys_fchownat(int dfd, co
 	error = __user_walk_fd(dfd, filename, follow, &nd);
 	if (error)
 		goto out;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_release;
-	error = chown_common(nd.path.dentry, user, group);
-	mnt_drop_write(nd.path.mnt);
-out_release:
+	error = chown_common(&nd.path, user, group);
 	path_put(&nd.path);
 out:
 	return error;
@@ -716,12 +696,7 @@ asmlinkage long sys_lchown(const char __
 	error = user_path_walk_link(filename, &nd);
 	if (error)
 		goto out;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_release;
-	error = chown_common(nd.path.dentry, user, group);
-	mnt_drop_write(nd.path.mnt);
-out_release:
+	error = chown_common(&nd.path, user, group);
 	path_put(&nd.path);
 out:
 	return error;
@@ -732,20 +707,13 @@ asmlinkage long sys_fchown(unsigned int 
 {
 	struct file * file;
 	int error = -EBADF;
-	struct dentry * dentry;
 
 	file = fget(fd);
 	if (!file)
 		goto out;
 
-	error = mnt_want_write(file->f_path.mnt);
-	if (error)
-		goto out_fput;
-	dentry = file->f_path.dentry;
-	audit_inode(NULL, dentry);
-	error = chown_common(dentry, user, group);
-	mnt_drop_write(file->f_path.mnt);
-out_fput:
+	audit_inode(NULL, file->f_path.dentry);
+	error = chown_common(&file->f_path, user, group);
 	fput(file);
 out:
 	return error;
Index: linux-2.6/fs/attr.c
===================================================================
--- linux-2.6.orig/fs/attr.c	2008-05-06 11:04:36.000000000 +0200
+++ linux-2.6/fs/attr.c	2008-05-06 11:04:41.000000000 +0200
@@ -14,6 +14,7 @@
 #include <linux/fcntl.h>
 #include <linux/quotaops.h>
 #include <linux/security.h>
+#include <linux/mount.h>
 
 /* Taken over from the old code... */
 
@@ -195,4 +196,15 @@ int notify_change(struct dentry * dentry
 	return error;
 }
 
-EXPORT_SYMBOL(notify_change);
+int path_setattr(struct path *path, struct iattr *attr)
+{
+	int error = mnt_want_write(path->mnt);
+
+	if (!error) {
+		error = notify_change(path->dentry, attr);
+		mnt_drop_write(path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_setattr);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:41.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:41.000000000 +0200
@@ -392,8 +392,11 @@ nfsd_setattr(struct svc_rqst *rqstp, str
 
 	err = nfserr_notsync;
 	if (!check_guard || guardtime == inode->i_ctime.tv_sec) {
+		struct path path;
+
 		fh_lock(fhp);
-		host_err = notify_change(dentry, iap);
+		fh_to_path(fhp, &path);
+		host_err = path_setattr(&path, iap);
 		err = nfserrno(host_err);
 		fh_unlock(fhp);
 	}
@@ -949,14 +952,16 @@ out:
 	return err;
 }
 
-static void kill_suid(struct dentry *dentry)
+static void kill_suid(struct svc_fh *fhp)
 {
+	struct path	path;
 	struct iattr	ia;
 	ia.ia_valid = ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
 
-	mutex_lock(&dentry->d_inode->i_mutex);
-	notify_change(dentry, &ia);
-	mutex_unlock(&dentry->d_inode->i_mutex);
+	mutex_lock(&path.dentry->d_inode->i_mutex);
+	fh_to_path(fhp, &path);
+	path_setattr(&path, &ia);
+	mutex_unlock(&path.dentry->d_inode->i_mutex);
 }
 
 static __be32
@@ -1014,7 +1019,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, s
 
 	/* clear setuid/setgid flag after write */
 	if (host_err >= 0 && (inode->i_mode & (S_ISUID | S_ISGID)))
-		kill_suid(dentry);
+		kill_suid(fhp);
 
 	if (host_err >= 0 && stable) {
 		static ino_t	last_ino;
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-06 11:04:41.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-06 11:04:41.000000000 +0200
@@ -1762,6 +1762,7 @@ extern int do_remount_sb(struct super_bl
 extern sector_t bmap(struct inode *, sector_t);
 #endif
 extern int notify_change(struct dentry *, struct iattr *);
+extern int path_setattr(struct path *, struct iattr *);
 extern int permission(struct inode *, int, struct nameidata *);
 extern int generic_permission(struct inode *, int,
 		int (*check_acl)(struct inode *, int));
Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-06 11:04:41.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-06 11:04:41.000000000 +0200
@@ -843,7 +843,7 @@ ecryptfs_permission(struct inode *inode,
 static int ecryptfs_setattr(struct dentry *dentry, struct iattr *ia)
 {
 	int rc = 0;
-	struct dentry *lower_dentry;
+	struct path lower_path;
 	struct inode *inode;
 	struct inode *lower_inode;
 	struct ecryptfs_crypt_stat *crypt_stat;
@@ -853,7 +853,8 @@ static int ecryptfs_setattr(struct dentr
 		ecryptfs_init_crypt_stat(crypt_stat);
 	inode = dentry->d_inode;
 	lower_inode = ecryptfs_inode_to_lower(inode);
-	lower_dentry = ecryptfs_dentry_to_lower(dentry);
+	lower_path.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_path.dentry = ecryptfs_dentry_to_lower(dentry);
 	mutex_lock(&crypt_stat->cs_mutex);
 	if (S_ISDIR(dentry->d_inode->i_mode))
 		crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED);
@@ -904,9 +905,9 @@ static int ecryptfs_setattr(struct dentr
 	if (ia->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID))
 		ia->ia_valid &= ~ATTR_MODE;
 
-	mutex_lock(&lower_dentry->d_inode->i_mutex);
-	rc = notify_change(lower_dentry, ia);
-	mutex_unlock(&lower_dentry->d_inode->i_mutex);
+	mutex_lock(&lower_path.dentry->d_inode->i_mutex);
+	rc = path_setattr(&lower_path, ia);
+	mutex_unlock(&lower_path.dentry->d_inode->i_mutex);
 out:
 	fsstack_copy_attr_all(inode, lower_inode, NULL);
 	return rc;
Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c	2008-05-06 11:04:37.000000000 +0200
+++ linux-2.6/fs/utimes.c	2008-05-06 11:04:41.000000000 +0200
@@ -78,11 +78,7 @@ static int utimes_common(struct path *pa
 		}
 	}
 	mutex_lock(&path->dentry->d_inode->i_mutex);
-	error = mnt_want_write(path->mnt);
-	if (!error) {
-		error = notify_change(path->dentry, &newattrs);
-		mnt_drop_write(path->mnt);
-	}
+	error = path_setattr(path, &newattrs);
 	mutex_unlock(&path->dentry->d_inode->i_mutex);
 
 	return error;

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 23/24] vfs: add path_setxattr()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (21 preceding siblings ...)
  2008-05-06  9:13 ` [patch 22/24] vfs: add path_setattr() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:13 ` [patch 24/24] vfs: add path_removexattr() Miklos Szeredi
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_setxattr.patch --]
[-- Type: text/plain, Size: 6486 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_setxattr().  Make vfs_setxattr() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/nfsd/vfs.c         |   18 ++++++++++--------
 fs/xattr.c            |   44 ++++++++++++++++++++++----------------------
 include/linux/xattr.h |    3 ++-
 3 files changed, 34 insertions(+), 31 deletions(-)

Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:41.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:42.000000000 +0200
@@ -434,7 +434,7 @@ static ssize_t nfsd_getxattr(struct dent
 
 #if defined(CONFIG_NFSD_V4)
 static int
-set_nfsv4_acl_one(struct dentry *dentry, struct posix_acl *pacl, char *key)
+set_nfsv4_acl_one(struct path *path, struct posix_acl *pacl, char *key)
 {
 	int len;
 	size_t buflen;
@@ -453,7 +453,7 @@ set_nfsv4_acl_one(struct dentry *dentry,
 		goto out;
 	}
 
-	error = vfs_setxattr(dentry, key, buf, len, 0);
+	error = path_setxattr(path, key, buf, len, 0);
 out:
 	kfree(buf);
 	return error;
@@ -465,7 +465,7 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
 {
 	__be32 error;
 	int host_error;
-	struct dentry *dentry;
+	struct path path;
 	struct inode *inode;
 	struct posix_acl *pacl = NULL, *dpacl = NULL;
 	unsigned int flags = 0;
@@ -475,8 +475,8 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
 	if (error)
 		return error;
 
-	dentry = fhp->fh_dentry;
-	inode = dentry->d_inode;
+	fh_to_path(fhp, &path);
+	inode = path.dentry->d_inode;
 	if (S_ISDIR(inode->i_mode))
 		flags = NFS4_ACL_DIR;
 
@@ -486,12 +486,12 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
 	} else if (host_error < 0)
 		goto out_nfserr;
 
-	host_error = set_nfsv4_acl_one(dentry, pacl, POSIX_ACL_XATTR_ACCESS);
+	host_error = set_nfsv4_acl_one(&path, pacl, POSIX_ACL_XATTR_ACCESS);
 	if (host_error < 0)
 		goto out_release;
 
 	if (S_ISDIR(inode->i_mode))
-		host_error = set_nfsv4_acl_one(dentry, dpacl, POSIX_ACL_XATTR_DEFAULT);
+		host_error = set_nfsv4_acl_one(&path, dpacl, POSIX_ACL_XATTR_DEFAULT);
 
 out_release:
 	posix_acl_release(pacl);
@@ -2038,6 +2038,7 @@ nfsd_get_posix_acl(struct svc_fh *fhp, i
 int
 nfsd_set_posix_acl(struct svc_fh *fhp, int type, struct posix_acl *acl)
 {
+	struct path path;
 	struct inode *inode = fhp->fh_dentry->d_inode;
 	char *name;
 	void *value = NULL;
@@ -2073,8 +2074,9 @@ nfsd_set_posix_acl(struct svc_fh *fhp, i
 	error = mnt_want_write(fhp->fh_export->ex_path.mnt);
 	if (error)
 		goto getout;
+	fh_to_path(fhp, &path);
 	if (size)
-		error = vfs_setxattr(fhp->fh_dentry, name, value, size, 0);
+		error = path_setxattr(&path, name, value, size, 0);
 	else {
 		if (!S_ISDIR(inode->i_mode) && type == ACL_TYPE_DEFAULT)
 			error = 0;
Index: linux-2.6/fs/xattr.c
===================================================================
--- linux-2.6.orig/fs/xattr.c	2008-05-06 11:04:27.000000000 +0200
+++ linux-2.6/fs/xattr.c	2008-05-06 11:04:42.000000000 +0200
@@ -66,7 +66,7 @@ xattr_permission(struct inode *inode, co
 	return permission(inode, mask, NULL);
 }
 
-int
+static int
 vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
 		size_t size, int flags)
 {
@@ -101,7 +101,20 @@ out:
 	mutex_unlock(&inode->i_mutex);
 	return error;
 }
-EXPORT_SYMBOL_GPL(vfs_setxattr);
+
+int path_setxattr(struct path *path, const char *name, const void *value,
+		   size_t size, int flags)
+{
+	int error = mnt_want_write(path->mnt);
+
+	if (!error) {
+		error = vfs_setxattr(path->dentry, name, value, size, flags);
+		mnt_drop_write(path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_setxattr);
 
 ssize_t
 xattr_getsecurity(struct inode *inode, const char *name, void *value,
@@ -218,7 +231,7 @@ EXPORT_SYMBOL_GPL(vfs_removexattr);
  * Extended attribute SET operations
  */
 static long
-setxattr(struct dentry *d, const char __user *name, const void __user *value,
+setxattr(struct path *path, const char __user *name, const void __user *value,
 	 size_t size, int flags)
 {
 	int error;
@@ -246,7 +259,7 @@ setxattr(struct dentry *d, const char __
 		}
 	}
 
-	error = vfs_setxattr(d, kname, kvalue, size, flags);
+	error = path_setxattr(path, kname, kvalue, size, flags);
 	kfree(kvalue);
 	return error;
 }
@@ -261,11 +274,8 @@ sys_setxattr(const char __user *path, co
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;
-	error = mnt_want_write(nd.path.mnt);
-	if (!error) {
-		error = setxattr(nd.path.dentry, name, value, size, flags);
-		mnt_drop_write(nd.path.mnt);
-	}
+
+	error = setxattr(&nd.path, name, value, size, flags);
 	path_put(&nd.path);
 	return error;
 }
@@ -280,11 +290,7 @@ sys_lsetxattr(const char __user *path, c
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;
-	error = mnt_want_write(nd.path.mnt);
-	if (!error) {
-		error = setxattr(nd.path.dentry, name, value, size, flags);
-		mnt_drop_write(nd.path.mnt);
-	}
+	error = setxattr(&nd.path, name, value, size, flags);
 	path_put(&nd.path);
 	return error;
 }
@@ -294,19 +300,13 @@ sys_fsetxattr(int fd, const char __user 
 	      size_t size, int flags)
 {
 	struct file *f;
-	struct dentry *dentry;
 	int error = -EBADF;
 
 	f = fget(fd);
 	if (!f)
 		return error;
-	dentry = f->f_path.dentry;
-	audit_inode(NULL, dentry);
-	error = mnt_want_write(f->f_path.mnt);
-	if (!error) {
-		error = setxattr(dentry, name, value, size, flags);
-		mnt_drop_write(f->f_path.mnt);
-	}
+	audit_inode(NULL, f->f_path.dentry);
+	error = setxattr(&f->f_path, name, value, size, flags);
 	fput(f);
 	return error;
 }
Index: linux-2.6/include/linux/xattr.h
===================================================================
--- linux-2.6.orig/include/linux/xattr.h	2008-05-06 11:04:27.000000000 +0200
+++ linux-2.6/include/linux/xattr.h	2008-05-06 11:04:42.000000000 +0200
@@ -35,6 +35,7 @@
 
 struct inode;
 struct dentry;
+struct path;
 
 struct xattr_handler {
 	char *prefix;
@@ -49,7 +50,7 @@ struct xattr_handler {
 ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
 ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
 ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
-int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
+int path_setxattr(struct path *, const char *, const void *, size_t, int);
 int vfs_removexattr(struct dentry *, const char *);
 
 ssize_t generic_getxattr(struct dentry *dentry, const char *name, void *buffer, size_t size);

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [patch 24/24] vfs: add path_removexattr()
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (22 preceding siblings ...)
  2008-05-06  9:13 ` [patch 23/24] vfs: add path_setxattr() Miklos Szeredi
@ 2008-05-06  9:13 ` Miklos Szeredi
  2008-05-06  9:16 ` [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Christoph Hellwig
  2008-05-07 11:57 ` Miklos Szeredi
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06  9:13 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

[-- Attachment #1: path_removexattr.patch --]
[-- Type: text/plain, Size: 4607 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_removexattr().  Make vfs_removexattr() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/nfsd/vfs.c         |    6 +-----
 fs/xattr.c            |   41 +++++++++++++++++++----------------------
 include/linux/xattr.h |    2 +-
 3 files changed, 21 insertions(+), 28 deletions(-)

Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-06 11:04:42.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-06 11:04:43.000000000 +0200
@@ -2071,9 +2071,6 @@ nfsd_set_posix_acl(struct svc_fh *fhp, i
 	} else
 		size = 0;
 
-	error = mnt_want_write(fhp->fh_export->ex_path.mnt);
-	if (error)
-		goto getout;
 	fh_to_path(fhp, &path);
 	if (size)
 		error = path_setxattr(&path, name, value, size, 0);
@@ -2081,12 +2078,11 @@ nfsd_set_posix_acl(struct svc_fh *fhp, i
 		if (!S_ISDIR(inode->i_mode) && type == ACL_TYPE_DEFAULT)
 			error = 0;
 		else {
-			error = vfs_removexattr(fhp->fh_dentry, name);
+			error = path_removexattr(&path, name);
 			if (error == -ENODATA)
 				error = 0;
 		}
 	}
-	mnt_drop_write(fhp->fh_export->ex_path.mnt);
 
 getout:
 	kfree(value);
Index: linux-2.6/fs/xattr.c
===================================================================
--- linux-2.6.orig/fs/xattr.c	2008-05-06 11:04:42.000000000 +0200
+++ linux-2.6/fs/xattr.c	2008-05-06 11:04:43.000000000 +0200
@@ -199,7 +199,7 @@ vfs_listxattr(struct dentry *d, char *li
 }
 EXPORT_SYMBOL_GPL(vfs_listxattr);
 
-int
+static int
 vfs_removexattr(struct dentry *dentry, const char *name)
 {
 	struct inode *inode = dentry->d_inode;
@@ -224,8 +224,19 @@ vfs_removexattr(struct dentry *dentry, c
 		fsnotify_xattr(dentry);
 	return error;
 }
-EXPORT_SYMBOL_GPL(vfs_removexattr);
 
+int path_removexattr(struct path *path, const char *name)
+{
+	int error = mnt_want_write(path->mnt);
+
+	if (!error) {
+		error = vfs_removexattr(path->dentry, name);
+		mnt_drop_write(path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_removexattr);
 
 /*
  * Extended attribute SET operations
@@ -471,7 +482,7 @@ sys_flistxattr(int fd, char __user *list
  * Extended attribute REMOVE operations
  */
 static long
-removexattr(struct dentry *d, const char __user *name)
+removexattr(struct path *path, const char __user *name)
 {
 	int error;
 	char kname[XATTR_NAME_MAX + 1];
@@ -482,7 +493,7 @@ removexattr(struct dentry *d, const char
 	if (error < 0)
 		return error;
 
-	return vfs_removexattr(d, kname);
+	return path_removexattr(path, kname);
 }
 
 asmlinkage long
@@ -494,11 +505,7 @@ sys_removexattr(const char __user *path,
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;
-	error = mnt_want_write(nd.path.mnt);
-	if (!error) {
-		error = removexattr(nd.path.dentry, name);
-		mnt_drop_write(nd.path.mnt);
-	}
+	error = removexattr(&nd.path, name);
 	path_put(&nd.path);
 	return error;
 }
@@ -512,11 +519,7 @@ sys_lremovexattr(const char __user *path
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;
-	error = mnt_want_write(nd.path.mnt);
-	if (!error) {
-		error = removexattr(nd.path.dentry, name);
-		mnt_drop_write(nd.path.mnt);
-	}
+	error = removexattr(&nd.path, name);
 	path_put(&nd.path);
 	return error;
 }
@@ -525,19 +528,13 @@ asmlinkage long
 sys_fremovexattr(int fd, const char __user *name)
 {
 	struct file *f;
-	struct dentry *dentry;
 	int error = -EBADF;
 
 	f = fget(fd);
 	if (!f)
 		return error;
-	dentry = f->f_path.dentry;
-	audit_inode(NULL, dentry);
-	error = mnt_want_write(f->f_path.mnt);
-	if (!error) {
-		error = removexattr(dentry, name);
-		mnt_drop_write(f->f_path.mnt);
-	}
+	audit_inode(NULL, f->f_path.dentry);
+	error = removexattr(&f->f_path, name);
 	fput(f);
 	return error;
 }
Index: linux-2.6/include/linux/xattr.h
===================================================================
--- linux-2.6.orig/include/linux/xattr.h	2008-05-06 11:04:42.000000000 +0200
+++ linux-2.6/include/linux/xattr.h	2008-05-06 11:04:43.000000000 +0200
@@ -51,7 +51,7 @@ ssize_t xattr_getsecurity(struct inode *
 ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
 ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
 int path_setxattr(struct path *, const char *, const void *, size_t, int);
-int vfs_removexattr(struct dentry *, const char *);
+int path_removexattr(struct path *, const char *);
 
 ssize_t generic_getxattr(struct dentry *dentry, const char *name, void *buffer, size_t size);
 ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size);

--

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (23 preceding siblings ...)
  2008-05-06  9:13 ` [patch 24/24] vfs: add path_removexattr() Miklos Szeredi
@ 2008-05-06  9:16 ` Christoph Hellwig
  2008-05-06 10:33   ` Miklos Szeredi
  2008-05-07 11:57 ` Miklos Szeredi
  25 siblings, 1 reply; 45+ messages in thread
From: Christoph Hellwig @ 2008-05-06  9:16 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: akpm, hch, viro, linux-fsdevel, linux-kernel

On Tue, May 06, 2008 at 11:13:27AM +0200, Miklos Szeredi wrote:
> Here it is again as a single series (without the ecryptfs one already
> merged).
> 
> I've addressed Christoph's comments.

And totally ignored Al's while merging the second rejected series into
the first one.   I also still want to go over the various
setattr-related changes, pleawse give me some time for those.


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 13/24] vfs: utimes cleanup
  2008-05-06  9:13 ` [patch 13/24] vfs: utimes cleanup Miklos Szeredi
@ 2008-05-06 10:03   ` Miklos Szeredi
  2008-05-15  7:39     ` Al Viro
  0 siblings, 1 reply; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06 10:03 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel, drepper

One hunk got left out, I'm sorry...  Here's the fixed patch.

Miklos
----

Subject: vfs: utimes cleanup

From: Miklos Szeredi <mszeredi@suse.cz>

Untange the mess that is do_utimes()

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Ulrich Drepper <drepper@redhat.com>
---
 fs/compat.c          |    4 -
 fs/utimes.c          |  197 +++++++++++++++++++++++++++++++--------------------
 include/linux/time.h |    5 +
 3 files changed, 128 insertions(+), 78 deletions(-)

Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c	2008-05-06 11:59:47.000000000 +0200
+++ linux-2.6/fs/utimes.c	2008-05-06 11:59:49.000000000 +0200
@@ -53,54 +53,10 @@ 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;
-
-		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);
-	if (error)
-		goto dput_and_out;
 
 	/* Don't worry, the checks are done in inode_change_ok() */
 	newattrs.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_ATIME;
@@ -121,41 +77,132 @@ long do_utimes(int dfd, char __user *fil
 			newattrs.ia_valid |= ATTR_MTIME_SET;
 		}
 	}
+	mutex_lock(&path->dentry->d_inode->i_mutex);
+	error = mnt_want_write(path->mnt);
+	if (!error) {
+		error = notify_change(path->dentry, &newattrs);
+		mnt_drop_write(path->mnt);
+	}
+	mutex_unlock(&path->dentry->d_inode->i_mutex);
+
+	return error;
+}
+
+/*
+ * If times is NULL or both times are either UTIME_OMIT or UTIME_NOW, then
+ * need to check permissions, because inode_change_ok() won't do it.
+ */
+static bool utimes_need_permission(struct timespec *times)
+{
+	return !times || (nsec_special(times[0].tv_nsec) &&
+			  nsec_special(times[1].tv_nsec));
+}
+
+static int do_futimes(int fd, struct timespec *times)
+{
+	int error;
+	struct file *file = fget(fd);
+
+	if (!file)
+		return -EBADF;
+
+	if (utimes_need_permission(times)) {
+		struct inode *inode = file->f_path.dentry->d_inode;
+
+		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:
+	fput(file);
+
+	return error;
+}
+
+
+/*
+ * do_futimesat - change times on filename
+ * @dfd: open file descriptor, -1 or AT_FDCWD
+ * @filename: path name
+ * @times: new times or NULL
+ * @flags: zero or more flags (only AT_SYMLINK_NOFOLLOW for the moment)
+ *
+ * Caller must verify the values in times (if not NULL)
+ *
+ * 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.
+ */
+int do_utimes(int dfd, char __user *filename, struct timespec *times, int flags)
+{
+	int error;
+	struct nameidata nd;
+	int lookup_flags;
+
+	if (flags & ~AT_SYMLINK_NOFOLLOW)
+		return -EINVAL;
+
+	lookup_flags = (flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+	error = __user_walk_fd(dfd, filename, lookup_flags, &nd);
+	if (error)
+		return error;
+
+
+	if (utimes_need_permission(times)) {
+		struct inode *inode = nd.path.dentry->d_inode;
 
-	/*
-	 * If times is NULL or both times are either UTIME_OMIT or
-	 * UTIME_NOW, then need to check permissions, because
-	 * inode_change_ok() won't do it.
-	 */
-	if (!times || (nsec_special(times[0].tv_nsec) &&
-		       nsec_special(times[1].tv_nsec))) {
 		error = -EACCES;
-		if (!f && IS_IMMUTABLE(inode))
-			goto mnt_drop_write_and_out;
+		if (IS_IMMUTABLE(inode))
+			goto out_path_put;
 
 		if (!is_owner_or_cap(inode)) {
-			if (f) {
-				if (!(f->f_mode & FMODE_WRITE))
-					goto mnt_drop_write_and_out;
-			} else {
-				error = vfs_permission(&nd, MAY_WRITE);
-				if (error)
-					goto mnt_drop_write_and_out;
-			}
+			error = vfs_permission(&nd, MAY_WRITE);
+			if (error)
+				goto out_path_put;
 		}
 	}
-	mutex_lock(&inode->i_mutex);
-	error = notify_change(dentry, &newattrs);
-	mutex_unlock(&inode->i_mutex);
-mnt_drop_write_and_out:
-	mnt_drop_write(mnt);
-dput_and_out:
-	if (f)
-		fput(f);
-	else
-		path_put(&nd.path);
-out:
+	error = utimes_common(&nd.path, times);
+
+ out_path_put:
+	path_put(&nd.path);
+
 	return error;
+
+}
+
+/*
+ * do_futimesat - 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 defers 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.
+ */
+int do_futimesat(int dfd, char __user *filename, struct timespec *times,
+		 int flags)
+{
+	if (times && (!nsec_valid(times[0].tv_nsec) ||
+		      !nsec_valid(times[1].tv_nsec))) {
+		return -EINVAL;
+	}
+
+	if (filename == NULL && dfd != AT_FDCWD) {
+		if (flags)
+			return -EINVAL;
+
+		return do_futimes(dfd, times);
+	} else {
+		return do_utimes(dfd, filename, times, flags);
+	}
 }
 
 asmlinkage long sys_utimensat(int dfd, char __user *filename, struct timespec __user *utimes, int flags)
@@ -180,7 +227,7 @@ asmlinkage long sys_utimensat(int dfd, c
 			return 0;
 	}
 
-	return do_utimes(dfd, filename, utimes ? tstimes : NULL, flags);
+	return do_futimesat(dfd, filename, utimes ? tstimes : NULL, flags);
 }
 
 asmlinkage long sys_futimesat(int dfd, char __user *filename, struct timeval __user *utimes)
@@ -207,7 +254,7 @@ asmlinkage long sys_futimesat(int dfd, c
 		tstimes[1].tv_nsec = 1000 * times[1].tv_usec;
 	}
 
-	return do_utimes(dfd, filename, utimes ? tstimes : NULL, 0);
+	return do_futimesat(dfd, filename, utimes ? tstimes : NULL, 0);
 }
 
 asmlinkage long sys_utimes(char __user *filename, struct timeval __user *utimes)
Index: linux-2.6/fs/compat.c
===================================================================
--- linux-2.6.orig/fs/compat.c	2008-05-06 11:58:12.000000000 +0200
+++ linux-2.6/fs/compat.c	2008-05-06 11:59:49.000000000 +0200
@@ -110,7 +110,7 @@ asmlinkage long compat_sys_utimensat(uns
 		if (tv[0].tv_nsec == UTIME_OMIT && tv[1].tv_nsec == UTIME_OMIT)
 			return 0;
 	}
-	return do_utimes(dfd, filename, t ? tv : NULL, flags);
+	return do_futimesat(dfd, filename, t ? tv : NULL, flags);
 }
 
 asmlinkage long compat_sys_futimesat(unsigned int dfd, char __user *filename, struct compat_timeval __user *t)
@@ -129,7 +129,7 @@ asmlinkage long compat_sys_futimesat(uns
 		tv[0].tv_nsec *= 1000;
 		tv[1].tv_nsec *= 1000;
 	}
-	return do_utimes(dfd, filename, t ? tv : NULL, 0);
+	return do_futimesat(dfd, filename, t ? tv : NULL, 0);
 }
 
 asmlinkage long compat_sys_utimes(char __user *filename, struct compat_timeval __user *t)
Index: linux-2.6/include/linux/time.h
===================================================================
--- linux-2.6.orig/include/linux/time.h	2008-05-06 12:00:07.000000000 +0200
+++ linux-2.6/include/linux/time.h	2008-05-06 12:00:10.000000000 +0200
@@ -109,7 +109,10 @@ extern void do_gettimeofday(struct timev
 extern int do_settimeofday(struct timespec *tv);
 extern int do_sys_settimeofday(struct timespec *tv, struct timezone *tz);
 #define do_posix_clock_monotonic_gettime(ts) ktime_get_ts(ts)
-extern long do_utimes(int dfd, char __user *filename, struct timespec *times, int flags);
+extern int do_utimes(int dfd, char __user *filename, struct timespec *times,
+		     int flags);
+extern int do_futimesat(int dfd, char __user *filename, struct timespec *times,
+			int flags);
 struct itimerval;
 extern int do_setitimer(int which, struct itimerval *value,
 			struct itimerval *ovalue);

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts
  2008-05-06  9:16 ` [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Christoph Hellwig
@ 2008-05-06 10:33   ` Miklos Szeredi
  2008-05-14 14:55     ` Pavel Machek
  0 siblings, 1 reply; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-06 10:33 UTC (permalink / raw)
  To: hch; +Cc: miklos, akpm, hch, viro, linux-fsdevel, linux-kernel

> > Here it is again as a single series (without the ecryptfs one already
> > merged).
> > 
> > I've addressed Christoph's comments.
> 
> And totally ignored Al's while merging the second rejected series into
> the first one.

Al doesn't like it but everybody else does.  His excuses are very
hollow and hypocritical, just take this for example: "Except that it
fixes nothing in nfsd, as we'd already figured out".  That's after
discussing extensively how racy the current nfsd code is.  And even
then the only acknowledgement I get is that the code has always been
racy.  You bet it has been bloody racy, that's what the r-o-bind
patches are supposed to fix.

All through that tread, I got a barrage of nasties thrown at me, and
every time it turned out that I was right.  And then just that issue
disappears from the next reply.  No "sorry about that, you were
right".  Yes, I'm getting tired from Al's attitude towards this thing,
and as long as no better patches from him or anybody else are
forthcoming, or comments actually addressing the _patches_ themselves,
instead of some hypothetical users of these interfaces, I'm just going
to ignore Al's rejections outright.

Now I have much respect for work he has and is doing on the kernel,
make no mistake.  But that doesn't make him somebody who can reject
patches on a whim.

> I also still want to go over the various setattr-related changes,
> pleawse give me some time for those.

Sure, and thanks for looking at the patches.

Miklos

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts
  2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
                   ` (24 preceding siblings ...)
  2008-05-06  9:16 ` [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Christoph Hellwig
@ 2008-05-07 11:57 ` Miklos Szeredi
  25 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-07 11:57 UTC (permalink / raw)
  To: akpm; +Cc: hch, viro, linux-fsdevel, linux-kernel

> Here it is again as a single series (without the ecryptfs one already
> merged).
> 
> I've addressed Christoph's comments.

And here is the same (well, with some cosmetic fixes) as a git tree,
in case it's more convenient for somebody to review/test.

git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git vfs-cleanups


 fs/attr.c                   |   26 +++++-
 fs/compat.c                 |    4 +-
 fs/ecryptfs/inode.c         |  137 +++++++++++++-------------
 fs/exec.c                   |    2 +-
 fs/fat/file.c               |    5 +-
 fs/hpfs/namei.c             |    3 +-
 fs/namei.c                  |  230 +++++++++++++++++++++++++++----------------
 fs/nfsd/nfs4recover.c       |   12 +--
 fs/nfsd/vfs.c               |  148 ++++++++++++----------------
 fs/open.c                   |  106 ++++++--------------
 fs/reiserfs/namei.c         |    2 +-
 fs/reiserfs/xattr.c         |   32 +++++-
 fs/splice.c                 |   29 +++---
 fs/sysfs/file.c             |    5 +-
 fs/utimes.c                 |  197 ++++++++++++++++++++++---------------
 fs/xattr.c                  |   85 ++++++++--------
 include/linux/fs.h          |   21 +++--
 include/linux/reiserfs_fs.h |    1 +
 include/linux/time.h        |    5 +-
 include/linux/xattr.h       |    5 +-
 ipc/mqueue.c                |   16 ++-
 kernel/cgroup.c             |    5 +-
 mm/filemap.c                |    2 +-
 mm/tiny-shmem.c             |    2 +-
 net/unix/af_unix.c          |    6 +-
 25 files changed, 587 insertions(+), 499 deletions(-)

Miklos Szeredi (25):
      ecryptfs: clean up (un)lock_parent
      nfsd: clean up mnt_want_write calls
      cgroup: don't call vfs_mkdir
      reiserfs: don't call vfs_rmdir
      reiserfs: don't call notify_change
      sysfs: don't call notify_change
      hpfs: don't call notify_change
      fat: don't call notify_change
      vfs: immutable inode checking cleanup
      vfs: truncate: don't check immutable twice
      vfs: truncate: append-only checking cleanup
      vfs: create file_truncate() helper
      vfs: utimes immutable fix
      vfs: utimes cleanup
      vfs: splice remove_suid() cleanup
      vfs: add path_create() and path_mknod()
      vfs: add path_mkdir()
      vfs: add path_rmdir()
      vfs: add path_unlink()
      vfs: add path_symlink()
      vfs: add path_link()
      vfs: add path_rename()
      vfs: add path_setattr()
      vfs: add path_setxattr()
      vfs: add path_removexattr()


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 06/24] hpfs: dont call notify_change
  2008-05-06  9:13 ` [patch 06/24] hpfs: " Miklos Szeredi
@ 2008-05-08  0:41   ` Mikulas Patocka
  2008-05-08 16:52   ` Christoph Hellwig
  1 sibling, 0 replies; 45+ messages in thread
From: Mikulas Patocka @ 2008-05-08  0:41 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: akpm, hch, viro, linux-fsdevel, linux-kernel

> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> hpfs_unlink() calls notify_change() to truncate the file before
> deleting.  Replace with explicit call to hpfs_notify_change().
> 
> This is equivalent, except that:
>  - security_inode_setattr() is not called before hpfs_notify_change()
>  - fsnotify_change() is not called after hpfs_notify_change()
> 
> The truncation is just an implementation detail, so both the security
> check and the notification are unnecessary.
> 
> Possibly even the ctime modification is wrong?

OK, commit the patch.

Signed-off-by: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>

Mikulas

> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> CC: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
> ---
>  fs/hpfs/namei.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/fs/hpfs/namei.c
> ===================================================================
> --- linux-2.6.orig/fs/hpfs/namei.c	2008-05-06 11:04:29.000000000 +0200
> +++ linux-2.6/fs/hpfs/namei.c	2008-05-06 11:04:34.000000000 +0200
> @@ -426,7 +426,8 @@ again:
>  			/*printk("HPFS: truncating file before delete.\n");*/
>  			newattrs.ia_size = 0;
>  			newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
> -			err = notify_change(dentry, &newattrs);
> +			newattrs.ia_ctime = current_fs_time(inode->i_sb);
> +			err = hpfs_notify_change(dentry, &newattrs);
>  			put_write_access(inode);
>  			if (!err)
>  				goto again;
> 
> --
> 

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 04/24] reiserfs: dont call notify_change
  2008-05-06  9:13 ` [patch 04/24] reiserfs: dont call notify_change Miklos Szeredi
@ 2008-05-08 16:49   ` Christoph Hellwig
  0 siblings, 0 replies; 45+ messages in thread
From: Christoph Hellwig @ 2008-05-08 16:49 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: akpm, hch, viro, linux-fsdevel, linux-kernel, Jeff Mahoney

On Tue, May 06, 2008 at 11:13:31AM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Reiserfs calls notify_change() on it's private extended attribute
> files to resize and change ownership of these files.  Replace with
> reiserfs_setattr().
> 
> This is equivalent, except that:
> 
>  - i_alloc_sem locking is not performed around reiserfs_setattr() when
>    the size is changed
>  - security_inode_setattr() is not called before reiserfs_setattr()
>  - fsnotify_change() is not called after reiserfs_setattr()
> 
> None of the above is necessary, since the files are private to
> reiserfs and inaccessible from userspace.
> 
> Also remove setting of ctime on the xattr files.

I think splitting out the ATTR_SIZE case of reiserfs_setattr into
a separate helper would be better.  That would a) avoid the useless
c and mtime updates on the attribue inode and b) clean up
reiserfs_setattr a little bit.


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 05/24] sysfs: dont call notify_change
  2008-05-06  9:13 ` [patch 05/24] sysfs: " Miklos Szeredi
@ 2008-05-08 16:50   ` Christoph Hellwig
  0 siblings, 0 replies; 45+ messages in thread
From: Christoph Hellwig @ 2008-05-08 16:50 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: akpm, hch, viro, linux-fsdevel, linux-kernel, Greg Kroah-Hartman

On Tue, May 06, 2008 at 11:13:32AM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> sysfs_chmod_file() calls notify_change() to change the permission bits
> on a sysfs file.  Replace with explicit call to sysfs_setattr() and
> fsnotify_change().
> 
> This is equivalent, except that security_inode_setattr() is not
> called.  This function is called by drivers, so the security checks do
> not make any sense.

Looks correct to me, although I really don't see the point for it.


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 06/24] hpfs: dont call notify_change
  2008-05-06  9:13 ` [patch 06/24] hpfs: " Miklos Szeredi
  2008-05-08  0:41   ` Mikulas Patocka
@ 2008-05-08 16:52   ` Christoph Hellwig
  2008-05-09  1:59     ` Mikulas Patocka
  1 sibling, 1 reply; 45+ messages in thread
From: Christoph Hellwig @ 2008-05-08 16:52 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: akpm, hch, viro, linux-fsdevel, linux-kernel, Mikulas Patocka

On Tue, May 06, 2008 at 11:13:33AM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> hpfs_unlink() calls notify_change() to truncate the file before
> deleting.  Replace with explicit call to hpfs_notify_change().
> 
> This is equivalent, except that:
>  - security_inode_setattr() is not called before hpfs_notify_change()
>  - fsnotify_change() is not called after hpfs_notify_change()
> 
> The truncation is just an implementation detail, so both the security
> check and the notification are unnecessary.
> 
> Possibly even the ctime modification is wrong?

This code is rahter scary, as we'd lost the content without the file
when the second remove_dirent attempt fails.  Because of that we should
at least keep the ctime change so an app can know the file was touched.

Again, looks correct but I'm not convinced about all these changes.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 07/24] fat: dont call notify_change
  2008-05-06  9:13 ` [patch 07/24] fat: " Miklos Szeredi
@ 2008-05-08 16:57   ` Christoph Hellwig
  2008-05-08 18:51     ` Miklos Szeredi
                       ` (2 more replies)
  0 siblings, 3 replies; 45+ messages in thread
From: Christoph Hellwig @ 2008-05-08 16:57 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: akpm, hch, viro, linux-fsdevel, linux-kernel, OGAWA Hirofumi,
	sds, jmorris, casey

On Tue, May 06, 2008 at 11:13:34AM +0200, Miklos Szeredi wrote:
> 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 call to fat_setattr().
> 
> This is equivalent, except that security_inode_setattr() is not called
> before fat_setattr().  I think this is not needed, since the mode
> change is just a side effect of the attribute change.

Actually I think we want the security_inode_setattr.  This is an
implicit chmode when switching the ATTR_RO flag on and off and we should
have the full security check for it.  Then again I'm not sure the
security modules care about this level of detail because there's
probably even worse ioctl hidden somewhere.

Ccing the Selinux guys and Casey in case they care.


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 07/24] fat: dont call notify_change
  2008-05-08 16:57   ` Christoph Hellwig
@ 2008-05-08 18:51     ` Miklos Szeredi
  2008-05-08 19:42     ` OGAWA Hirofumi
  2008-05-08 23:45     ` James Morris
  2 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-08 18:51 UTC (permalink / raw)
  To: hch
  Cc: miklos, akpm, hch, viro, linux-fsdevel, linux-kernel, hirofumi,
	sds, jmorris, casey

> > 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 call to fat_setattr().
> > 
> > This is equivalent, except that security_inode_setattr() is not called
> > before fat_setattr().  I think this is not needed, since the mode
> > change is just a side effect of the attribute change.
> 
> Actually I think we want the security_inode_setattr.  This is an
> implicit chmode when switching the ATTR_RO flag on and off and we should
> have the full security check for it.

Yes, I'm a bit uncertain about this.  It's a user action that triggers
the mode change.  On the other hand it's just a side effect of the
ioctl.  I can re-add the security check just to be on the safe side,
but probably nobody cares.

Miklos

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 07/24] fat: dont call notify_change
  2008-05-08 16:57   ` Christoph Hellwig
  2008-05-08 18:51     ` Miklos Szeredi
@ 2008-05-08 19:42     ` OGAWA Hirofumi
  2008-05-08 23:45     ` James Morris
  2 siblings, 0 replies; 45+ messages in thread
From: OGAWA Hirofumi @ 2008-05-08 19:42 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Miklos Szeredi, akpm, viro, linux-fsdevel, linux-kernel, sds,
	jmorris, casey

Christoph Hellwig <hch@infradead.org> writes:

> Actually I think we want the security_inode_setattr.  This is an
> implicit chmode when switching the ATTR_RO flag on and off and we should
> have the full security check for it.  Then again I'm not sure the
> security modules care about this level of detail because there's
> probably even worse ioctl hidden somewhere.
>
> Ccing the Selinux guys and Casey in case they care.

Sounds good.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 07/24] fat: dont call notify_change
  2008-05-08 16:57   ` Christoph Hellwig
  2008-05-08 18:51     ` Miklos Szeredi
  2008-05-08 19:42     ` OGAWA Hirofumi
@ 2008-05-08 23:45     ` James Morris
  2 siblings, 0 replies; 45+ messages in thread
From: James Morris @ 2008-05-08 23:45 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Miklos Szeredi, akpm, viro, linux-fsdevel, linux-kernel,
	OGAWA Hirofumi, sds, casey

On Thu, 8 May 2008, Christoph Hellwig wrote:

> On Tue, May 06, 2008 at 11:13:34AM +0200, Miklos Szeredi wrote:
> > 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 call to fat_setattr().
> > 
> > This is equivalent, except that security_inode_setattr() is not called
> > before fat_setattr().  I think this is not needed, since the mode
> > change is just a side effect of the attribute change.
> 
> Actually I think we want the security_inode_setattr.  This is an
> implicit chmode when switching the ATTR_RO flag on and off and we should
> have the full security check for it.  Then again I'm not sure the
> security modules care about this level of detail because there's
> probably even worse ioctl hidden somewhere.
> 
> Ccing the Selinux guys and Casey in case they care.
> 

I don't know of any situation where we'd have policy differentating the 
ioctl check from setattr for FAT (or any filesystem).


- James
-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 06/24] hpfs: dont call notify_change
  2008-05-08 16:52   ` Christoph Hellwig
@ 2008-05-09  1:59     ` Mikulas Patocka
  0 siblings, 0 replies; 45+ messages in thread
From: Mikulas Patocka @ 2008-05-09  1:59 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Miklos Szeredi, akpm, viro, linux-fsdevel, linux-kernel

On Thu, 8 May 2008, Christoph Hellwig wrote:

> On Tue, May 06, 2008 at 11:13:33AM +0200, Miklos Szeredi wrote:
> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > hpfs_unlink() calls notify_change() to truncate the file before
> > deleting.  Replace with explicit call to hpfs_notify_change().
> > 
> > This is equivalent, except that:
> >  - security_inode_setattr() is not called before hpfs_notify_change()
> >  - fsnotify_change() is not called after hpfs_notify_change()
> > 
> > The truncation is just an implementation detail, so both the security
> > check and the notification are unnecessary.
> > 
> > Possibly even the ctime modification is wrong?
> 
> This code is rahter scary, as we'd lost the content without the file
> when the second remove_dirent attempt fails.  Because of that we should
> at least keep the ctime change so an app can know the file was touched.

Unfortunatelly this is design bug in HPFS --- removing a dirent can 
allocate more space. There's nothing that can be done about it. OS/2 
crashes on panic when this situation is triggered :)

Mikulas

> Again, looks correct but I'm not convinced about all these changes.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts
  2008-05-06 10:33   ` Miklos Szeredi
@ 2008-05-14 14:55     ` Pavel Machek
  0 siblings, 0 replies; 45+ messages in thread
From: Pavel Machek @ 2008-05-14 14:55 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: hch, akpm, viro, linux-fsdevel, linux-kernel

Hi!

> > > Here it is again as a single series (without the ecryptfs one already
> > > merged).
> > > 
> > > I've addressed Christoph's comments.
> > 
> > And totally ignored Al's while merging the second rejected series into
> > the first one.
> 
> Al doesn't like it but everybody else does.  His excuses are very
> hollow and hypocritical, just take this for example: "Except that it

Ok, but perhaps you should have mentioned that in the changelog at the
beggining of series?

'I addressed Christoph's comments, but I believe Al is wrong'...?


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 08/24] vfs: immutable inode checking cleanup
  2008-05-06  9:13 ` [patch 08/24] vfs: immutable inode checking cleanup Miklos Szeredi
@ 2008-05-15  7:23   ` Al Viro
  2008-05-15  7:39     ` Miklos Szeredi
  0 siblings, 1 reply; 45+ messages in thread
From: Al Viro @ 2008-05-15  7:23 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: akpm, hch, linux-fsdevel, linux-kernel

> --- linux-2.6.orig/fs/attr.c	2008-05-06 11:04:29.000000000 +0200
> +++ linux-2.6/fs/attr.c	2008-05-06 11:04:35.000000000 +0200
> @@ -108,6 +108,12 @@ 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_ATIME_SET | ATTR_MTIME_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/utimes.c
> ===================================================================
> --- linux-2.6.orig/fs/utimes.c	2008-05-06 11:04:29.000000000 +0200
> +++ linux-2.6/fs/utimes.c	2008-05-06 11:04:35.000000000 +0200
> @@ -105,10 +105,6 @@ long do_utimes(int dfd, char __user *fil
>  	/* Don't worry, the checks are done in inode_change_ok() */
>  	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) {

Erm...  What happens if we have UTIME_NOW on both and inode is marked
immutable?  AFAICS, you've just switched from -EPERM to -EACCES.
For append-only it's even more interesting - you go from -EPERM to
success.

It might or might not be the right thing to do, and the syscall is
new, but...  Do you have an ACK from Ulrich?

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 13/24] vfs: utimes cleanup
  2008-05-06 10:03   ` Miklos Szeredi
@ 2008-05-15  7:39     ` Al Viro
  2008-05-15  7:45       ` Miklos Szeredi
  0 siblings, 1 reply; 45+ messages in thread
From: Al Viro @ 2008-05-15  7:39 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: akpm, hch, linux-fsdevel, linux-kernel, drepper

On Tue, May 06, 2008 at 12:03:46PM +0200, Miklos Szeredi wrote:
> + * do_futimesat - change times on filename

> +int do_utimes(int dfd, char __user *filename, struct timespec *times, int flags)

Erm...  Is there any reason to
	* have do_utimes() used anywhere
	* _not_ have do_futimesat() called "do_utimes"?

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 08/24] vfs: immutable inode checking cleanup
  2008-05-15  7:23   ` Al Viro
@ 2008-05-15  7:39     ` Miklos Szeredi
  0 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-15  7:39 UTC (permalink / raw)
  To: viro; +Cc: miklos, akpm, hch, linux-fsdevel, linux-kernel

> > --- linux-2.6.orig/fs/attr.c	2008-05-06 11:04:29.000000000 +0200
> > +++ linux-2.6/fs/attr.c	2008-05-06 11:04:35.000000000 +0200
> > @@ -108,6 +108,12 @@ 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_ATIME_SET | ATTR_MTIME_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/utimes.c
> > ===================================================================
> > --- linux-2.6.orig/fs/utimes.c	2008-05-06 11:04:29.000000000 +0200
> > +++ linux-2.6/fs/utimes.c	2008-05-06 11:04:35.000000000 +0200
> > @@ -105,10 +105,6 @@ long do_utimes(int dfd, char __user *fil
> >  	/* Don't worry, the checks are done in inode_change_ok() */
> >  	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) {
> 
> Erm...  What happens if we have UTIME_NOW on both and inode is marked
> immutable?  AFAICS, you've just switched from -EPERM to -EACCES.
> For append-only it's even more interesting - you go from -EPERM to
> success.

Yup.  These are defensible on the ground that times == NULL should be
equivalent to UTIME_NOW on both.  There's really no reason to be
stricter on the latter case.

> It might or might not be the right thing to do, and the syscall is
> new, but...  Do you have an ACK from Ulrich?

No.  I could CC him on these, but he's not even acking my security
patches on utimesat, so I don't have high hopes.

Miklos

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 13/24] vfs: utimes cleanup
  2008-05-15  7:39     ` Al Viro
@ 2008-05-15  7:45       ` Miklos Szeredi
  0 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-15  7:45 UTC (permalink / raw)
  To: viro; +Cc: miklos, akpm, hch, linux-fsdevel, linux-kernel, drepper

> > + * do_futimesat - change times on filename
> 
> > +int do_utimes(int dfd, char __user *filename, struct timespec *times, int flags)
> 
> Erm...  Is there any reason to
> 	* have do_utimes() used anywhere
> 	* _not_ have do_futimesat() called "do_utimes"?

Review ping-pong.  My original patch did that, and Christoph didn't
like it.  I don't care that much actually, one thing I like better in
the second version is to have a separate function for the path lookup
thing (now called do_utimes(), but it could be renamed to
name_utimes() or whatever).

Miklos

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 10/24] vfs: truncate: append-only checking cleanup
  2008-05-06  9:13 ` [patch 10/24] vfs: truncate: append-only checking cleanup Miklos Szeredi
@ 2008-05-15  7:49   ` Al Viro
  2008-05-15  7:58     ` Miklos Szeredi
  0 siblings, 1 reply; 45+ messages in thread
From: Al Viro @ 2008-05-15  7:49 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: akpm, hch, linux-fsdevel, linux-kernel

On Tue, May 06, 2008 at 11:13:37AM +0200, Miklos Szeredi wrote:
> -	error = -EPERM;
> -	if (IS_APPEND(inode))
> -		goto mnt_drop_write_and_out;
> -
>  	error = get_write_access(inode);
>  	if (error)
>  		goto mnt_drop_write_and_out;

errno change and worse than that - we
	a) get different errno with append-only file mmaped shared
	b) break_lease() happening, despite the file being append-only

> @@ -321,10 +317,6 @@ static long do_sys_ftruncate(unsigned in
>  	if (small && length > MAX_NON_LFS)
>  		goto out_putf;
>  
> -	error = -EPERM;
> -	if (IS_APPEND(inode))
> -		goto out_putf;
> -
>  	error = locks_verify_truncate(inode, file, length);

Again, errno change...

> @@ -1639,8 +1639,6 @@ int may_open(struct nameidata *nd, int a
>  	if (IS_APPEND(inode)) {
>  		if  ((flag & FMODE_WRITE) && !(flag & O_APPEND))
>  			return -EPERM;
> -		if (flag & O_TRUNC)
> -			return -EPERM;
>  	}

Bogus break_lease() following that...

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [patch 10/24] vfs: truncate: append-only checking cleanup
  2008-05-15  7:49   ` Al Viro
@ 2008-05-15  7:58     ` Miklos Szeredi
  0 siblings, 0 replies; 45+ messages in thread
From: Miklos Szeredi @ 2008-05-15  7:58 UTC (permalink / raw)
  To: viro; +Cc: miklos, akpm, hch, linux-fsdevel, linux-kernel

> > -	error = -EPERM;
> > -	if (IS_APPEND(inode))
> > -		goto mnt_drop_write_and_out;
> > -
> >  	error = get_write_access(inode);
> >  	if (error)
> >  		goto mnt_drop_write_and_out;
> 
> errno change and worse than that - we
> 	a) get different errno with append-only file mmaped shared
> 	b) break_lease() happening, despite the file being append-only
> 
> > @@ -321,10 +317,6 @@ static long do_sys_ftruncate(unsigned in
> >  	if (small && length > MAX_NON_LFS)
> >  		goto out_putf;
> >  
> > -	error = -EPERM;
> > -	if (IS_APPEND(inode))
> > -		goto out_putf;
> > -
> >  	error = locks_verify_truncate(inode, file, length);
> 
> Again, errno change...
> 
> > @@ -1639,8 +1639,6 @@ int may_open(struct nameidata *nd, int a
> >  	if (IS_APPEND(inode)) {
> >  		if  ((flag & FMODE_WRITE) && !(flag & O_APPEND))
> >  			return -EPERM;
> > -		if (flag & O_TRUNC)
> > -			return -EPERM;
> >  	}
> 
> Bogus break_lease() following that...
> 

OK, I'll drop this patch.  Truncate is weird, it should really have a
separate i_op/f_op.

Miklos

^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2008-05-15  7:58 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-05-06  9:13 [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Miklos Szeredi
2008-05-06  9:13 ` [patch 01/24] nfsd: clean up mnt_want_write calls Miklos Szeredi
2008-05-06  9:13 ` [patch 02/24] cgroup: dont call vfs_mkdir Miklos Szeredi
2008-05-06  9:13 ` [patch 03/24] reiserfs: dont call vfs_rmdir Miklos Szeredi
2008-05-06  9:13 ` [patch 04/24] reiserfs: dont call notify_change Miklos Szeredi
2008-05-08 16:49   ` Christoph Hellwig
2008-05-06  9:13 ` [patch 05/24] sysfs: " Miklos Szeredi
2008-05-08 16:50   ` Christoph Hellwig
2008-05-06  9:13 ` [patch 06/24] hpfs: " Miklos Szeredi
2008-05-08  0:41   ` Mikulas Patocka
2008-05-08 16:52   ` Christoph Hellwig
2008-05-09  1:59     ` Mikulas Patocka
2008-05-06  9:13 ` [patch 07/24] fat: " Miklos Szeredi
2008-05-08 16:57   ` Christoph Hellwig
2008-05-08 18:51     ` Miklos Szeredi
2008-05-08 19:42     ` OGAWA Hirofumi
2008-05-08 23:45     ` James Morris
2008-05-06  9:13 ` [patch 08/24] vfs: immutable inode checking cleanup Miklos Szeredi
2008-05-15  7:23   ` Al Viro
2008-05-15  7:39     ` Miklos Szeredi
2008-05-06  9:13 ` [patch 09/24] vfs: truncate: dont check immutable twice Miklos Szeredi
2008-05-06  9:13 ` [patch 10/24] vfs: truncate: append-only checking cleanup Miklos Szeredi
2008-05-15  7:49   ` Al Viro
2008-05-15  7:58     ` Miklos Szeredi
2008-05-06  9:13 ` [patch 11/24] vfs: create file_truncate() helper Miklos Szeredi
2008-05-06  9:13 ` [patch 12/24] vfs: utimes immutable fix Miklos Szeredi
2008-05-06  9:13 ` [patch 13/24] vfs: utimes cleanup Miklos Szeredi
2008-05-06 10:03   ` Miklos Szeredi
2008-05-15  7:39     ` Al Viro
2008-05-15  7:45       ` Miklos Szeredi
2008-05-06  9:13 ` [patch 14/24] vfs: splice remove_suid() cleanup Miklos Szeredi
2008-05-06  9:13 ` [patch 15/24] vfs: add path_create() and path_mknod() Miklos Szeredi
2008-05-06  9:13 ` [patch 16/24] vfs: add path_mkdir() Miklos Szeredi
2008-05-06  9:13 ` [patch 17/24] vfs: add path_rmdir() Miklos Szeredi
2008-05-06  9:13 ` [patch 18/24] vfs: add path_unlink() Miklos Szeredi
2008-05-06  9:13 ` [patch 19/24] vfs: add path_symlink() Miklos Szeredi
2008-05-06  9:13 ` [patch 20/24] vfs: add path_link() Miklos Szeredi
2008-05-06  9:13 ` [patch 21/24] vfs: add path_rename() Miklos Szeredi
2008-05-06  9:13 ` [patch 22/24] vfs: add path_setattr() Miklos Szeredi
2008-05-06  9:13 ` [patch 23/24] vfs: add path_setxattr() Miklos Szeredi
2008-05-06  9:13 ` [patch 24/24] vfs: add path_removexattr() Miklos Szeredi
2008-05-06  9:16 ` [patch 00/24] vfs: fixes and cleanups + add helpers to check r/o bind mounts Christoph Hellwig
2008-05-06 10:33   ` Miklos Szeredi
2008-05-14 14:55     ` Pavel Machek
2008-05-07 11:57 ` Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).