[PATCH git 0/3] audit inode hash bug fix, cleanup

[PATCH git 0/3] audit inode hash bug fix, cleanup

[Amy Griffis]

The following patches include a few bug fixes and some cleanup for
audit's inode hash.

Al, these should be folded in with the main filesystem auditing patch:
lspp.b18 74457481a11b6283cecdfbaa6ec644e2edc1c8ef

Thanks,
Amy

Re: [PATCH git 1/3] fix inode rules not added to inode hash

[Amy Griffis]

Rules with field "inode=" belong in the inode hash.

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

---

 kernel/auditfilter.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

4d6785169aa274de9171ad2d34d2547412af4d4c
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index f35f535..d53a80d 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -248,6 +248,8 @@ static inline int audit_to_inode(struct 
 	if (krule->listnr != AUDIT_FILTER_EXIT ||
 	    krule->watch || krule->inode_f)
 		return -EINVAL;
+
+	krule->inode_f = f;
 	return 0;
 }
 
-- 
1.3.0

Re: [PATCH git 2/3] don't allow duplicate inode or watch rules

[Amy Griffis]

In audit_add_rule(), check the inode hash so duplicate rules can't be
added.

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

---

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

745105541b430c90127a2b7ad1806d995fd793b0
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index d53a80d..3cedc73 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -880,6 +880,37 @@ static void audit_inotify_unregister(str
 	}
 }
 
+/* Find an existing audit rule.
+ * Caller must hold audit_filter_mutex to prevent stale rule data. */
+static struct audit_entry *audit_find_rule(struct audit_entry *entry,
+					   struct list_head *list)
+{
+	struct audit_entry *e, *found = NULL;
+	int h;
+
+	if (entry->rule.watch) {
+		/* we don't know the inode number, so must walk entire hash */
+		for (h = 0; h < AUDIT_INODE_BUCKETS; h++) {
+			list = &audit_inode_hash[h];
+			list_for_each_entry(e, list, list)
+				if (!audit_compare_rule(&entry->rule, &e->rule)) {
+					found = e;
+					goto out;
+				}
+		}
+		goto out;
+	}
+
+	list_for_each_entry(e, list, list)
+		if (!audit_compare_rule(&entry->rule, &e->rule)) {
+			found = e;
+			goto out;
+		}
+
+out:
+	return found;
+}
+
 /* Get path information necessary for adding watches. */
 static int audit_get_nd(char *path, struct nameidata **ndp,
 			struct nameidata **ndw)
@@ -1017,16 +1048,18 @@ static inline int audit_add_rule(struct 
 	struct nameidata *ndp, *ndw;
 	int h, err, putnd_needed = 0;
 
-	/* Taking audit_filter_mutex protects from stale rule data. */
-	mutex_lock(&audit_filter_mutex);
-	list_for_each_entry(e, list, list) {
-		if (!audit_compare_rule(&entry->rule, &e->rule)) {
-			err = -EEXIST;
-			mutex_unlock(&audit_filter_mutex);
-			goto error;
-		}
+	if (inode_f) {
+		h = audit_hash_ino(inode_f->val);
+		list = &audit_inode_hash[h];
 	}
+
+	mutex_lock(&audit_filter_mutex);
+	e = audit_find_rule(entry, list);
 	mutex_unlock(&audit_filter_mutex);
+	if (e) {
+		err = -EEXIST;
+		goto error;
+	}
 
 	/* Avoid calling path_lookup under audit_filter_mutex. */
 	if (watch) {
@@ -1046,9 +1079,6 @@ static inline int audit_add_rule(struct 
 		}
 		h = audit_hash_ino((u32)watch->ino);
 		list = &audit_inode_hash[h];
-	} else if (inode_f) {
-		h = audit_hash_ino(inode_f->val);
-		list = &audit_inode_hash[h];
 	}
 
 	if (entry->rule.flags & AUDIT_FILTER_PREPEND) {
-- 
1.3.0

Re: [PATCH git 3/3] make audit_del_rule() use audit_find_rule()

[Amy Griffis]

Now that we've added audit_find_rule(), make audit_del_rule() use it
too.

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

---

 kernel/auditfilter.c |  101 +++++++++++++++++++-------------------------------
 1 files changed, 39 insertions(+), 62 deletions(-)

49331df422d7d3f7b1d387c4373bb3113f45f04d
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 3cedc73..461f404 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1102,83 +1102,60 @@ error:
 	return err;
 }
 
-/* Rule removal helper.
- * Caller must hold audit_filter_mutex. */
-static inline int audit_do_del_rule(struct audit_entry *entry,
-				    struct list_head *list,
-				    struct list_head *inotify_list)
+/* Remove an existing rule from filterlist. */
+static inline int audit_del_rule(struct audit_entry *entry,
+				 struct list_head *list)
 {
 	struct audit_entry  *e;
+	struct audit_field *inode_f = entry->rule.inode_f;
+	struct audit_watch *watch, *tmp_watch = entry->rule.watch;
+	LIST_HEAD(inotify_list);
+	int h, ret = 0;
 
-	list_for_each_entry(e, list, list) {
-		struct audit_watch *watch = e->rule.watch;
+	if (inode_f) {
+		h = audit_hash_ino(inode_f->val);
+		list = &audit_inode_hash[h];
+	}
 
-		if (audit_compare_rule(&entry->rule, &e->rule))
-			continue;
+	mutex_lock(&audit_filter_mutex);
+	e = audit_find_rule(entry, list);
+	if (!e) {
+		mutex_unlock(&audit_filter_mutex);
+		ret = -ENOENT;
+		goto out;
+	}
 
-		if (watch) {
-			struct audit_parent *parent = watch->parent;
+	watch = e->rule.watch;
+	if (watch) {
+		struct audit_parent *parent = watch->parent;
 
-			list_del(&e->rule.rlist);
+		list_del(&e->rule.rlist);
 
-			if (list_empty(&watch->rules)) {
-				audit_remove_watch(watch);
+		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);
-				}
+			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);
-
-		return 0;
 	}
-	return -ENOENT;		/* No matching rule */
-}
 
-/* Remove an existing rule from filterlist. */
-static inline int audit_del_rule(struct audit_entry *entry,
-				 struct list_head *list)
-{
-	int h, ret;
-	struct audit_field *inode_f = entry->rule.inode_f;
-	struct audit_watch *watch = entry->rule.watch;
-	LIST_HEAD(inotify_list);
+	list_del_rcu(&e->list);
+	call_rcu(&e->rcu, audit_free_rule_rcu);
 
-	if (watch) {
-		mutex_lock(&audit_filter_mutex);
-		/* we don't know the inode number, so must walk entire hash */
-		for (h = 0; h < AUDIT_INODE_BUCKETS; h++) {
-			list = &audit_inode_hash[h];
-			ret = audit_do_del_rule(entry, list, &inotify_list);
-			if (!ret)
-				break;
-		}
-		mutex_unlock(&audit_filter_mutex);
-
-		if (!list_empty(&inotify_list))
-			audit_inotify_unregister(&inotify_list);
+	mutex_unlock(&audit_filter_mutex);
 
-		/* match initial get for tmp watch */
-		audit_put_watch(watch);
+	if (!list_empty(&inotify_list))
+		audit_inotify_unregister(&inotify_list);
 
-	} else {
-		if (inode_f) {
-			h = audit_hash_ino(inode_f->val);
-			list = &audit_inode_hash[h];
-		}
-		mutex_lock(&audit_filter_mutex);
-		ret = audit_do_del_rule(entry, list, &inotify_list);
-		mutex_unlock(&audit_filter_mutex);
-	}
+out:
+	if (tmp_watch)
+		audit_put_watch(tmp_watch); /* match initial get */
 
 	return ret;
 }
-- 
1.3.0