| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1188786059299-git-send-email-jsipek@cs.sunysb.edu>
Date: Sun, 2 Sep 2007 22:20:55 -0400
From: "Josef 'Jeff' Sipek" <jsipek@...sunysb.edu>
To: akpm@...ux-foundation.org
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
hch@...radead.org, viro@....linux.org.uk,
bharata@...ux.vnet.ibm.com, j.blunck@...harburg.de,
"Josef 'Jeff' Sipek" <jsipek@...sunysb.edu>
Subject: [PATCH 32/32] Unionfs: unionfs_create rewrite
The code was hard to follow and violated some invariants (e.g., never modify
a read only branch, and always create on branch 0).
Signed-off-by: Josef 'Jeff' Sipek <jsipek@...sunysb.edu>
---
fs/unionfs/inode.c | 207 +++++++++++++++-------------------------------------
1 files changed, 58 insertions(+), 149 deletions(-)
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 9adf272..08c1ae0 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -24,9 +24,7 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
int err = 0;
struct dentry *lower_dentry = NULL;
struct dentry *wh_dentry = NULL;
- struct dentry *new_lower_dentry;
struct dentry *lower_parent_dentry = NULL;
- int bindex = 0, bstart;
char *name = NULL;
int valid = 0;
@@ -47,177 +45,88 @@ static int unionfs_create(struct inode *parent, struct dentry *dentry,
*/
BUG_ON(!valid && dentry->d_inode);
- /* We start out in the leftmost branch. */
- bstart = dbstart(dentry);
- lower_dentry = unionfs_lower_dentry(dentry);
-
/*
- * check if whiteout exists in this branch, i.e. lookup .wh.foo
- * first.
+ * We shouldn't create things in a read-only branch; this check is a
+ * bit redundant as we don't allow branch 0 to be read-only at the
+ * moment
*/
- name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
- if (IS_ERR(name)) {
- err = PTR_ERR(name);
- goto out;
- }
-
- wh_dentry = lookup_one_len(name, lower_dentry->d_parent,
- dentry->d_name.len + UNIONFS_WHLEN);
- if (IS_ERR(wh_dentry)) {
- err = PTR_ERR(wh_dentry);
- wh_dentry = NULL;
+ if ((err = is_robranch_super(dentry->d_sb, 0))) {
+ err = -EROFS;
goto out;
}
- if (wh_dentry->d_inode) {
+ /*
+ * We _always_ create on branch 0
+ */
+ lower_dentry = unionfs_lower_dentry_idx(dentry, 0);
+ if (lower_dentry) {
/*
- * .wh.foo has been found.
- * First truncate it and then rename it to foo (hence having
- * the same overall effect as a normal create.
+ * check if whiteout exists in this branch, i.e. lookup .wh.foo
+ * first.
*/
- struct dentry *lower_dir_dentry;
- struct iattr newattrs;
-
- mutex_lock(&wh_dentry->d_inode->i_mutex);
- newattrs.ia_valid = ATTR_CTIME | ATTR_MODE | ATTR_ATIME
- | ATTR_MTIME | ATTR_UID | ATTR_GID | ATTR_FORCE
- | ATTR_KILL_SUID | ATTR_KILL_SGID;
-
- newattrs.ia_mode = mode & ~current->fs->umask;
- newattrs.ia_uid = current->fsuid;
- newattrs.ia_gid = current->fsgid;
-
- if (wh_dentry->d_inode->i_size != 0) {
- newattrs.ia_valid |= ATTR_SIZE;
- newattrs.ia_size = 0;
- }
-
- err = notify_change(wh_dentry, &newattrs);
-
- mutex_unlock(&wh_dentry->d_inode->i_mutex);
-
- if (err)
- printk(KERN_WARNING "unionfs: %s:%d: notify_change "
- "failed: %d, ignoring..\n",
- __FILE__, __LINE__, err);
-
- new_lower_dentry = unionfs_lower_dentry(dentry);
- dget(new_lower_dentry);
-
- lower_dir_dentry = dget_parent(wh_dentry);
- lock_rename(lower_dir_dentry, lower_dir_dentry);
-
- if (!(err = is_robranch_super(dentry->d_sb, bstart))) {
- err = vfs_rename(lower_dir_dentry->d_inode,
- wh_dentry,
- lower_dir_dentry->d_inode,
- new_lower_dentry);
- }
- if (!err) {
- fsstack_copy_attr_times(parent,
- new_lower_dentry->d_parent->
- d_inode);
- fsstack_copy_inode_size(parent,
- new_lower_dentry->d_parent->
- d_inode);
- parent->i_nlink = unionfs_get_nlinks(parent);
+ name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
+ if (IS_ERR(name)) {
+ err = PTR_ERR(name);
+ goto out;
}
- unlock_rename(lower_dir_dentry, lower_dir_dentry);
- dput(lower_dir_dentry);
-
- dput(new_lower_dentry);
-
- if (err) {
- /* exit if the error returned was NOT -EROFS */
- if (!IS_COPYUP_ERR(err))
- goto out;
- /*
- * We were not able to create the file in this
- * branch, so, we try to create it in one branch to
- * left
- */
- bstart--;
- } else {
- /*
- * reset the unionfs dentry to point to the .wh.foo
- * entry.
- */
-
- /* Discard any old reference. */
- dput(unionfs_lower_dentry(dentry));
-
- /* Trade one reference to another. */
- unionfs_set_lower_dentry_idx(dentry, bstart,
- wh_dentry);
+ wh_dentry = lookup_one_len(name, lower_dentry->d_parent,
+ dentry->d_name.len + UNIONFS_WHLEN);
+ if (IS_ERR(wh_dentry)) {
+ err = PTR_ERR(wh_dentry);
wh_dentry = NULL;
-
- /*
- * Only INTERPOSE_LOOKUP can return a value other
- * than 0 on err.
- */
- err = PTR_ERR(unionfs_interpose(dentry,
- parent->i_sb, 0));
goto out;
}
- }
- for (bindex = bstart; bindex >= 0; bindex--) {
- lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
- if (!lower_dentry) {
+ if (wh_dentry->d_inode) {
/*
- * if lower_dentry is NULL, create the entire
- * dentry directory structure in branch 'bindex'.
- * lower_dentry will NOT be null when bindex == bstart
- * because lookup passed as a negative unionfs dentry
- * pointing to a lone negative underlying dentry.
+ * .wh.foo has been found, so let's unlink it
*/
- lower_dentry = create_parents(parent, dentry,
- dentry->d_name.name,
- bindex);
- if (!lower_dentry || IS_ERR(lower_dentry)) {
- if (IS_ERR(lower_dentry))
- err = PTR_ERR(lower_dentry);
- continue;
+ struct dentry *lower_dir_dentry;
+
+ lower_dir_dentry = lock_parent(wh_dentry);
+ err = vfs_unlink(lower_dir_dentry->d_inode, wh_dentry);
+ unlock_dir(lower_dir_dentry);
+
+ if (err) {
+ printk("unionfs_create: could not unlink "
+ "whiteout, err = %d\n", err);
+ goto out;
}
}
-
- lower_parent_dentry = lock_parent(lower_dentry);
- if (IS_ERR(lower_parent_dentry)) {
- err = PTR_ERR(lower_parent_dentry);
+ } else {
+ /*
+ * if lower_dentry is NULL, create the entire
+ * dentry directory structure in branch 0.
+ */
+ lower_dentry = create_parents(parent, dentry, dentry->d_name.name, 0);
+ if (IS_ERR(lower_dentry)) {
+ err = PTR_ERR(lower_dentry);
goto out;
}
- /* We shouldn't create things in a read-only branch. */
- if (!(err = is_robranch_super(dentry->d_sb, bindex)))
- err = vfs_create(lower_parent_dentry->d_inode,
- lower_dentry, mode, nd);
+ }
- if (err || !lower_dentry->d_inode) {
- unlock_dir(lower_parent_dentry);
+ lower_parent_dentry = lock_parent(lower_dentry);
+ if (IS_ERR(lower_parent_dentry)) {
+ err = PTR_ERR(lower_parent_dentry);
+ goto out;
+ }
- /* break out of for loop if the error wasn't -EROFS */
- if (!IS_COPYUP_ERR(err))
- break;
- } else {
- /*
- * Only INTERPOSE_LOOKUP can return a value other
- * than 0 on err.
- */
- err = PTR_ERR(unionfs_interpose(dentry,
- parent->i_sb, 0));
- if (!err) {
- unionfs_copy_attr_times(parent);
- fsstack_copy_inode_size(parent,
- lower_parent_dentry->
- d_inode);
- /* update no. of links on parent directory */
- parent->i_nlink = unionfs_get_nlinks(parent);
- }
- unlock_dir(lower_parent_dentry);
- break;
+ err = vfs_create(lower_parent_dentry->d_inode, lower_dentry, mode, nd);
+
+ if (!err) {
+ err = PTR_ERR(unionfs_interpose(dentry, parent->i_sb, 0));
+ if (!err) {
+ unionfs_copy_attr_times(parent);
+ fsstack_copy_inode_size(parent,
+ lower_parent_dentry->d_inode);
+ /* update no. of links on parent directory */
+ parent->i_nlink = unionfs_get_nlinks(parent);
}
}
+ unlock_dir(lower_parent_dentry);
+
out:
dput(wh_dentry);
kfree(name);
--
1.5.2.2.238.g7cbf2f2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists