vfs: truncate bug fixes

PATCH Superseded Dmitry Kasatkin 2 participants 5 messages Dec 22, 2011

Subsystems (2)

VFS FUSE filesystem core

lore

Share & tools

lore

Dmitry Kasatkin

author

2011-12-22 08:55 UTC #0 lore raw

v2 cover vfs: truncate bug fixes

When a file is opened with O_TRUNC or truncated with truncate()/ftruncate(), and then closed, iversion is not updated. This patch uses ATTR_OPEN flag, which is currently used to identify files opened with O_TRUNC, as an indication to increment iversion.
Also it adds passing of this flag from do_sys_truncate and do_sys_ftruncate. To reflect real meaning, ATTR_OPEN is suggested to be renamed to ATTR_TRUNC.

Dmitry Kasatkin (2):
vfs: iversion truncate bug fix
vfs: ATTR_OPEN rename

 fs/attr.c          |    3 +++
 fs/fuse/dir.c      |    4 ++--
 fs/namei.c         |    2 +-
 fs/open.c          |    5 +++--
 include/linux/fs.h |    2 +-
 5 files changed, 10 insertions(+), 6 deletions(-)

Signature · 1 line

1.7.4.1

Dmitry Kasatkin

author

2011-12-22 08:55 UTC #1 lore raw

PATCH v2 1/2 vfs: iversion truncate bug fix

Signed-off-by Dmitry Kasatkin <[email protected]>

---
 fs/attr.c |    3 +++
 fs/open.c |    5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

fs/attr.c +3 -0

index 7ee7ba4..e02370d 100644
@@ -176,6 +176,9 @@ int notify_change(struct dentry * dentry, struct iattr * attr)
 			return -EPERM;
 	}
 
+	if ((ia_valid & ATTR_OPEN) && IS_I_VERSION(inode))
+		inode_inc_iversion(inode);
+
 	if ((ia_valid & ATTR_MODE)) {
 		mode_t amode = attr->ia_mode;
 		/* Flag setting protected by i_mutex */

fs/open.c +3 -2

index 22c41b5..4ee6847 100644
@@ -113,7 +113,7 @@ static long do_sys_truncate(const char __user *pathname, loff_t length)
 	if (!error)
 		error = security_path_truncate(&path);
 	if (!error)
-		error = do_truncate(path.dentry, length, 0, NULL);
+		error = do_truncate(path.dentry, length, ATTR_OPEN, NULL);
 
 put_write_and_out:
 	put_write_access(inode);

@@ -168,7 +168,8 @@ static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
 	if (!error)
 		error = security_path_truncate(&file->f_path);
 	if (!error)
-		error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
+		error = do_truncate(dentry, length,
+				    ATTR_MTIME|ATTR_CTIME|ATTR_OPEN, file);
 out_putf:
 	fput(file);
 out:

Signature · 1 line

1.7.4.1

Dmitry Kasatkin

author

2011-12-22 08:55 UTC #2 lore raw

PATCH v2 2/2 vfs: ATTR_OPEN rename

In reply to Dmitry Kasatkin

ATTR_OPEN is used to indicate truncate situation.
The correct name would be ATTR_TRUNC.

Signed-off-by Dmitry Kasatkin <[email protected]>

---
 fs/attr.c          |    2 +-
 fs/fuse/dir.c      |    4 ++--
 fs/namei.c         |    2 +-
 fs/open.c          |    4 ++--
 include/linux/fs.h |    2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

fs/attr.c +1 -1

index e02370d..8e1e27d 100644
@@ -176,7 +176,7 @@ int notify_change(struct dentry * dentry, struct iattr * attr)
 			return -EPERM;
 	}
 
-	if ((ia_valid & ATTR_OPEN) && IS_I_VERSION(inode))
+	if ((ia_valid & ATTR_TRUNC) && IS_I_VERSION(inode))
 		inode_inc_iversion(inode);
 
 	if ((ia_valid & ATTR_MODE)) {

fs/fuse/dir.c +2 -2

index 9f63e49..006cbc8 100644
@@ -1189,7 +1189,7 @@ static bool update_mtime(unsigned ivalid)
 		return true;
 
 	/* If it's an open(O_TRUNC) or an ftruncate(), don't update */
-	if ((ivalid & ATTR_SIZE) && (ivalid & (ATTR_OPEN | ATTR_FILE)))
+	if ((ivalid & ATTR_SIZE) && (ivalid & (ATTR_TRUNC | ATTR_FILE)))
 		return false;
 
 	/* In all other cases update */

@@ -1298,7 +1298,7 @@ static int fuse_do_setattr(struct dentry *entry, struct iattr *attr,
 	if (err)
 		return err;
 
-	if (attr->ia_valid & ATTR_OPEN) {
+	if (attr->ia_valid & ATTR_TRUNC) {
 		if (fc->atomic_o_trunc)
 			return 0;
 		file = NULL;

fs/namei.c +1 -1

index 5008f01..40fd610 100644
@@ -2064,7 +2064,7 @@ static int handle_truncate(struct file *filp)
 		error = security_path_truncate(path);
 	if (!error) {
 		error = do_truncate(path->dentry, 0,
-				    ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
+				    ATTR_MTIME|ATTR_CTIME|ATTR_TRUNC,
 				    filp);
 	}
 	put_write_access(inode);

fs/open.c +2 -2

index 4ee6847..989fad1 100644
@@ -113,7 +113,7 @@ static long do_sys_truncate(const char __user *pathname, loff_t length)
 	if (!error)
 		error = security_path_truncate(&path);
 	if (!error)
-		error = do_truncate(path.dentry, length, ATTR_OPEN, NULL);
+		error = do_truncate(path.dentry, length, ATTR_TRUNC, NULL);
 
 put_write_and_out:
 	put_write_access(inode);

@@ -169,7 +169,7 @@ static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
 		error = security_path_truncate(&file->f_path);
 	if (!error)
 		error = do_truncate(dentry, length,
-				    ATTR_MTIME|ATTR_CTIME|ATTR_OPEN, file);
+				    ATTR_MTIME|ATTR_CTIME|ATTR_TRUNC, file);
 out_putf:
 	fput(file);
 out:

include/linux/fs.h +1 -1

index e0bc4ff..7548356 100644
@@ -447,7 +447,7 @@ typedef void (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define ATTR_KILL_SGID	(1 << 12)
 #define ATTR_FILE	(1 << 13)
 #define ATTR_KILL_PRIV	(1 << 14)
-#define ATTR_OPEN	(1 << 15) /* Truncating from open(O_TRUNC) */
+#define ATTR_TRUNC	(1 << 15) /* Truncating from open(O_TRUNC) */
 #define ATTR_TIMES_SET	(1 << 16)
 
 /*

Signature · 1 line

1.7.4.1

Al Viro

maintainer

2011-12-22 09:10 UTC #3 lore raw

In reply to Dmitry Kasatkin

Quoted 6 lines · Dmitry Kasatkin

When a file is opened with O_TRUNC or truncated with truncate()/ftruncate(), and then closed, iversion is not updated. This patch uses ATTR_OPEN flag, which is currently used to identify files opened with O_TRUNC, as an indication to increment iversion.
Also it adds passing of this flag from do_sys_truncate and do_sys_ftruncate. To reflect real meaning, ATTR_OPEN is suggested to be renamed to ATTR_TRUNC.

Um... Is there any difference left between that and ATTR_SIZE, then?

Kasatkin, Dmitry

author

2011-12-22 10:29 UTC #4 lore raw

Quoted 9 lines · Al Viro

Quoted 6 lines · Dmitry Kasatkin
When a file is opened with O_TRUNC or truncated with truncate()/ftruncate(), and then closed, iversion is not updated. This patch uses ATTR_OPEN flag, which is currently used to identify files opened with O_TRUNC, as an indication to increment iversion.
Also it adds passing of this flag from do_sys_truncate and do_sys_ftruncate. To reflect real meaning, ATTR_OPEN is suggested to be renamed to ATTR_TRUNC.
Um... Is there any difference left between that and ATTR_SIZE, then?

Oops. Indeed. It becomes simpler..

Comments and using ATTR_OPEN in fs/fuse/dir.c confused me.

update_mtime()

{
	/* If it's an open(O_TRUNC) or an ftruncate(), don't update */
	if ((ivalid & ATTR_SIZE) && (ivalid & (ATTR_OPEN | ATTR_FILE)))
}

do_sys_truncate() calls do_truncate(path.dentry, length, 0, NULL); while do_sys_ftruncate calls do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);

Notice the last file parameter... NULL or file..

As truncate and ftruncate syscalls should do the same job as fstat or stat. It will lead to different results for FUSE.
Following will be 0: (ivalid & (ATTR_OPEN | ATTR_FILE)

FUSE comments says.. OR
/* If it's an open(O_TRUNC) or an ftruncate(), don't update */

Will make new version..

- Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html