[Devel] [PATCH RHEL7 COMMIT] overlayfs: verify upper dentry before unlink and rename

Vladimir Davydov vdavydov at virtuozzo.com
Thu Jul 7 07:32:38 PDT 2016


The commit is pushed to "branch-rh7-3.10.0-327.18.2.vz7.14.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.18.2.vz7.14.23
------>
commit 4791f46af35069ef00b582a7f229974f6b16e5bd
Author: Maxim Patlasov <mpatlasov at virtuozzo.com>
Date:   Thu Jul 7 18:32:38 2016 +0400

    overlayfs: verify upper dentry before unlink and rename
    
    Without this patch it is easy to crash node by fiddling
    with overlayfs dirs. Backport commit 11f37104 from ms:
    
    From: Miklos Szeredi <mszeredi at redhat.com>
    
    ovl: verify upper dentry before unlink and rename
    
    Unlink and rename in overlayfs checked the upper dentry for staleness by
    verifying upper->d_parent against upperdir.  However the dentry can go
    stale also by being unhashed, for example.
    
    Expand the verification to actually look up the name again (under parent
    lock) and check if it matches the upper dentry.  This matches what the VFS
    does before passing the dentry to filesytem's unlink/rename methods, which
    excludes any inconsistency caused by overlayfs.
    
    Signed-off-by: Miklos Szeredi <mszeredi at redhat.com>
    
    https://jira.sw.ru/browse/PSBM-47981
---
 fs/overlayfs/dir.c | 59 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 33c47712d16f..229b9e4be3bd 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -596,21 +596,25 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
 {
 	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
 	struct inode *dir = upperdir->d_inode;
-	struct dentry *upper = ovl_dentry_upper(dentry);
+	struct dentry *upper;
 	int err;
 
 	mutex_lock_nested(&dir->i_mutex, I_MUTEX_PARENT);
+	upper = lookup_one_len(dentry->d_name.name, upperdir,
+			       dentry->d_name.len);
+	err = PTR_ERR(upper);
+	if (IS_ERR(upper))
+		goto out_unlock;
+
 	err = -ESTALE;
-	if (upper->d_parent == upperdir) {
-		/* Don't let d_delete() think it can reset d_inode */
-		dget(upper);
+	if (upper == ovl_dentry_upper(dentry)) {
 		if (is_dir)
 			err = vfs_rmdir(dir, upper);
 		else
 			err = vfs_unlink(dir, upper, NULL);
-		dput(upper);
 		ovl_dentry_version_inc(dentry->d_parent);
 	}
+	dput(upper);
 
 	/*
 	 * Keeping this dentry hashed would mean having to release
@@ -619,6 +623,7 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
 	 * now.
 	 */
 	d_drop(dentry);
+out_unlock:
 	mutex_unlock(&dir->i_mutex);
 
 	return err;
@@ -839,29 +844,39 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old,
 
 	trap = lock_rename(new_upperdir, old_upperdir);
 
-	olddentry = ovl_dentry_upper(old);
-	newdentry = ovl_dentry_upper(new);
-	if (newdentry) {
+
+	olddentry = lookup_one_len(old->d_name.name, old_upperdir,
+				   old->d_name.len);
+	err = PTR_ERR(olddentry);
+	if (IS_ERR(olddentry))
+		goto out_unlock;
+
+	err = -ESTALE;
+	if (olddentry != ovl_dentry_upper(old))
+		goto out_dput_old;
+
+	newdentry = lookup_one_len(new->d_name.name, new_upperdir,
+				   new->d_name.len);
+	err = PTR_ERR(newdentry);
+	if (IS_ERR(newdentry))
+		goto out_dput_old;
+
+	err = -ESTALE;
+	if (ovl_dentry_upper(new)) {
 		if (opaquedir) {
-			newdentry = opaquedir;
-			opaquedir = NULL;
+			if (newdentry != opaquedir)
+				goto out_dput;
 		} else {
-			dget(newdentry);
+			if (newdentry != ovl_dentry_upper(new))
+				goto out_dput;
 		}
 	} else {
 		new_create = true;
-		newdentry = lookup_one_len(new->d_name.name, new_upperdir,
-					   new->d_name.len);
-		err = PTR_ERR(newdentry);
-		if (IS_ERR(newdentry))
-			goto out_unlock;
+		if (!d_is_negative(newdentry) &&
+		    (!new_opaque || !ovl_is_whiteout(newdentry)))
+			goto out_dput;
 	}
 
-	err = -ESTALE;
-	if (olddentry->d_parent != old_upperdir)
-		goto out_dput;
-	if (newdentry->d_parent != new_upperdir)
-		goto out_dput;
 	if (olddentry == trap)
 		goto out_dput;
 	if (newdentry == trap)
@@ -917,6 +932,8 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old,
 
 out_dput:
 	dput(newdentry);
+out_dput_old:
+	dput(olddentry);
 out_unlock:
 	unlock_rename(new_upperdir, old_upperdir);
 out_revert_creds:


More information about the Devel mailing list