Skip to content

Commit

Permalink
autofs4 - fix get_next_positive_subdir()
Browse files Browse the repository at this point in the history
Following a report of a crash during an automount expire I found that
the locking in fs/autofs4/expire.c:get_next_positive_subdir() was wrong.
Not only is the locking wrong but the function is more complex than it
needs to be.

The function is meant to calculate (and dget) the next entry in the list
of directories contained in the root of an autofs mount point (an autofs
indirect mount to be precise). The main problem was that the d_lock of
the owner of the list was not being taken when walking the list, which
lead to list corruption under load. The only other lock that needs to
be taken is against the next dentry candidate so it can be checked for
usability.

Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
raven-au authored and torvalds committed Aug 16, 2012
1 parent 63ca5f1 commit a45440f
Showing 1 changed file with 13 additions and 18 deletions.
31 changes: 13 additions & 18 deletions fs/autofs4/expire.c
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -94,25 +94,21 @@ static struct dentry *get_next_positive_subdir(struct dentry *prev,
{ {
struct autofs_sb_info *sbi = autofs4_sbi(root->d_sb); struct autofs_sb_info *sbi = autofs4_sbi(root->d_sb);
struct list_head *next; struct list_head *next;
struct dentry *p, *q; struct dentry *q;


spin_lock(&sbi->lookup_lock); spin_lock(&sbi->lookup_lock);
spin_lock(&root->d_lock);


if (prev == NULL) { if (prev)
spin_lock(&root->d_lock); next = prev->d_u.d_child.next;
else {
prev = dget_dlock(root); prev = dget_dlock(root);
next = prev->d_subdirs.next; next = prev->d_subdirs.next;
p = prev;
goto start;
} }


p = prev; cont:
spin_lock(&p->d_lock);
again:
next = p->d_u.d_child.next;
start:
if (next == &root->d_subdirs) { if (next == &root->d_subdirs) {
spin_unlock(&p->d_lock); spin_unlock(&root->d_lock);
spin_unlock(&sbi->lookup_lock); spin_unlock(&sbi->lookup_lock);
dput(prev); dput(prev);
return NULL; return NULL;
Expand All @@ -121,16 +117,15 @@ static struct dentry *get_next_positive_subdir(struct dentry *prev,
q = list_entry(next, struct dentry, d_u.d_child); q = list_entry(next, struct dentry, d_u.d_child);


spin_lock_nested(&q->d_lock, DENTRY_D_LOCK_NESTED); spin_lock_nested(&q->d_lock, DENTRY_D_LOCK_NESTED);
/* Negative dentry - try next */ /* Already gone or negative dentry (under construction) - try next */
if (!simple_positive(q)) { if (q->d_count == 0 || !simple_positive(q)) {
spin_unlock(&p->d_lock); spin_unlock(&q->d_lock);
lock_set_subclass(&q->d_lock.dep_map, 0, _RET_IP_); next = q->d_u.d_child.next;
p = q; goto cont;
goto again;
} }
dget_dlock(q); dget_dlock(q);
spin_unlock(&q->d_lock); spin_unlock(&q->d_lock);
spin_unlock(&p->d_lock); spin_unlock(&root->d_lock);
spin_unlock(&sbi->lookup_lock); spin_unlock(&sbi->lookup_lock);


dput(prev); dput(prev);
Expand Down

0 comments on commit a45440f

Please sign in to comment.