| 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: <1428569224-23820-136-git-send-email-lizf@kernel.org>
Date: Thu, 9 Apr 2015 16:46:24 +0800
From: lizf@...nel.org
To: stable@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Al Viro <viro@...iv.linux.org.uk>,
Ben Hutchings <ben@...adent.org.uk>,
Zefan Li <lizefan@...wei.com>
Subject: [PATCH 3.4 136/176] deal with deadlock in d_walk()
From: Al Viro <viro@...iv.linux.org.uk>
3.4.107-rc1 review patch. If anyone has any objections, please let me know.
------------------
commit ca5358ef75fc69fee5322a38a340f5739d997c10 upstream.
... by not hitting rename_retry for reasons other than rename having
happened. In other words, do _not_ restart when finding that
between unlocking the child and locking the parent the former got
into __dentry_kill(). Skip the killed siblings instead...
Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
[bwh: Backported to 3.2:
- As we only have try_to_ascend() and not d_walk(), apply this
change to all callers of try_to_ascend()
- Adjust context to make __dentry_kill() apply to d_kill()]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
[lizf: Backported to 3.4: fold the fix 2d5a2e6775fa in 3.2.y into this patch]
Signed-off-by: Zefan Li <lizefan@...wei.com>
---
fs/dcache.c | 105 +++++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 65 insertions(+), 40 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 2ccc646..8038a78 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -368,9 +368,9 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
__releases(parent->d_lock)
__releases(dentry->d_inode->i_lock)
{
- list_del(&dentry->d_child);
+ __list_del_entry(&dentry->d_child);
/*
- * Inform try_to_ascend() that we are no longer attached to the
+ * Inform ascending readers that we are no longer attached to the
* dentry tree
*/
dentry->d_flags |= DCACHE_DENTRY_KILLED;
@@ -1011,34 +1011,6 @@ void shrink_dcache_for_umount(struct super_block *sb)
}
}
-/*
- * This tries to ascend one level of parenthood, but
- * we can race with renaming, so we need to re-check
- * the parenthood after dropping the lock and check
- * that the sequence number still matches.
- */
-static struct dentry *try_to_ascend(struct dentry *old, int locked, unsigned seq)
-{
- struct dentry *new = old->d_parent;
-
- rcu_read_lock();
- spin_unlock(&old->d_lock);
- spin_lock(&new->d_lock);
-
- /*
- * might go back up the wrong parent if we have had a rename
- * or deletion
- */
- if (new != old->d_parent ||
- (old->d_flags & DCACHE_DENTRY_KILLED) ||
- (!locked && read_seqretry(&rename_lock, seq))) {
- spin_unlock(&new->d_lock);
- new = NULL;
- }
- rcu_read_unlock();
- return new;
-}
-
/*
* Search for at least 1 mount point in the dentry's subdirs.
@@ -1094,30 +1066,48 @@ resume:
/*
* All done at this level ... ascend and resume the search.
*/
+ rcu_read_lock();
+ascend:
if (this_parent != parent) {
struct dentry *child = this_parent;
- this_parent = try_to_ascend(this_parent, locked, seq);
- if (!this_parent)
+ this_parent = child->d_parent;
+
+ spin_unlock(&child->d_lock);
+ spin_lock(&this_parent->d_lock);
+
+ /* might go back up the wrong parent if we have had a rename */
+ if (!locked && read_seqretry(&rename_lock, seq))
goto rename_retry;
next = child->d_child.next;
+ while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) {
+ if (next == &this_parent->d_subdirs)
+ goto ascend;
+ child = list_entry(next, struct dentry, d_child);
+ next = next->next;
+ }
+ rcu_read_unlock();
goto resume;
}
- spin_unlock(&this_parent->d_lock);
if (!locked && read_seqretry(&rename_lock, seq))
goto rename_retry;
+ spin_unlock(&this_parent->d_lock);
+ rcu_read_unlock();
if (locked)
write_sequnlock(&rename_lock);
return 0; /* No mount points found in tree */
positive:
if (!locked && read_seqretry(&rename_lock, seq))
- goto rename_retry;
+ goto rename_retry_unlocked;
if (locked)
write_sequnlock(&rename_lock);
return 1;
rename_retry:
+ spin_unlock(&this_parent->d_lock);
+ rcu_read_unlock();
if (locked)
goto again;
+rename_retry_unlocked:
locked = 1;
write_seqlock(&rename_lock);
goto again;
@@ -1182,6 +1172,7 @@ resume:
*/
if (found && need_resched()) {
spin_unlock(&dentry->d_lock);
+ rcu_read_lock();
goto out;
}
@@ -1201,23 +1192,40 @@ resume:
/*
* All done at this level ... ascend and resume the search.
*/
+ rcu_read_lock();
+ascend:
if (this_parent != parent) {
struct dentry *child = this_parent;
- this_parent = try_to_ascend(this_parent, locked, seq);
- if (!this_parent)
+ this_parent = child->d_parent;
+
+ spin_unlock(&child->d_lock);
+ spin_lock(&this_parent->d_lock);
+
+ /* might go back up the wrong parent if we have had a rename */
+ if (!locked && read_seqretry(&rename_lock, seq))
goto rename_retry;
next = child->d_child.next;
+ while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) {
+ if (next == &this_parent->d_subdirs)
+ goto ascend;
+ child = list_entry(next, struct dentry, d_child);
+ next = next->next;
+ }
+ rcu_read_unlock();
goto resume;
}
out:
- spin_unlock(&this_parent->d_lock);
if (!locked && read_seqretry(&rename_lock, seq))
goto rename_retry;
+ spin_unlock(&this_parent->d_lock);
+ rcu_read_unlock();
if (locked)
write_sequnlock(&rename_lock);
return found;
rename_retry:
+ spin_unlock(&this_parent->d_lock);
+ rcu_read_unlock();
if (found)
return found;
if (locked)
@@ -2955,26 +2963,43 @@ resume:
}
spin_unlock(&dentry->d_lock);
}
+ rcu_read_lock();
+ascend:
if (this_parent != root) {
struct dentry *child = this_parent;
if (!(this_parent->d_flags & DCACHE_GENOCIDE)) {
this_parent->d_flags |= DCACHE_GENOCIDE;
this_parent->d_count--;
}
- this_parent = try_to_ascend(this_parent, locked, seq);
- if (!this_parent)
+ this_parent = child->d_parent;
+
+ spin_unlock(&child->d_lock);
+ spin_lock(&this_parent->d_lock);
+
+ /* might go back up the wrong parent if we have had a rename */
+ if (!locked && read_seqretry(&rename_lock, seq))
goto rename_retry;
next = child->d_child.next;
+ while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) {
+ if (next == &this_parent->d_subdirs)
+ goto ascend;
+ child = list_entry(next, struct dentry, d_child);
+ next = next->next;
+ }
+ rcu_read_unlock();
goto resume;
}
- spin_unlock(&this_parent->d_lock);
if (!locked && read_seqretry(&rename_lock, seq))
goto rename_retry;
+ spin_unlock(&this_parent->d_lock);
+ rcu_read_unlock();
if (locked)
write_sequnlock(&rename_lock);
return;
rename_retry:
+ spin_unlock(&this_parent->d_lock);
+ rcu_read_unlock();
if (locked)
goto again;
locked = 1;
--
1.9.1
--
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