Re: [PATCH 12/14] audit: Simplify locking around untag_chunk()

untag_chunk() has to be called with hash_lock, it drops it and reacquires it when returning. The unlocking of hash_lock is thus hidden from the callers of untag_chunk() with is rather error prone. Reorganize the code so that untag_chunk() is called without hash_lock, only with mark reference preventing the chunk from going away. Since this requires some more code in the caller of untag_chunk() to assure forward progress, factor out loop pruning tree from all chunks into a common helper function. Signed-off-by: Jan Kara <jack(a)suse.cz&gt; --- kernel/audit_tree.c | 75 ++++++++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index 145e8c92dd31..5deb4e1ed648 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -332,27 +332,19 @@ static int chunk_count_trees(struct audit_chunk *chunk) return ret; } -static void untag_chunk(struct node *p) +static void untag_chunk(struct audit_chunk *chunk, struct fsnotify_mark *entry) { - struct audit_chunk *chunk = find_chunk(p); - struct fsnotify_mark *entry = chunk->mark; - struct audit_chunk *new = NULL; + struct audit_chunk *new; int size; - remove_chunk_node(chunk, p); - fsnotify_get_mark(entry); - spin_unlock(&hash_lock); - - mutex_lock(&entry->group->mark_mutex); + mutex_lock(&audit_tree_group->mark_mutex); /* * mark_mutex protects mark from getting detached and thus also from * mark->connector->obj getting NULL. */ if (chunk->dead || !(entry->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) { - mutex_unlock(&entry->group->mark_mutex); - if (new) - fsnotify_put_mark(new->mark); - goto out; + mutex_unlock(&audit_tree_group->mark_mutex); + return;

} size = chunk_count_trees(chunk); @@ -363,9 +355,9 @@ static void untag_chunk(struct node *p) list_del_rcu(&chunk->hash); spin_unlock(&hash_lock); fsnotify_detach_mark(entry); - mutex_unlock(&entry->group->mark_mutex); + mutex_unlock(&audit_tree_group->mark_mutex); fsnotify_free_mark(entry); - goto out; + return; } new = alloc_chunk(size); @@ -387,16 +379,13 @@ static void untag_chunk(struct node *p) replace_chunk(new, chunk); spin_unlock(&hash_lock); fsnotify_detach_mark(entry); - mutex_unlock(&entry->group->mark_mutex); + mutex_unlock(&audit_tree_group->mark_mutex); fsnotify_free_mark(entry); fsnotify_put_mark(new->mark); /* drop initial reference */ - goto out; + return; out_mutex: - mutex_unlock(&entry->group->mark_mutex); -out: - fsnotify_put_mark(entry); - spin_lock(&hash_lock); + mutex_unlock(&audit_tree_group->mark_mutex); } /* Call with group->mark_mutex held, releases it */ @@ -579,22 +568,45 @@ static void kill_rules(struct audit_tree *tree) } /* - * finish killing struct audit_tree + * Remove tree from chunks. If 'tagged' is set, remove tree only from tagged + * chunks. The function expects tagged chunks are all at the beginning of the + * chunks list. */ -static void prune_one(struct audit_tree *victim) +static void prune_tree_chunks(struct audit_tree *victim, bool tagged) { spin_lock(&hash_lock); while (!list_empty(&victim->chunks)) { struct node *p; + struct audit_chunk *chunk; + struct fsnotify_mark *mark; + + p = list_first_entry(&victim->chunks, struct node, list); + /* have we run out of marked? */ + if (tagged && !(p->index & (1U<<31))) + break; + chunk = find_chunk(p); + mark = chunk->mark; + remove_chunk_node(chunk, p); + fsnotify_get_mark(mark); + spin_unlock(&hash_lock); - p = list_entry(victim->chunks.next, struct node, list); + untag_chunk(chunk, mark); + fsnotify_put_mark(mark); - untag_chunk(p); + spin_lock(&hash_lock); } spin_unlock(&hash_lock); put_tree(victim); } +/* + * finish killing struct audit_tree + */ +static void prune_one(struct audit_tree *victim) +{ + prune_tree_chunks(victim, false); +} + /* trim the uncommitted chunks from tree */ static void trim_marked(struct audit_tree *tree) @@ -614,18 +626,11 @@ static void trim_marked(struct audit_tree *tree) list_add(p, &tree->chunks); } } + spin_unlock(&hash_lock); - while (!list_empty(&tree->chunks)) { - struct node *node; - - node = list_entry(tree->chunks.next, struct node, list); - - /* have we run out of marked? */ - if (!(node->index & (1U<<31))) - break; + prune_tree_chunks(tree, true); - untag_chunk(node); - } + spin_lock(&hash_lock); if (!tree->root && !tree->goner) { tree->goner = 1; spin_unlock(&hash_lock); -- 2.16.4