This patch fixes the following issues in the latest filesystem audit
patch:
- fix error handling on call to audit_init_watch()
- remove unnecessary check for watch in audit_add_rule()
- in audit_del_rule(), don't check e->rule.watch after calling
audit_free_rule_rcu
- always update watch filter fields in audit_add_watch(), in case
we don't find an existing watch struct
- don't drop audit_filter_mutex between adding to existing
watches/parents and adding the rule to the filterlist
If these fixes look correct, please fold them in with lspp.b7
f11fcf7556067b41d592d970f5f5c6ffbdd1e8fd on next rebase.
Signed-off-by: Amy Griffis <[EMAIL PROTECTED]>
---
auditfilter.c | 144 ++++++++++++++++++++++++++++++++++------------------------
1 files changed, 86 insertions(+), 58 deletions(-)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index f1151a2..eb102ff 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -585,9 +585,9 @@ static inline struct audit_watch *audit_
return ERR_PTR(-ENOMEM);
new = audit_init_watch(path);
- if (unlikely(!new)) {
+ if (unlikely(IS_ERR(new))) {
kfree(path);
- return ERR_PTR(-ENOMEM);
+ goto out;
}
new->dev = old->dev;
@@ -595,6 +595,7 @@ static inline struct audit_watch *audit_
audit_get_parent(old->parent);
new->parent = old->parent;
+out:
return new;
}
@@ -853,77 +854,103 @@ static inline void audit_put_nd(struct n
}
}
-/* Find a matching watch entry, or add this one. */
-static inline int audit_add_watch(struct audit_krule *krule,
- struct nameidata *ndp, struct nameidata *ndw,
- struct list_head *inotify_list)
+/* Add a parent inotify_watch for the given rule. */
+static int audit_add_parent(struct audit_krule *krule,
+ struct list_head *inotify_list)
+{
+ struct audit_parent *parent;
+ struct audit_watch *watch = krule->watch;
+
+ parent = audit_init_parent();
+ if (IS_ERR(parent))
+ return PTR_ERR(parent);
+
+ audit_get_parent(parent);
+ watch->parent = parent;
+
+ /* krule, watch and parent have not been added to any global
+ * lists, so we don't need to take audit_filter_mutex. */
+ list_add(&watch->wlist, &parent->watches);
+ list_add(&krule->rlist, &watch->rules);
+
+ /* add parent to inotify registration list */
+ list_add(&parent->ilist, inotify_list);
+
+ return 0;
+}
+
+/* Associate the given rule with an existing parent inotify_watch.
+ * Caller must hold audit_filter_mutex. */
+static int audit_add_to_parent(struct audit_krule *krule,
+ struct inotify_watch *iwatch)
{
- int ret;
- struct inotify_watch *iwatch;
struct audit_parent *parent;
struct audit_watch *w, *watch = krule->watch;
int watch_found = 0;
- ret = inotify_find_watch(audit_ih, ndp->dentry->d_inode, &iwatch);
- if (ret < 0) {
- parent = audit_init_parent();
- if (IS_ERR(parent))
- return PTR_ERR(parent);
+ parent = container_of(iwatch, struct audit_parent, wdata);
- audit_get_parent(parent);
- watch->parent = parent;
+ /* parent was moved before we took audit_filter_mutex */
+ if (parent->flags & AUDIT_PARENT_INVALID)
+ return -ENOENT;
- /* krule, watch and parent have not been added to any global
- * lists, so we don't need to take audit_filter_mutex.
- */
- list_add(&watch->wlist, &parent->watches);
- list_add(&krule->rlist, &watch->rules);
+ list_for_each_entry(w, &parent->watches, wlist) {
+ if (strcmp(watch->path, w->path))
+ continue;
- /* update watch filter fields */
- if (ndw) {
- watch->dev = ndw->dentry->d_inode->i_sb->s_dev;
- watch->ino = ndw->dentry->d_inode->i_ino;
- }
-
- /* add parent to inotify registration list */
- list_add(&parent->ilist, inotify_list);
- } else {
- parent = container_of(iwatch, struct audit_parent, wdata);
+ watch_found = 1;
- mutex_lock(&audit_filter_mutex);
- if (parent->flags & AUDIT_PARENT_INVALID) {
- /* parent was moved before we took the lock */
- mutex_unlock(&audit_filter_mutex);
- return -ENOENT;
- }
+ /* put krule's and initial refs to temporary watch */
+ audit_put_watch(watch);
+ audit_put_watch(watch);
- list_for_each_entry(w, &parent->watches, wlist) {
- if (strcmp(watch->path, w->path))
- continue;
+ audit_get_watch(w);
+ krule->watch = watch = w;
+ break;
+ }
- watch_found = 1;
+ if (!watch_found) {
+ audit_get_parent(parent);
+ watch->parent = parent;
- /* put krule's and initial refs to temporary watch */
- audit_put_watch(watch);
- audit_put_watch(watch);
+ list_add(&watch->wlist, &parent->watches);
+ }
- audit_get_watch(w);
- krule->watch = watch = w;
- break;
- }
+ list_add(&krule->rlist, &watch->rules);
- if (!watch_found) {
- audit_get_parent(parent);
- watch->parent = parent;
+ return 0;
+}
- list_add(&watch->wlist, &parent->watches);
- }
+/* Find a matching watch entry, or add this one.
+ * Caller must hold audit_filter_mutex. */
+static int audit_add_watch(struct audit_krule *krule, struct nameidata *ndp,
+ struct nameidata *ndw,
+ struct list_head *inotify_list)
+{
+ struct audit_watch *watch = krule->watch;
+ struct inotify_watch *iwatch;
+ int ret;
- list_add(&krule->rlist, &watch->rules);
- mutex_unlock(&audit_filter_mutex);
+ /* update watch filter fields */
+ if (ndw) {
+ watch->dev = ndw->dentry->d_inode->i_sb->s_dev;
+ watch->ino = ndw->dentry->d_inode->i_ino;
}
- return 0;
+ /* The audit_filter_mutex must not be held during inotify calls because
+ * we hold it during inotify event callback processing.
+ * We can trust iwatch to stick around because we hold nameidata (ndp).
*/
+ mutex_unlock(&audit_filter_mutex);
+
+ if (inotify_find_watch(audit_ih, ndp->dentry->d_inode, &iwatch) < 0) {
+ ret = audit_add_parent(krule, inotify_list);
+ mutex_lock(&audit_filter_mutex);
+ } else {
+ mutex_lock(&audit_filter_mutex);
+ ret = audit_add_to_parent(krule, iwatch);
+ }
+
+ return ret;
}
/* Add rule to given filterlist if not a duplicate. */
@@ -954,7 +981,9 @@ static inline int audit_add_rule(struct
goto error;
}
+ mutex_lock(&audit_filter_mutex);
if (watch) {
+ /* audit_filter_mutex is dropped and re-taken during this call
*/
err = audit_add_watch(&entry->rule, ndp, ndw, &inotify_list);
if (err) {
audit_put_nd(ndp, ndw);
@@ -962,7 +991,6 @@ static inline int audit_add_rule(struct
}
}
- mutex_lock(&audit_filter_mutex);
if (entry->rule.flags & AUDIT_FILTER_PREPEND) {
list_add_rcu(&entry->list, list);
} else {
@@ -970,7 +998,7 @@ static inline int audit_add_rule(struct
}
mutex_unlock(&audit_filter_mutex);
- if (watch && !list_empty(&inotify_list)) {
+ if (!list_empty(&inotify_list)) {
err = audit_inotify_register(ndp, &inotify_list);
if (err)
goto error;
@@ -1031,7 +1059,7 @@ static inline int audit_del_rule(struct
call_rcu(&e->rcu, audit_free_rule_rcu);
mutex_unlock(&audit_filter_mutex);
- if (e->rule.watch && !list_empty(&inotify_list))
+ if (!list_empty(&inotify_list))
audit_inotify_unregister(&inotify_list);
return 0;
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
