linux-kernel - [PATCH 2/3] sysfs: Fix and sysfs_mv_dir by using lock

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: <m1vdw465lm.fsf_-_@frodo.ebiederm.org>
Date:	Tue, 07 Oct 2008 03:51:17 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Greg KH <greg@...ah.com>
Cc:	Al Viro <viro@...IV.linux.org.uk>,
	Benjamin Thery <benjamin.thery@...l.net>,
	linux-kernel@...r.kernel.org, "Serge E. Hallyn" <serue@...ibm.com>,
	Al Viro <viro@....linux.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Tejun Heo <tj@...nel.org>
Subject: [PATCH 2/3] sysfs: Fix and sysfs_mv_dir by using lock_rename.


sysfs_mv_dir picks an arbitrary mutex lock ordering and busy waits
until it can grab the inode mutexes in that order.  This can dead lock
on UP systems.

Instead use the generic lock_rename that guarantees we grab parent
directories before child directories, so there is not need to spin and
no chance of deadlock.

Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>
---
 fs/sysfs/dir.c |   43 +++++++++++++++++++------------------------
 1 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index a76fb54..94f2ed1 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -955,12 +955,22 @@ err_out:
 	return error;
 }
 
+static void sysfs_lock_rename(struct sysfs_rename_struct *srs)
+{
+	lock_rename(srs->old_parent, srs->new_parent);
+}
+
+static void sysfs_unlock_rename(struct sysfs_rename_struct *srs)
+{
+	unlock_rename(srs->old_parent, srs->new_parent);
+}
+
 static int sysfs_mv_dir(struct sysfs_dirent *sd,
 	struct sysfs_dirent *new_parent_sd, const char *new_name)
 {
 	struct list_head todo;
 	struct sysfs_rename_struct *srs;
-	struct inode *old_parent_inode = NULL, *new_parent_inode = NULL;
+	struct sysfs_dirent *old_parent_sd;
 	const char *dup_name = NULL;
 	const void *old_tag, *tag;
 	int error;
@@ -971,11 +981,12 @@ static int sysfs_mv_dir(struct sysfs_dirent *sd,
 	if (!new_parent_sd)
 		new_parent_sd = &sysfs_root;
 
+	old_parent_sd = sysfs_get(sd->s_parent);
 	old_tag = sd->s_tag;
-	tag = sysfs_creation_tag(sd->s_parent, sd);
+	tag = sysfs_creation_tag(old_parent_sd, sd);
 
 	error = 0;
-	if ((sd->s_parent == new_parent_sd) && (old_tag == tag) &&
+	if ((old_parent_sd == new_parent_sd) && (old_tag == tag) &&
 	    (strcmp(sd->s_name, new_name) == 0))
 		goto out;	/* nothing to do */
 
@@ -986,22 +997,8 @@ static int sysfs_mv_dir(struct sysfs_dirent *sd,
 			goto out_release;
 	}
 
-	error = -ENOMEM;
-	mutex_lock(&sysfs_mutex);
-	old_parent_inode = sysfs_get_inode(sd->s_parent);
-	new_parent_inode = sysfs_get_inode(new_parent_sd);
-	mutex_unlock(&sysfs_mutex);
-	if (!old_parent_inode || !new_parent_inode)
-		goto out_release;
-
-again:
-	mutex_lock(&old_parent_inode->i_mutex);
-	if (old_parent_inode != new_parent_inode) {
-		if (!mutex_trylock(&new_parent_inode->i_mutex)) {
-			mutex_unlock(&old_parent_inode->i_mutex);
-			goto again;
-		}
-	}
+	if (!list_empty(&todo))
+		sysfs_lock_rename(list_first_entry(&todo, struct sysfs_rename_struct, list));
 	mutex_lock(&sysfs_mutex);
 
 	error = -EEXIST;
@@ -1048,17 +1045,15 @@ again:
 	error = 0;
 out_unlock:
 	mutex_unlock(&sysfs_mutex);
-	if (new_parent_inode != old_parent_inode)
-		mutex_unlock(&new_parent_inode->i_mutex);
-	mutex_unlock(&old_parent_inode->i_mutex);
+	if (!list_empty(&todo))
+		sysfs_unlock_rename(list_first_entry(&todo, struct sysfs_rename_struct, list));
 	kfree(dup_name);
 
 out_release:
-	iput(new_parent_inode);
-	iput(old_parent_inode);
 	post_rename(&todo);
 	sysfs_release_supers();
 out:
+	sysfs_put(old_parent_sd);
 	mutex_unlock(&sysfs_rename_mutex);
 	return error;
 }
-- 
1.5.3.rc6.17.g1911

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