[PATCH] vfs: reorganize inode chattr checks

* [PATCH] vfs: reorganize inode chattr checks
@ 2010-03-17 17:09 Dmitry Monakhov
  2010-03-17 17:18 ` Dmitry Monakhov
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Dmitry Monakhov @ 2010-03-17 17:09 UTC (permalink / raw)
  To: linux-fsdevel, Al Viro, Nick Piggin

[-- Attachment #1: Type: text/plain, Size: 319 bytes --]


This patch fix nasty bug with truncate on swapfile.
It contains only vfs core helpers and fix isize check path for
fs w/o i_op->setattr method. Later i'll send corresponding changes
to other fs which has ->setattr method.

This patch is depends on Nick's patch
http://marc.info/?l=linux-fsdevel&m=126752788514574&w=2


[-- Attachment #2: 0001-vfs-reorganize-inode-chattr-checks.patch --]
[-- Type: text/plain, Size: 4735 bytes --]

>From e8587585b2e2fefb75d47f8d50ddc7099a50873c Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Wed, 17 Mar 2010 19:49:22 +0300
Subject: [PATCH] vfs: reorganize inode chattr checks

vmtrucate may fail due to IS_SWAPFILE flag or due to RLIMIT_FSIZE.
In some situations it is not acceptable behaviour. Off course
we can check IS_SWAPFILE before but what about RLIMIT_FSIZE?

Let's divide newsize seting logic in two parts
 First which perform necessery size checks
 Second which does the work and can not fail.
And perform size check before any inode modifications.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/attr.c          |   45 ++++++++++++++++++++++++++++++++++-----------
 fs/libfs.c         |    8 ++++----
 include/linux/fs.h |    5 +++--
 3 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 1bca479..1abaa05 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -18,7 +18,7 @@
 /* Taken over from the old code... */
 
 /* POSIX UID/GID verification for setting inode attributes. */
-int inode_change_ok(const struct inode *inode, struct iattr *attr)
+int inode_change_posix_ok(const struct inode *inode, struct iattr *attr)
 {
 	int retval = -EPERM;
 	unsigned int ia_valid = attr->ia_valid;
@@ -60,7 +60,7 @@ fine:
 error:
 	return retval;
 }
-EXPORT_SYMBOL(inode_change_ok);
+EXPORT_SYMBOL(inode_change_posix_ok);
 
 /**
  * inode_newsize_ok - may this inode be truncated to a given size
@@ -105,6 +105,19 @@ out_big:
 }
 EXPORT_SYMBOL(inode_newsize_ok);
 
+/* Generic verification for setting inode attributes. */
+int inode_change_attr_ok(const struct inode *inode, struct iattr *attr)
+{
+	int ret;
+	ret = inode_change_posix_ok(inode, attr);
+	if (ret)
+		return ret;
+	if (attr->ia_valid & ATTR_SIZE)
+		ret = inode_newsize_ok(inode, attr->ia_size);
+	return ret;
+}
+EXPORT_SYMBOL(inode_change_attr_ok);
+
 /**
  * generic_setattr - copy simple metadata updates into the generic inode
  * @inode:	the inode to be updated
@@ -241,17 +254,27 @@ int notify_change(struct dentry * dentry, struct iattr * attr)
 	if (inode->i_op && inode->i_op->setattr) {
 		error = inode->i_op->setattr(dentry, attr);
 	} else {
-		error = inode_change_ok(inode, attr);
-		if (!error) {
-			if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
-			    (ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid))
-				error = vfs_dq_transfer(inode, attr) ?
-					-EDQUOT : 0;
-			if (!error)
-				error = inode_setattr(inode, attr);
+		loff_t oldsize = inode->i_size;
+		error = inode_change_attr_ok(inode, attr);
+		if (error)
+			goto out;
+		if ((ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
+			(ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) {
+			error = vfs_dq_transfer(inode, attr) ?	-EDQUOT : 0;
+			if (error)
+				goto out;
+		}
+		if (attr->ia_valid & ATTR_SIZE &&
+			oldsize != attr->ia_size) {
+			i_size_write(inode, attr->ia_size);
+			truncate_pagecache(inode, oldsize, attr->ia_size);
+			if (inode->i_op->truncate)
+				inode->i_op->truncate(inode);
 		}
+		generic_setattr(inode, attr);
+		mark_inode_dirty(inode);
 	}
-
+out:
 	if (ia_valid & ATTR_SIZE)
 		up_write(&dentry->d_inode->i_alloc_sem);
 
diff --git a/fs/libfs.c b/fs/libfs.c
index a76934d..98f9a40 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -389,7 +389,7 @@ int simple_setattr(struct dentry *dentry, struct iattr *iattr)
 	struct inode *inode = dentry->d_inode;
 	int error;
 
-	error = inode_change_ok(inode, iattr);
+	error = inode_change_attr_ok(inode, iattr);
 	if (error)
 		return error;
 
@@ -401,9 +401,9 @@ int simple_setattr(struct dentry *dentry, struct iattr *iattr)
 	}
 
 	if (iattr->ia_valid & ATTR_SIZE) {
-		error = simple_setsize(inode, iattr->ia_size);
-		if (error)
-			return error;
+		loff_t oldsize = inode->i_size;
+		i_size_write(inode, iattr->ia_size);
+		truncate_pagecache(inode, oldsize, iattr->ia_size);
 	}
 
 	generic_setattr(inode, iattr);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 81c91d9..fb6809d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2388,9 +2388,10 @@ extern int buffer_migrate_page(struct address_space *,
 #else
 #define buffer_migrate_page NULL
 #endif
-
-extern int inode_change_ok(const struct inode *, struct iattr *);
+#define inode_change_ok(inode, iattr) inode_change_posix_ok(inode, iattr)
+extern int inode_change_posix_ok(const struct inode *, struct iattr *);
 extern int inode_newsize_ok(const struct inode *, loff_t offset);
+extern int inode_change_attr_ok(const struct inode *, struct iattr *);
 extern int __must_check inode_setattr(struct inode *, const struct iattr *);
 extern void generic_setattr(struct inode *inode, const struct iattr *attr);
 
-- 
1.6.6.1


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