Hello, This patch allows audit to operate as an inotify client. It adds a list of parents, which represent the dentry parents of the filesystem locations to be watched. When created, a parent registers an inotify watch on itself. If all the audit rules corresponding to a parent are removed by the admin, the parent removes its inotify watch before it is destroyed. Audit's inotify callback, audit_handle_fsevent(), is called following a specified group of inotify events. These events translate to one of two activities for audit. It may update rules in the syscall exit filterlist with a new inode number or the value -1, or it may implicitly remove all watches and rules associated with a particular parent (that has been removed from the filesystem). The patch is based off of Al Viro's audit git tree, applying after these previously posted patches: audit rule interface changes: http://www.redhat.com/archives/linux-audit/2006-January/msg00064.html http://www.redhat.com/archives/linux-audit/2006-January/msg00082.html inotify kernel api: https://www.redhat.com/archives/linux-audit/2006-January/msg00084.html This patch is still a work in progress. There are a couple of race conditions that need to be properly handled, as well as some additional synchronization needed for concurrent manipulations of the filterlist (list_del_rcu, list_update_rcu, etc.). For this I'm planning to add a per-element spinlock to be taken for list manipulations only. Before finishing up these last pieces, I wanted to post my current work for comments on the locking approach. Please have a look and let me know what you think. Thanks, Amy diff --git a/kernel/audit.c b/kernel/audit.c index bdda766..e8b6b8f 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -55,6 +55,9 @@ #include <net/netlink.h> #include <linux/skbuff.h> #include <linux/netlink.h> +#include <linux/inotify.h> + +#include "audit.h" /* No auditing will take place until audit_initialized != 0. * (Initialization happens after skb_init is called.) */ @@ -99,6 +102,9 @@ static atomic_t audit_lost = ATOMIC_I /* The netlink socket. */ static struct sock *audit_sock; +/* Inotify device. */ +struct inotify_device *audit_idev; + /* The audit_freelist is a list of pre-allocated audit buffers (if more * than AUDIT_MAXFREE are in use, the audit buffer is freed instead of * being placed on the freelist). */ @@ -564,6 +570,11 @@ static int __init audit_init(void) audit_initialized = 1; audit_enabled = audit_default; audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized"); + + audit_idev = inotify_init(audit_handle_fsevent); + if (IS_ERR(audit_idev)) + audit_panic("cannot initialize inotify device"); + return 0; } __initcall(audit_init); diff --git a/kernel/audit.h b/kernel/audit.h index 5033e1f..e6a3135 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -22,6 +22,8 @@ #include <linux/fs.h> #include <linux/audit.h> +struct inotify_event; + /* 0 = no checking 1 = put_count checking 2 = verbose put_count checking @@ -52,10 +54,22 @@ enum audit_state { }; /* Rule lists */ +struct audit_parent { + atomic_t count; /* reference count */ + unsigned long ino; /* associated inode number */ + u32 wd; /* inotify watch descriptor */ + struct list_head mlist; /* entry in master_parents */ + struct list_head watches; /* associated watches */ + struct semaphore watches_sem; /* protects parent's watches list*/ +}; + struct audit_watch { + atomic_t count; /* reference count */ char *path; /* watch insertion path */ - struct list_head mlist; /* entry in master_watchlist */ struct list_head rules; /* associated rules */ + struct semaphore rules_sem; /* protects watch's rules list */ + struct list_head wlist; /* entry in audit_parent.watches list*/ + struct audit_parent *parent; /* associated parent */ }; struct audit_field { @@ -86,7 +100,9 @@ struct audit_entry { extern int audit_pid; extern int audit_comparator(const u32 left, const u32 op, const u32 right); - +extern void audit_handle_fsevent(struct inotify_event *event, + const char *name, struct inode * inode, + void *ptr); extern void audit_send_reply(int pid, int seq, int type, int done, int multi, void *payload, int size); diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 6506084..dbe0c98 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -25,6 +25,7 @@ #include <linux/fs.h> #include <linux/namei.h> #include <linux/netlink.h> +#include <linux/inotify.h> #include "audit.h" /* There are three lists of rules -- one to search at task creation @@ -42,7 +43,84 @@ struct list_head audit_filter_list[AUDIT #endif }; -static LIST_HEAD(master_watchlist); +static LIST_HEAD(master_parents); +static DEFINE_SPINLOCK(master_parents_lock); + +/* Inotify device. */ +extern struct inotify_device *audit_idev; + +/* Inotify events we care about. */ +#define AUDIT_FSEVENTS IN_MOVE|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF + +static inline void audit_get_parent(struct audit_parent *parent) +{ + atomic_inc(&parent->count); +} + +static inline void audit_put_parent(struct audit_parent *parent) +{ + if (atomic_dec_and_test(&parent->count)) + kfree(parent); +} + +static inline void audit_get_watch(struct audit_watch *watch) +{ + atomic_inc(&watch->count); +} + +static inline void audit_put_watch(struct audit_watch *watch) +{ + if (atomic_dec_and_test(&watch->count)) { + if (watch->parent) + audit_put_parent(watch->parent); + kfree(watch->path); + kfree(watch); + } +} + +/* Initialize a parent watch entry. Caller must pin inode (via nameidata). */ +static inline struct audit_parent *audit_init_parent(struct inode *inode) +{ + struct audit_parent *parent; + u32 wd; + + parent = kmalloc(sizeof(*parent), GFP_KERNEL); + if (unlikely(!parent)) + return ERR_PTR(-ENOMEM); + + INIT_LIST_HEAD(&parent->watches); + init_MUTEX(&parent->watches_sem); + atomic_set(&parent->count, 0); + parent->ino = inode->i_ino; + audit_get_parent(parent); + + wd = inotify_add_watch(audit_idev, inode, AUDIT_FSEVENTS, parent); + if (wd < 0) { + audit_put_parent(parent); + return ERR_PTR(wd); + } + parent->wd = wd; + + return parent; +} + +/* Initialize a watch entry. */ +static inline struct audit_watch *audit_init_watch(char *path) +{ + struct audit_watch *watch; + + watch = kmalloc(sizeof(*watch), GFP_KERNEL); + if (unlikely(!watch)) + return ERR_PTR(-ENOMEM); + + INIT_LIST_HEAD(&watch->rules); + init_MUTEX(&watch->rules_sem); + atomic_set(&watch->count, 0); + watch->path = path; + audit_get_watch(watch); + + return watch; +} /* Unpack a filter field's string representation from user-space * buffer. */ @@ -75,7 +153,6 @@ static char *audit_unpack_string(void ** static int audit_to_watch(char *path, struct audit_krule *krule, int fidx) { struct audit_field *f = &krule->fields[fidx]; - struct nameidata nd; struct audit_watch *watch; if (path[0] != '/' || path[f->val-1] == '/' || @@ -83,17 +160,13 @@ static int audit_to_watch(char *path, st f->op & ~AUDIT_EQUAL) return -EINVAL; - if (path_lookup(path, 0, &nd) == 0) - f->val = nd.dentry->d_inode->i_ino; - else - f->val = (unsigned int)-1; - path_release(&nd); + watch = audit_init_watch(path); + if (unlikely(IS_ERR(watch))) + return PTR_ERR(watch); - watch = kmalloc(sizeof(*watch), GFP_KERNEL); - if (unlikely(!watch)) - return -ENOMEM; - watch->path = path; + audit_get_watch(watch); krule->watch = watch; + f->val = (unsigned int)-1; return 0; } @@ -325,7 +398,7 @@ static inline int audit_compare_watch(st * don't match. */ static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b) { - int i; + int i, nomatch; if (a->flags != b->flags || a->listnr != b->listnr || @@ -340,7 +413,8 @@ static int audit_compare_rule(struct aud switch(a->fields[i].type) { case AUDIT_WATCH: - if (audit_compare_watch(a->watch, b->watch)) + nomatch = audit_compare_watch(a->watch, b->watch); + if (nomatch) return 1; break; default: @@ -356,36 +430,184 @@ static int audit_compare_rule(struct aud return 0; } -static inline void audit_free_watch(struct audit_watch *watch) +static inline void audit_free_rule(struct audit_entry *e) { - kfree(watch->path); - kfree(watch); + if (e->rule.watch) + audit_put_watch(e->rule.watch); + kfree(e); } -static inline void audit_free_rule(struct rcu_head *head) +static inline void audit_free_rule_rcu(struct rcu_head *head) { struct audit_entry *e = container_of(head, struct audit_entry, rcu); - kfree(e); + audit_free_rule(e); +} + +static void audit_update_field(struct audit_krule *krule, u32 type, u32 val) +{ + int i; + struct audit_entry *old_e, *entry; + + for (i = 0; i < AUDIT_MAX_FIELDS; i++) + if (krule->fields[i].type == type) { + entry = kmalloc(sizeof(*entry), GFP_KERNEL); + if (unlikely(!entry)) + return; /* XXX */ + + old_e = container_of(krule, struct audit_entry, rule); + memcpy(entry, old_e, sizeof(struct audit_entry)); + entry->rule.fields[i].val = val; + + list_replace_rcu(&old_e->list, &entry->list); + call_rcu(&old_e->rcu, audit_free_rule_rcu); + return; + } +} + +static inline void audit_handle_update(struct audit_parent *parent, + const char *name, u32 ino) +{ + struct audit_watch *w; + struct audit_krule *r; + + audit_get_parent(parent); + down(&parent->watches_sem); + list_for_each_entry(w, &parent->watches, wlist) { + audit_get_watch(w); + if (strcmp(w->path, name)) { /* XXX */ + audit_put_watch(w); + continue; + } + + down(&w->rules_sem); + list_for_each_entry(r, &w->rules, rlist) + audit_update_field(r, AUDIT_WATCH, ino); + up(&w->rules_sem); + audit_put_watch(w); + } + up(&parent->watches_sem); + audit_put_parent(parent); } -/* Attach krule's watch to master_watchlist, using existing watches - * when possible. */ -static inline void audit_add_watch(struct audit_krule *krule) +static inline void audit_handle_removal(struct audit_parent *parent) { struct audit_watch *w; + struct audit_krule *r; + struct audit_entry *e; + + audit_get_parent(parent); + down(&parent->watches_sem); + list_for_each_entry(w, &parent->watches, wlist) { + audit_get_watch(w); + down(&w->rules_sem); + list_for_each_entry(r, &w->rules, rlist) { + e = container_of(r, struct audit_entry, rule); + list_del_rcu(&e->list); + call_rcu(&e->rcu, audit_free_rule_rcu); + } + up(&w->rules_sem); + audit_put_watch(w); + } + spin_lock(&master_parents_lock); + list_del(&parent->mlist); + spin_unlock(&master_parents_lock); + up(&parent->watches_sem); + audit_put_parent(parent); + +} - list_for_each_entry(w, &master_watchlist, mlist) { - if (audit_compare_watch(w, krule->watch)) +void audit_handle_fsevent(struct inotify_event *event, const char *name, + struct inode *inode, void *ptr) +{ + struct audit_parent *parent = (struct audit_parent *)ptr; + + if (event->mask & (IN_CREATE|IN_MOVED_TO) && inode) + audit_handle_update(parent, name, (unsigned int)inode->i_ino); + else if (event->mask & (IN_DELETE|IN_MOVED_FROM)) + audit_handle_update(parent, name, (unsigned int)-1); + else if (event->mask & (IN_DELETE_SELF|IN_MOVE_SELF)) + audit_handle_removal(parent); +} + +/* Find an existing parent entry for this watch, or create a new one. */ +static inline struct audit_parent *audit_find_parent(struct inode *inode) +{ + struct audit_parent *p, *next, *parent; + + list_for_each_entry_safe(p, next, &master_parents, mlist) { + if (p->ino != inode->i_ino) continue; - audit_free_watch(krule->watch); - krule->watch = w; - list_add(&krule->rlist, &w->rules); - return; - } - INIT_LIST_HEAD(&krule->watch->rules); - list_add(&krule->rlist, &krule->watch->rules); - list_add(&krule->watch->mlist, &master_watchlist); + audit_get_parent(p); /* hold ref until we take watches_sem */ + parent = p; + goto out; + } + + parent = audit_init_parent(inode); + if (unlikely(IS_ERR(parent))) + goto out; + + audit_get_parent(parent); /* hold ref until we take watches_sem */ + + spin_lock(&master_parents_lock); + list_add(&parent->mlist, &master_parents); + spin_unlock(&master_parents_lock); + +out: + return parent; +} + +/* Find a matching watch entry, or add this one. */ +static inline int audit_add_watch(struct audit_krule *krule) +{ + struct nameidata nd; + struct audit_parent *parent; + struct audit_watch *w, *watch = krule->watch; + int ret = 0; + + ret = path_lookup(watch->path, LOOKUP_PARENT, &nd); + if (ret) + goto out; + + parent = audit_find_parent(nd.dentry->d_inode); + if (IS_ERR(parent)) { + ret = PTR_ERR(parent); + path_release(&nd); + goto out; + } + path_release(&nd); + + down(&parent->watches_sem); + audit_put_parent(parent); + list_for_each_entry(w, &parent->watches, wlist) { + if (audit_compare_watch(watch, w)) + continue; + + audit_put_watch(watch); /* krule's ref */ + audit_put_watch(watch); /* destroy */ + + audit_get_watch(w); + krule->watch = watch = w; + goto add_rule; + } + + audit_get_parent(parent); + watch->parent = parent; + list_add(&watch->wlist, &parent->watches); + +add_rule: + down(&watch->rules_sem); + list_add(&krule->rlist, &watch->rules); + up(&watch->rules_sem); + up(&parent->watches_sem); + + if (path_lookup(watch->path, 0, &nd) == 0) + audit_update_field(krule, AUDIT_WATCH, + nd.dentry->d_inode->i_ino); + path_release(&nd); + +out: + return ret; } /* Add rule to given filterlist if not a duplicate. Protected by @@ -394,16 +616,25 @@ static inline int audit_add_rule(struct struct list_head *list) { struct audit_entry *e; + int err; - /* Do not use the _rcu iterator here, since this is the only - * addition routine. */ - list_for_each_entry(e, list, list) { - if (!audit_compare_rule(&entry->rule, &e->rule)) - return -EEXIST; + /* The *_rcu iterator is needed to protect from filesystem + * updates or removals. */ + rcu_read_lock(); + list_for_each_entry_rcu(e, list, list) { + if (!audit_compare_rule(&entry->rule, &e->rule)) { + rcu_read_unlock(); + err = -EEXIST; + goto error; + } } + rcu_read_unlock(); - if (entry->rule.watch) - audit_add_watch(&entry->rule); + if (entry->rule.watch) { + err = audit_add_watch(&entry->rule); + if (err) + goto error; + } if (entry->rule.flags & AUDIT_FILTER_PREPEND) { list_add_rcu(&entry->list, list); } else { @@ -411,20 +642,48 @@ static inline int audit_add_rule(struct } return 0; + +error: + if (entry->rule.watch) + audit_put_watch(entry->rule.watch); + return err; } -/* Detach watch from krule, freeing if it has no associated rules. */ -static inline void audit_detach_watch(struct audit_krule *krule) +/* Remove given krule from watch */ +static inline void audit_remove_watch_krule(struct audit_krule *krule) { struct audit_watch *watch = krule->watch; + struct audit_parent *parent = krule->watch->parent; + + audit_get_parent(parent); + audit_get_watch(watch); + + down(&parent->watches_sem); + down(&watch->rules_sem); - list_del(&krule->rlist); - krule->watch = NULL; + if (!list_empty(&watch->rules)) + list_del(&krule->rlist); if (list_empty(&watch->rules)) { - list_del(&watch->mlist); - audit_free_watch(watch); + if (!list_empty(&parent->watches)) + list_del(&watch->wlist); + + if (list_empty(&parent->watches)) { + spin_lock(&master_parents_lock); + list_del(&parent->mlist); + spin_unlock(&master_parents_lock); + + inotify_ignore(audit_idev, parent->wd); + audit_put_parent(parent); + } + audit_put_watch(watch); } + + up(&watch->rules_sem); + up(&parent->watches_sem); + + audit_put_watch(watch); + audit_put_parent(parent); } /* Remove an existing rule from filterlist. Protected by @@ -433,19 +692,29 @@ static inline int audit_del_rule(struct struct list_head *list) { struct audit_entry *e; + int ret = 0; - /* Do not use the _rcu iterator here, since this is the only - * deletion routine. */ - list_for_each_entry(e, list, list) { - if (!audit_compare_rule(&entry->rule, &e->rule)) { - list_del_rcu(&e->list); - if (e->rule.watch) - audit_detach_watch(&e->rule); - call_rcu(&e->rcu, audit_free_rule); - return 0; - } + /* The *_rcu iterator is needed to protect from filesystem + * updates or removals. */ + rcu_read_lock(); + list_for_each_entry_rcu(e, list, list) { + if (audit_compare_rule(&entry->rule, &e->rule)) + continue; + + rcu_read_unlock(); + if (e->rule.watch) + audit_remove_watch_krule(&e->rule); + list_del_rcu(&e->list); + call_rcu(&e->rcu, audit_free_rule_rcu); + goto out; } - return -ENOENT; /* No matching rule */ + rcu_read_unlock(); + ret = -ENOENT; /* No matching rule */ + +out: + if (entry->rule.watch) + audit_put_watch(entry->rule.watch); + return ret; } /* List rules using struct audit_rule. Exists for backward @@ -463,10 +732,11 @@ static int audit_list(void *_dest) down(&audit_netlink_sem); - /* The *_rcu iterators not needed here because we are - always called with audit_netlink_sem held. */ + /* The *_rcu iterator is needed to protect from filesystem + * updates or removals. */ for (i=0; i<AUDIT_NR_FILTERS; i++) { - list_for_each_entry(entry, &audit_filter_list[i], list) { + rcu_read_lock(); + list_for_each_entry_rcu(entry, &audit_filter_list[i], list) { struct audit_rule *rule; rule = audit_krule_to_rule(&entry->rule); @@ -476,6 +746,7 @@ static int audit_list(void *_dest) rule, sizeof(*rule)); kfree(rule); } + rcu_read_unlock(); } audit_send_reply(pid, seq, AUDIT_LIST, 1, 1, NULL, 0); @@ -497,10 +768,11 @@ static int audit_list_rules(void *_dest) down(&audit_netlink_sem); - /* The *_rcu iterators not needed here because we are - always called with audit_netlink_sem held. */ + /* The *_rcu iterator is needed to protect from filesystem + * updates or removals. */ for (i=0; i<AUDIT_NR_FILTERS; i++) { - list_for_each_entry(e, &audit_filter_list[i], list) { + rcu_read_lock(); + list_for_each_entry_rcu(e, &audit_filter_list[i], list) { struct audit_rule_data *data; data = audit_krule_to_data(&e->rule); @@ -510,6 +782,7 @@ static int audit_list_rules(void *_dest) data, sizeof(*data) + data->buflen); kfree(data); } + rcu_read_unlock(); } audit_send_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0); @@ -574,11 +847,8 @@ int audit_receive_filter(int type, int p if (!err) audit_log(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE, "auid=%u added an audit rule\n", loginuid); - else { - if (entry->rule.watch) - audit_free_watch(entry->rule.watch); - kfree(entry); - } + else + audit_free_rule(entry); break; case AUDIT_DEL: case AUDIT_DEL_RULE: @@ -594,9 +864,7 @@ int audit_receive_filter(int type, int p if (!err) audit_log(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE, "auid=%u removed an audit rule\n", loginuid); - if (entry->rule.watch) - audit_free_watch(entry->rule.watch); - kfree(entry); + audit_free_rule(entry); break; default: return -EINVAL;