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 <amy.griffis(a)hp.com&gt; --- 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;