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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <12070840183389-git-send-email-ezk@cs.sunysb.edu>
Date:	Tue,  1 Apr 2008 17:06:47 -0400
From:	Erez Zadok <ezk@...sunysb.edu>
To:	akpm@...ux-foundation.org
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	viro@....linux.org.uk, hch@...radead.org,
	Erez Zadok <ezk@...sunysb.edu>
Subject: [PATCH 05/14] Unionfs: lock our dentry in file operations

Signed-off-by: Erez Zadok <ezk@...sunysb.edu>
---
 fs/unionfs/commonfops.c |   32 ++++++++++-----------------
 fs/unionfs/dirfops.c    |   18 ++++++++++-----
 fs/unionfs/file.c       |   55 +++++++++++++++++++++++++++++------------------
 fs/unionfs/union.h      |    1 -
 4 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 2add167..50f4eda 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -302,7 +302,7 @@ out:
  * @willwrite: true if caller may cause changes to the file; false otherwise.
  * Caller must lock/unlock dentry's branch configuration.
  */
-int unionfs_file_revalidate_locked(struct file *file, bool willwrite)
+int unionfs_file_revalidate(struct file *file, bool willwrite)
 {
 	struct super_block *sb;
 	struct dentry *dentry;
@@ -312,6 +312,7 @@ int unionfs_file_revalidate_locked(struct file *file, bool willwrite)
 	int err = 0;
 
 	dentry = file->f_path.dentry;
+	verify_locked(dentry);
 	sb = dentry->d_sb;
 
 	/*
@@ -419,17 +420,6 @@ out_nofree:
 	return err;
 }
 
-int unionfs_file_revalidate(struct file *file, bool willwrite)
-{
-	int err;
-
-	unionfs_lock_dentry(file->f_path.dentry, UNIONFS_DMUTEX_CHILD);
-	err = unionfs_file_revalidate_locked(file, willwrite);
-	unionfs_unlock_dentry(file->f_path.dentry);
-
-	return err;
-}
-
 /* unionfs_open helper function: open a directory */
 static int __open_dir(struct inode *inode, struct file *file)
 {
@@ -632,6 +622,8 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 	int fgen, err = 0;
 
 	unionfs_read_lock(sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+
 	/*
 	 * Yes, we have to revalidate this file even if it's being released.
 	 * This is important for open-but-unlinked files, as well as mmap
@@ -650,7 +642,6 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 	bstart = fbstart(file);
 	bend = fbend(file);
 
-	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 	for (bindex = bstart; bindex <= bend; bindex++) {
 		lower_file = unionfs_lower_file_idx(file, bindex);
 
@@ -665,7 +656,6 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 			unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
 		}
 	}
-	unionfs_unlock_dentry(dentry);
 
 	kfree(fileinfo->lower_files);
 	kfree(fileinfo->saved_branch_ids);
@@ -683,6 +673,7 @@ int unionfs_file_release(struct inode *inode, struct file *file)
 	kfree(fileinfo);
 
 out:
+	unionfs_unlock_dentry(dentry);
 	unionfs_read_unlock(sb);
 	return err;
 }
@@ -729,7 +720,6 @@ static int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,
 	struct vfsmount *mnt;
 
 	dentry = file->f_path.dentry;
-	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 	orig_bstart = dbstart(dentry);
 	orig_bend = dbend(dentry);
 	err = unionfs_partial_lookup(dentry);
@@ -771,15 +761,16 @@ static int unionfs_ioctl_queryfile(struct file *file, unsigned int cmd,
 		err = -EFAULT;
 
 out:
-	unionfs_unlock_dentry(dentry);
 	return err < 0 ? err : bend;
 }
 
 long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	long err;
+	struct dentry *dentry = file->f_path.dentry;
 
-	unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
 	err = unionfs_file_revalidate(file, true);
 	if (unlikely(err))
@@ -807,7 +798,8 @@ long unionfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 out:
 	unionfs_check_file(file);
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
+	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -821,7 +813,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
 	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
-	err = unionfs_file_revalidate_locked(file, true);
+	err = unionfs_file_revalidate(file, true);
 	if (unlikely(err))
 		goto out;
 	unionfs_check_file(file);
@@ -843,7 +835,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 out:
 	if (!err)
 		unionfs_check_file(file);
-	unionfs_unlock_dentry(file->f_path.dentry);
+	unionfs_unlock_dentry(dentry);
 	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
diff --git a/fs/unionfs/dirfops.c b/fs/unionfs/dirfops.c
index a613862..8272fb6 100644
--- a/fs/unionfs/dirfops.c
+++ b/fs/unionfs/dirfops.c
@@ -97,19 +97,21 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
 {
 	int err = 0;
 	struct file *lower_file = NULL;
+	struct dentry *dentry = file->f_path.dentry;
 	struct inode *inode = NULL;
 	struct unionfs_getdents_callback buf;
 	struct unionfs_dir_state *uds;
 	int bend;
 	loff_t offset;
 
-	unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
 	err = unionfs_file_revalidate(file, false);
 	if (unlikely(err))
 		goto out;
 
-	inode = file->f_path.dentry->d_inode;
+	inode = dentry->d_inode;
 
 	uds = UNIONFS_F(file)->rdstate;
 	if (!uds) {
@@ -189,7 +191,8 @@ static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
 	}
 
 out:
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
+	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -206,9 +209,11 @@ out:
 static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)
 {
 	struct unionfs_dir_state *rdstate;
+	struct dentry *dentry = file->f_path.dentry;
 	loff_t err;
 
-	unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
 	err = unionfs_file_revalidate(file, false);
 	if (unlikely(err))
@@ -250,7 +255,7 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)
 					err = -EINVAL;
 			} else {
 				struct inode *inode;
-				inode = file->f_path.dentry->d_inode;
+				inode = dentry->d_inode;
 				rdstate = find_rdstate(inode, offset);
 				if (rdstate) {
 					UNIONFS_F(file)->rdstate = rdstate;
@@ -269,7 +274,8 @@ static loff_t unionfs_dir_llseek(struct file *file, loff_t offset, int origin)
 	}
 
 out:
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
+	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
diff --git a/fs/unionfs/file.c b/fs/unionfs/file.c
index 9397ff3..0a39387 100644
--- a/fs/unionfs/file.c
+++ b/fs/unionfs/file.c
@@ -27,7 +27,7 @@ static ssize_t unionfs_read(struct file *file, char __user *buf,
 
 	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
 	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
-	err = unionfs_file_revalidate_locked(file, false);
+	err = unionfs_file_revalidate(file, false);
 	if (unlikely(err))
 		goto out;
 
@@ -55,7 +55,7 @@ static ssize_t unionfs_write(struct file *file, const char __user *buf,
 
 	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
 	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
-	err = unionfs_file_revalidate_locked(file, true);
+	err = unionfs_file_revalidate(file, true);
 	if (unlikely(err))
 		goto out;
 
@@ -94,9 +94,11 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
 	int err = 0;
 	bool willwrite;
 	struct file *lower_file;
+	struct dentry *dentry = file->f_path.dentry;
 	struct vm_operations_struct *saved_vm_ops = NULL;
 
-	unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
 	/* This might be deferred to mmap's writepage */
 	willwrite = ((vma->vm_flags | VM_SHARED | VM_WRITE) == vma->vm_flags);
@@ -157,10 +159,11 @@ static int unionfs_mmap(struct file *file, struct vm_area_struct *vma)
 out:
 	if (!err) {
 		/* copyup could cause parent dir times to change */
-		unionfs_copy_attr_times(file->f_path.dentry->d_parent->d_inode);
+		unionfs_copy_attr_times(dentry->d_parent->d_inode);
 		unionfs_check_file(file);
 	}
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
+	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -172,7 +175,8 @@ int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
 	struct inode *lower_inode, *inode;
 	int err = -EINVAL;
 
-	unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 	err = unionfs_file_revalidate(file, true);
 	if (unlikely(err))
 		goto out;
@@ -207,8 +211,10 @@ int unionfs_fsync(struct file *file, struct dentry *dentry, int datasync)
 	unionfs_copy_attr_times(inode);
 
 out:
-	unionfs_check_file(file);
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
+	if (!err)
+		unionfs_check_file(file);
+	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -216,11 +222,12 @@ int unionfs_fasync(int fd, struct file *file, int flag)
 {
 	int bindex, bstart, bend;
 	struct file *lower_file;
-	struct dentry *dentry;
+	struct dentry *dentry = file->f_path.dentry;
 	struct inode *lower_inode, *inode;
 	int err = 0;
 
-	unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 	err = unionfs_file_revalidate(file, true);
 	if (unlikely(err))
 		goto out;
@@ -231,7 +238,6 @@ int unionfs_fasync(int fd, struct file *file, int flag)
 	if (bstart < 0 || bend < 0)
 		goto out;
 
-	dentry = file->f_path.dentry;
 	inode = dentry->d_inode;
 	if (unlikely(!inode)) {
 		printk(KERN_ERR
@@ -253,8 +259,10 @@ int unionfs_fasync(int fd, struct file *file, int flag)
 	unionfs_copy_attr_times(inode);
 
 out:
-	unionfs_check_file(file);
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
+	if (!err)
+		unionfs_check_file(file);
+	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -264,8 +272,10 @@ static ssize_t unionfs_splice_read(struct file *file, loff_t *ppos,
 {
 	ssize_t err;
 	struct file *lower_file;
+	struct dentry *dentry = file->f_path.dentry;
 
-	unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 	err = unionfs_file_revalidate(file, false);
 	if (unlikely(err))
 		goto out;
@@ -274,14 +284,14 @@ static ssize_t unionfs_splice_read(struct file *file, loff_t *ppos,
 	err = vfs_splice_to(lower_file, ppos, pipe, len, flags);
 	/* update our inode atime upon a successful lower splice-read */
 	if (err >= 0) {
-		fsstack_copy_attr_atime(file->f_path.dentry->d_inode,
+		fsstack_copy_attr_atime(dentry->d_inode,
 					lower_file->f_path.dentry->d_inode);
 		unionfs_check_file(file);
 	}
 
 out:
-	unionfs_check_file(file);
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
+	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
@@ -291,8 +301,10 @@ static ssize_t unionfs_splice_write(struct pipe_inode_info *pipe,
 {
 	ssize_t err = 0;
 	struct file *lower_file;
+	struct dentry *dentry = file->f_path.dentry;
 
-	unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+	unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 	err = unionfs_file_revalidate(file, true);
 	if (unlikely(err))
 		goto out;
@@ -301,15 +313,16 @@ static ssize_t unionfs_splice_write(struct pipe_inode_info *pipe,
 	err = vfs_splice_from(pipe, lower_file, ppos, len, flags);
 	/* update our inode times+sizes upon a successful lower write */
 	if (err >= 0) {
-		fsstack_copy_inode_size(file->f_path.dentry->d_inode,
+		fsstack_copy_inode_size(dentry->d_inode,
 					lower_file->f_path.dentry->d_inode);
-		fsstack_copy_attr_times(file->f_path.dentry->d_inode,
+		fsstack_copy_attr_times(dentry->d_inode,
 					lower_file->f_path.dentry->d_inode);
 		unionfs_check_file(file);
 	}
 
 out:
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
+	unionfs_unlock_dentry(dentry);
+	unionfs_read_unlock(dentry->d_sb);
 	return err;
 }
 
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index f6fabf8..63f3b21 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -355,7 +355,6 @@ extern int unionfs_setlk(struct file *file, int cmd, struct file_lock *fl);
 extern int unionfs_getlk(struct file *file, struct file_lock *fl);
 
 /* Common file operations. */
-extern int unionfs_file_revalidate_locked(struct file *file, bool willwrite);
 extern int unionfs_file_revalidate(struct file *file, bool willwrite);
 extern int unionfs_open(struct inode *inode, struct file *file);
 extern int unionfs_file_release(struct inode *inode, struct file *file);
-- 
1.5.2.2

--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ