[WIP][PATCH] Audit inotify client
Amy Griffis
amy.griffis at hp.com
Mon Feb 6 20:04:51 UTC 2006
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;
More information about the Linux-audit
mailing list