[PATCH git] another audit watch fix

[Amy Griffis <amy.griffis@hp.com>]

This patch contains another fix for the audit watch implementation in
the git tree.  Al, both this patch and Monday's "audit watch fixes"
should fold into commit 95f06755b00c44d17757df4dc150997058c64849.

Don't call put_inotify_watch() via audit_free_rule_rcu.  If it's the
last ref it will call iput(), which can sleep.  We can't do that in
an rcu callback.

Signed-off-by: Amy Griffis <amy.griffis@hp.com>

---

 kernel/auditfilter.c |   88 +++++++++++++++++++++++---------------------------
 1 files changed, 41 insertions(+), 47 deletions(-)

00dff88f35a3a09e72411b06c847557d6d82da02
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 54bb856..7f2fcd6 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -49,12 +49,10 @@ #include "audit.h"
  * audit_parent: lifetime is from audit_init_parent() to receipt of an IN_IGNORED
  * 	event.  Each audit_watch holds a reference to its associated parent.
  *
- * audit_watch: if added to lists, lifetime is from audit_init_watch() to one
- * 	of: audit_remove_watch() [user removes], audit_update_watch() [kernel
- * 	replaces], or audit_remove_parent_watches() [kernel removes].
- * 	Additionally, an audit_watch may exist temporarily to assist in
- * 	searching existing filter data.  Each audit_krule holds a reference to
- * 	its associated watch.
+ * audit_watch: if added to lists, lifetime is from audit_init_watch() to
+ * 	audit_remove_watch().  Additionally, an audit_watch may exist
+ * 	temporarily to assist in searching existing filter data.  Each
+ * 	audit_krule holds a reference to its associated watch.
  */
 
 struct audit_parent {
@@ -68,12 +66,10 @@ struct audit_parent {
  * audit_parent status flags:
  *
  * AUDIT_PARENT_INVALID - set anytime rules/watches are auto-removed due to
- * a filesystem event.  Technically not needed for IN_DELETE_SELF or IN_UNMOUNT
- * events, as we cannot receive them while we have nameidata (during rule add)
- * and the audit_parent is immediately removed when processing the following
- * IN_IGNORED event.  The IN_MOVE_SELF event is different.  We can receive it
- * while holding nameidata, and inotify will not send us the IN_IGNORED so we
- * must later remove the inotify watch on audit_parent ourselves.
+ * a filesystem event to ensure we're adding audit watches to a valid parent.
+ * Technically not needed for IN_DELETE_SELF or IN_UNMOUNT events, as we cannot
+ * receive them while we have nameidata, but must be used for IN_MOVE_SELF which
+ * we can receive while holding nameidata.
  */
 #define AUDIT_PARENT_INVALID	0x001
 
@@ -115,15 +111,21 @@ static inline void audit_get_watch(struc
 static inline void audit_put_watch(struct audit_watch *watch)
 {
 	if (atomic_dec_and_test(&watch->count)) {
+		WARN_ON(watch->parent);
 		WARN_ON(!list_empty(&watch->rules));
-		/* watches that were never added don't have a parent */
-		if (watch->parent)
-			put_inotify_watch(&watch->parent->wdata);
 		kfree(watch->path);
 		kfree(watch);
 	}
 }
 
+static inline void audit_remove_watch(struct audit_watch *watch)
+{
+	list_del(&watch->wlist);
+	put_inotify_watch(&watch->parent->wdata);
+	watch->parent = NULL;
+	audit_put_watch(watch); /* match initial get */
+}
+
 static inline void audit_free_rule(struct audit_entry *e)
 {
 	int i;
@@ -779,8 +781,7 @@ static inline void audit_update_watch(st
 		audit_log_format(ab, " with dev=%u ino=%lu\n", dev, ino);
 		audit_log_end(ab);
 
-		list_del(&owatch->wlist);
-		audit_put_watch(owatch); /* matches initial get */
+		audit_remove_watch(owatch);
 		goto add_watch_to_parent; /* event applies to a single watch */
 	}
 	mutex_unlock(&audit_filter_mutex);
@@ -812,8 +813,7 @@ static inline void audit_remove_parent_w
 				 "audit implicitly removed rule from list=%d\n",
 				  AUDIT_FILTER_EXIT);
 		}
-		list_del(&w->wlist);
-		audit_put_watch(w); /* matches initial get */
+		audit_remove_watch(w);
 	}
 	mutex_unlock(&audit_filter_mutex);
 }
@@ -826,7 +826,7 @@ static void audit_inotify_unregister(str
 
 	list_for_each_entry(p, in_list, ilist) {
 		inotify_rm_watch(audit_ih, &p->wdata);
-		/* the put matching the get in audit_remove_watch() */
+		/* the put matching the get in audit_do_del_rule() */
 		put_inotify_watch(&p->wdata);
 	}
 }
@@ -953,7 +953,7 @@ static int audit_add_watch(struct audit_
 	else
 		audit_add_to_parent(krule, parent);
 
-	/* put ref grabbed by inotify_find_watch or before inotify_add_watch */
+	/* match get in audit_init_parent or inotify_find_watch */
 	put_inotify_watch(&parent->wdata);
 	return ret;
 }
@@ -1022,30 +1022,6 @@ error:
 	return err;
 }
 
-/* Remove given krule from its associated watch's rules list and clean up any
- * last instances of associated watch and parent.
- * Caller must hold audit_filter_mutex. */
-static inline void audit_remove_watch(struct audit_krule *krule,
-				      struct list_head *in_list)
-{
-	struct audit_watch *watch = krule->watch;
-	struct audit_parent *parent = watch->parent;
-
-	list_del(&krule->rlist);
-	if (list_empty(&watch->rules)) {
-		list_del(&watch->wlist);
-		audit_put_watch(watch); /* matches initial get */
-
-		if (list_empty(&parent->watches)) {
-			/* Put parent on the inotify un-registration list.
-			 * Grab a reference before releasing audit_filter_mutex,
-			 * to be released in audit_inotify_unregister(). */
-			list_add(&parent->ilist, in_list);
-			get_inotify_watch(&parent->wdata);
-		}
-	}
-}
-
 /* Rule removal helper.
  * Caller must hold audit_filter_mutex. */
 static inline int audit_do_del_rule(struct audit_entry *entry,
@@ -1055,11 +1031,29 @@ static inline int audit_do_del_rule(stru
 	struct audit_entry  *e;
 
 	list_for_each_entry(e, list, list) {
+		struct audit_watch *watch = e->rule.watch;
+		struct audit_parent *parent = watch->parent;
+
 		if (audit_compare_rule(&entry->rule, &e->rule))
 			continue;
 
-		if (e->rule.watch)
-			audit_remove_watch(&e->rule, inotify_list);
+		if (watch) {
+			list_del(&e->rule.rlist);
+
+			if (list_empty(&watch->rules)) {
+				audit_remove_watch(watch);
+
+				if (list_empty(&parent->watches)) {
+					/* Put parent on the inotify
+					 * un-registration list.  Grab
+					 * a reference before releasing
+					 * audit_filter_mutex, to be released in
+					 * audit_inotify_unregister(). */
+					list_add(&parent->ilist, inotify_list);
+					get_inotify_watch(&parent->wdata);
+				}
+			}
+		}
 
 		list_del_rcu(&e->list);
 		call_rcu(&e->rcu, audit_free_rule_rcu);
-- 
1.3.0