public inbox for linux-audit@redhat.com
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Paul Moore <paul@paul-moore.com>
Cc: rgb@redhat.com, Jan Kara <jack@suse.cz>,
	linux-audit@redhat.com, amir73il@gmail.com,
	Al Viro <viro@ZenIV.linux.org.uk>
Subject: [PATCH 12/14] audit: Simplify locking around untag_chunk()
Date: Wed, 17 Oct 2018 12:15:03 +0200	[thread overview]
Message-ID: <20181017101505.25881-13-jack@suse.cz> (raw)
In-Reply-To: <20181017101505.25881-1-jack@suse.cz>

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@suse.cz>
---
 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

  parent reply	other threads:[~2018-10-17 10:15 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-17 10:14 [PATCH 0/14 v4] audit: Fix various races when tagging and untagging mounts Jan Kara
2018-10-17 10:14 ` [PATCH 01/14] audit_tree: Remove mark->lock locking Jan Kara
2018-10-17 10:14 ` [PATCH 02/14] audit: Fix possible spurious -ENOSPC error Jan Kara
2018-10-17 10:14 ` [PATCH 03/14] audit: Fix possible tagging failures Jan Kara
2018-10-17 10:14 ` [PATCH 04/14] audit: Embed key into chunk Jan Kara
2018-10-17 10:14 ` [PATCH 05/14] audit: Make hash table insertion safe against concurrent lookups Jan Kara
2018-10-17 10:14 ` [PATCH 06/14] audit: Factor out chunk replacement code Jan Kara
2018-10-18 19:27   ` Richard Guy Briggs
2018-11-06 13:58     ` Paul Moore
2018-11-07  9:55       ` Jan Kara
2018-11-09 14:45         ` Paul Moore
2018-11-12 15:15           ` Paul Moore
2018-11-12 15:25             ` Jan Kara
2018-10-17 10:14 ` [PATCH 07/14] audit: Remove pointless check in insert_hash() Jan Kara
2018-10-17 10:14 ` [PATCH 08/14] audit: Provide helper for dropping mark's chunk reference Jan Kara
2018-10-17 10:15 ` [PATCH 09/14] audit: Allocate fsnotify mark independently of chunk Jan Kara
2018-10-17 10:15 ` [PATCH 10/14] audit: Guarantee forward progress of chunk untagging Jan Kara
2018-10-18 19:29   ` Richard Guy Briggs
2018-10-17 10:15 ` [PATCH 11/14] audit: Drop all unused chunk nodes during deletion Jan Kara
2018-10-18 19:32   ` Richard Guy Briggs
2018-11-06 14:14   ` Paul Moore
2018-11-07 10:00     ` Jan Kara
2018-10-17 10:15 ` Jan Kara [this message]
2018-10-18 12:27   ` [PATCH 12/14] audit: Simplify locking around untag_chunk() Richard Guy Briggs
2018-10-19  8:22     ` Jan Kara
2018-10-19 11:18       ` Richard Guy Briggs
2018-10-17 10:15 ` [PATCH 13/14] audit: Replace chunk attached to mark instead of replacing mark Jan Kara
2018-10-18 19:39   ` Richard Guy Briggs
2018-10-17 10:15 ` [PATCH 14/14] audit: Use 'mark' name for fsnotify_mark variables Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181017101505.25881-13-jack@suse.cz \
    --to=jack@suse.cz \
    --cc=amir73il@gmail.com \
    --cc=linux-audit@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=rgb@redhat.com \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox