From: Alexander Viro <aviro@redhat.com> Subject: [RHEL 5.1 PATCH] bz#248416 Date: Tue, 7 Aug 2007 19:08:21 -0400 Bugzilla: 248416 Message-Id: <20070807230821.GW13539@devserv.devel.redhat.com> Changelog: [audit] sub-tree cleanups That should be a followup to 182624, actually; fixes a bunch of bugs in there. diff -urN linux-2.6.18.i686/kernel/auditfilter.c linux-2.6.18.i686-fix/kernel/auditfilter.c --- linux-2.6.18.i686/kernel/auditfilter.c 2007-08-04 16:34:34.000000000 -0400 +++ linux-2.6.18.i686-fix/kernel/auditfilter.c 2007-08-04 16:03:53.000000000 -0400 @@ -628,7 +628,7 @@ goto exit_free; entry->rule.buflen += f->val; - err = audit_add_tree_rule(&entry->rule, str, f->op); + err = audit_make_tree(&entry->rule, str, f->op); if (err) { kfree(str); goto exit_free; @@ -920,6 +920,14 @@ new->inode_f = old->inode_f; new->watch = NULL; new->field_count = old->field_count; + /* + * note that we are OK with not refcounting here; audit_match_tree() + * never dereferences tree and we can't get false positives there + * since we'd have to have rule gone from the list *and* removed + * before the chunks found by lookup had been allocated, i.e. before + * the beginning of list scan. + */ + new->tree = old->tree; memcpy(new->fields, old->fields, sizeof(struct audit_field) * fcount); /* deep copy this information, updating the se_rule fields, because @@ -1247,6 +1255,7 @@ struct audit_entry *e; struct audit_field *inode_f = entry->rule.inode_f; struct audit_watch *watch = entry->rule.watch; + struct audit_tree *tree = entry->rule.tree; struct nameidata *ndp, *ndw; int h, err, putnd_needed = 0; #ifdef CONFIG_AUDITSYSCALL @@ -1268,6 +1277,9 @@ mutex_unlock(&audit_filter_mutex); if (e) { err = -EEXIST; + /* normally audit_add_tree_rule() will free it on failure */ + if (tree) + audit_put_tree(tree); goto error; } @@ -1290,6 +1302,13 @@ h = audit_hash_ino((u32)watch->ino); list = &audit_inode_hash[h]; } + if (tree) { + err = audit_add_tree_rule(&entry->rule); + if (err) { + mutex_unlock(&audit_filter_mutex); + goto error; + } + } if (entry->rule.flags & AUDIT_FILTER_PREPEND) { list_add_rcu(&entry->list, list); @@ -1326,6 +1345,7 @@ struct audit_entry *e; struct audit_field *inode_f = entry->rule.inode_f; struct audit_watch *watch, *tmp_watch = entry->rule.watch; + struct audit_tree *tree = entry->rule.tree; LIST_HEAD(inotify_list); int h, ret = 0; #ifdef CONFIG_AUDITSYSCALL @@ -1391,6 +1411,8 @@ out: if (tmp_watch) audit_put_watch(tmp_watch); /* match initial get */ + if (tree) + audit_put_tree(tree); /* that's the temporary one */ return ret; } @@ -1806,7 +1828,7 @@ &watch->rules); list_del(&entry->rule.rlist); } else if (tree) - list_replace(&entry->rule.rlist, + list_replace_init(&entry->rule.rlist, &nentry->rule.rlist); list_replace_rcu(&entry->list, &nentry->list); } diff -urN linux-2.6.18.i686/kernel/audit.h linux-2.6.18.i686-fix/kernel/audit.h --- linux-2.6.18.i686/kernel/audit.h 2007-08-04 16:34:24.000000000 -0400 +++ linux-2.6.18.i686-fix/kernel/audit.h 2007-08-04 16:03:53.000000000 -0400 @@ -141,13 +141,15 @@ extern struct audit_chunk *audit_tree_lookup(const struct inode *); extern void audit_put_chunk(struct audit_chunk *); extern int audit_tree_match(struct audit_chunk *, struct audit_tree *); -extern int audit_add_tree_rule(struct audit_krule *, char *, u32); +extern int audit_make_tree(struct audit_krule *, char *, u32); +extern int audit_add_tree_rule(struct audit_krule *); extern int audit_remove_tree_rule(struct audit_krule *); extern void audit_trim_trees(void); extern int audit_tag_tree(char *old, char *new); extern void audit_schedule_prune(void); extern void audit_prune_trees(void); extern const char *audit_tree_path(struct audit_tree *); +extern void audit_put_tree(struct audit_tree *); extern char *audit_unpack_string(void **, size_t *, size_t); diff -urN linux-2.6.18.i686/kernel/audit_tree.c linux-2.6.18.i686-fix/kernel/audit_tree.c --- linux-2.6.18.i686/kernel/audit_tree.c 2007-08-04 16:34:34.000000000 -0400 +++ linux-2.6.18.i686-fix/kernel/audit_tree.c 2007-08-04 16:31:10.000000000 -0400 @@ -14,6 +14,7 @@ struct list_head rules; struct list_head list; struct list_head same_root; + struct rcu_head head; char pathname[]; }; @@ -52,8 +53,8 @@ * chunk.hash is a hash with middle bits of watch.inode as * a hash function. RCU, hash_lock * - * tree is refcounted; one reference for "some rules refer to it", one for - * each chunk with pointer to it. + * tree is refcounted; one reference for "some rules on rules_list refer to + * it", one for each chunk with pointer to it. * * chunk is refcounted by embedded inotify_watch. * @@ -77,6 +78,7 @@ INIT_LIST_HEAD(&tree->rules); INIT_LIST_HEAD(&tree->list); INIT_LIST_HEAD(&tree->same_root); + tree->root = NULL; strcpy(tree->pathname, s); } return tree; @@ -87,12 +89,16 @@ atomic_inc(&tree->count); } +static void __put_tree(struct rcu_head *rcu) +{ + struct audit_tree *tree = container_of(rcu, struct audit_tree, head); + kfree(tree); +} + static inline void put_tree(struct audit_tree *tree) { - if (atomic_dec_and_test(&tree->count)) { - synchronize_rcu(); - kfree(tree); - } + if (atomic_dec_and_test(&tree->count)) + call_rcu(&tree->head, __put_tree); } /* to avoid bringing the entire thing in audit.h */ @@ -294,7 +300,7 @@ list_add(&chunk->owners[0].list, &tree->chunks); if (!tree->root) { tree->root = chunk; - list_add(&chunk->trees, &tree->same_root); + list_add(&tree->same_root, &chunk->trees); } insert_hash(chunk); spin_unlock(&hash_lock); @@ -366,7 +372,7 @@ old->dead = 1; if (!tree->root) { tree->root = chunk; - list_add(&chunk->trees, &tree->same_root); + list_add(&tree->same_root, &chunk->trees); } spin_unlock(&hash_lock); inotify_evict_watch(&old->watch); @@ -388,11 +394,10 @@ struct audit_entry *entry; struct audit_buffer *ab; - mutex_lock(&audit_filter_mutex); list_for_each_entry_safe(rule, next, &tree->rules, rlist) { entry = container_of(rule, struct audit_entry, rule); - list_del(&rule->rlist); + list_del_init(&rule->rlist); if (rule->tree) { /* not a half-baked one */ ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE); @@ -410,7 +415,6 @@ call_rcu(&entry->rcu, audit_free_rule_rcu); } } - mutex_unlock(&audit_filter_mutex); } /* @@ -589,37 +593,49 @@ return is_subdir(dentry, nd->dentry); } -int audit_add_tree_rule(struct audit_krule *rule, char *pathname, u32 op) +int audit_make_tree(struct audit_krule *rule, char *pathname, u32 op) { - struct audit_tree *tree; - struct nameidata nd; - struct vfsmount *mnt, *p; - struct list_head list; - int err; if (pathname[0] != '/' || rule->listnr != AUDIT_FILTER_EXIT || op & ~AUDIT_EQUAL || rule->inode_f || rule->watch || rule->tree) return -EINVAL; + rule->tree = alloc_tree(pathname); + if (!rule->tree) + return -ENOMEM; + return 0; +} + +void audit_put_tree(struct audit_tree *tree) +{ + put_tree(tree); +} + +/* called with audit_filter_mutex */ +int audit_add_tree_rule(struct audit_krule *rule) +{ + struct audit_tree *seed = rule->tree, *tree; + struct nameidata nd; + struct vfsmount *mnt, *p; + struct list_head list; + int err; - mutex_lock(&audit_filter_mutex); list_for_each_entry(tree, &tree_list, list) { - if (!strcmp(pathname, tree->pathname)) { - get_tree(tree); + if (!strcmp(seed->pathname, tree->pathname)) { + put_tree(seed); rule->tree = tree; list_add(&rule->rlist, &tree->rules); - mutex_unlock(&audit_filter_mutex); return 0; } } - tree = alloc_tree(pathname); + tree = seed; list_add(&tree->list, &tree_list); list_add(&rule->rlist, &tree->rules); /* do not set rule->tree yet */ mutex_unlock(&audit_filter_mutex); - err = path_lookup(pathname, 0, &nd); + err = path_lookup(tree->pathname, 0, &nd); if (err) goto Err; mnt = collect_mounts(nd.mnt, nd.dentry); @@ -653,12 +669,10 @@ mutex_lock(&audit_filter_mutex); if (list_empty(&rule->rlist)) { - mutex_unlock(&audit_filter_mutex); put_tree(tree); return -ENOENT; } rule->tree = tree; - mutex_unlock(&audit_filter_mutex); put_tree(tree); return 0; @@ -666,7 +680,6 @@ mutex_lock(&audit_filter_mutex); list_del_init(&tree->list); list_del_init(&tree->rules); - mutex_unlock(&audit_filter_mutex); put_tree(tree); return err; }