Discussion:
[PATCH] filesystem location based auditing
(too old to reply)
Amy Griffis
2006-02-24 20:19:02 UTC
Permalink
Hello,

This patch provides the functionality which allows a user to specify
audit rules targeting specific filesystem locations. It is an update
of the previously posted patch which provided functionality solely for
adding/removing rules with AUDIT_WATCH fields:

https://www.redhat.com/archives/linux-audit/2006-February/msg00034.html

The patch is intended to be a replacement for linux-2.6-audit-string-2.patch
in the current LSPP test kernel, and the git tree patch represented by
commit 7f54bafe959bcecda97a4a1e4a083a39fb4d9fc8 in branch amg.b0.
Since it depends on the inotify kernel API patch, the commits should
probably be re-ordered in the next branch.

This patch adds parent data structures for filesystem watches
specified in audit rules. A 'parent' is analogous to the dentry
parent for a watched filesystem location. When a parent is added, a
corresponding inotify watch is registered for it. The inotify event
handler specified during during initialization (audit_init) determines
an action to take for various events received from inotify.

The actions taken fall into two groups. When an event is received
which indicates a change at a watched filesystem location, audit
replaces rules in the AUDIT_FILTER_EXIT list with updated copies
containing the new inode # (or the invalid value on removal). When
inotify indicates the parent is being removed from the filesystem,
audit removes the parent, its associated watches, and their
associated rules.

Part of this functionality was also previously posted here as a WIP:

https://www.redhat.com/archives/linux-audit/2006-February/msg00030.html

This patch improves on the WIP patch in the following ways:

- Adds per-filterlist spinlocks to protect against concurrent
filterlist writes/manipulations.

- Improves code for copying rules for list_replace_rcu().

- Removes the per-watch and per-parent semaphores, which weren't
playing well with rcu.

- Moves calls to add/remove inotify watches out to separate
threads, which enables us to hold the filterlist lock through
the duration of operations to add or remove audit rules. I
attempted a solution involving dropping the filterlist locks
part way through, and found it to be difficult to handle the
races.

The locking model implemented in this patch consists of a two-level
lock hierarchy:

per-filterlist spinlock
parentlist spinlock

The per-filterlist spinlock protects the filterlist, the rules it
contains, and the watch.rules and parent.watches lists that are
associated with the filterlist. The contents of the watch structure
are write-once, so don't need an additional lock.

The parentlist spinlock protects the master_parents list and the data
in the parent structure, excluding the parent.watches list.

In this patch, only the AUDIT_FILTER_EXIT list lock is used, but I
added the others as I anticipate they will be used for rule updates
from SELinux policy reload.

This locking model seems to be sufficient to cover the potential race
conditions, but more granularity could be added for performance or
code clarity. I decided to keep it as simple as possible for the
initial implementation.

Limitations:

This implementation has the following known limitations:

- The dentry parent for a watched filesystem location must exist.
- Only one watch may be specified per rule.

Todo:

I anticipate some changes may need to be made to audit_update_rule()
and audit_dupe_rule() to properly accommodate the SELinux filtering
that Darrel and Dustin are implementing.

Additionally, the update of rules resulting from the removal of a file
at a watched filesystem location happens too early in the syscall
path. This causes the inode number for the rule to be invalidated
prior to syscall exit filtering, and the record that should have been
emitted is not. I'm still working on a good solution for this.

TIA for reviewing this patch.

Regards,
Amy

include/linux/audit.h | 1
kernel/audit.c | 22 +
kernel/audit.h | 34 ++
kernel/auditfilter.c | 700 +++++++++++++++++++++++++++++++++++++++++++++-----
kernel/auditsc.c | 53 +--
5 files changed, 709 insertions(+), 101 deletions(-)


diff --git a/include/linux/audit.h b/include/linux/audit.h
index c208554..d76fa58 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -148,6 +148,7 @@
#define AUDIT_INODE 102
#define AUDIT_EXIT 103
#define AUDIT_SUCCESS 104 /* exit >= 0; value ignored */
+#define AUDIT_WATCH 105

#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
diff --git a/kernel/audit.c b/kernel/audit.c
index 4eb97b6..82d926e 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,12 @@ static atomic_t audit_lost = ATOMIC_I
/* The netlink socket. */
static struct sock *audit_sock;

+/* Inotify device. */
+struct inotify_device *audit_idev;
+
+/* Audit filter lists, initialized in audit_init() */
+extern struct audit_flist audit_filter_list[];
+
/* 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). */
@@ -552,6 +561,14 @@ static void audit_receive(struct sock *s
/* Initialize audit support at boot time. */
static int __init audit_init(void)
{
+ int i;
+
+ /* must be initialized before any audit_log calls */
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ INIT_LIST_HEAD(&audit_filter_list[i].head);
+ spin_lock_init(&audit_filter_list[i].lock);
+ }
+
printk(KERN_INFO "audit: initializing netlink socket (%s)\n",
audit_default ? "enabled" : "disabled");
audit_sock = netlink_kernel_create(NETLINK_AUDIT, 0, audit_receive,
@@ -564,6 +581,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_ievent);
+ 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 4b602cd..ecb69d0 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,6 +54,23 @@ enum audit_state {
};

/* Rule lists */
+struct audit_parent {
+ atomic_t count; /* reference count */
+ unsigned int flags; /* flag in-process removals */
+ 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 audit_watch {
+ atomic_t count; /* reference count */
+ char *path; /* watch insertion path */
+ struct list_head rules; /* associated rules */
+ struct list_head wlist; /* entry in audit_parent.watches list*/
+ struct audit_parent *parent; /* associated parent */
+};
+
struct audit_field {
u32 type;
u32 val;
@@ -67,18 +86,29 @@ struct audit_krule {
u32 buflen; /* for data alloc on list rules */
u32 field_count;
struct audit_field *fields;
+ struct audit_watch *watch; /* associated watch */
+ struct list_head rlist; /* entry in audit_watch.rules list */
};

struct audit_entry {
struct list_head list;
struct rcu_head rcu;
- struct audit_krule rule;
+ struct audit_entry *replacement; /* used with list_replace_rcu() */
+ unsigned int flags; /* flag in-process adds and removals */
+ struct audit_krule rule; /* audit rule data */
};

+struct audit_flist {
+ struct list_head head;
+ spinlock_t lock; /* syncs filter data manipulation */
+};

extern int audit_pid;
extern int audit_comparator(const u32 left, const u32 op, const u32 right);
-
+extern int audit_compare_dname_path(const char *dname, const char *path);
+extern void audit_handle_ievent(struct inotify_event *event,
+ const char *dname, 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 5735acd..697a688 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -22,26 +22,65 @@
#include <linux/kernel.h>
#include <linux/audit.h>
#include <linux/kthread.h>
+#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
- * time, one to search at syscall entry time, and another to search at
- * syscall exit time. */
-struct list_head audit_filter_list[AUDIT_NR_FILTERS] = {
- LIST_HEAD_INIT(audit_filter_list[0]),
- LIST_HEAD_INIT(audit_filter_list[1]),
- LIST_HEAD_INIT(audit_filter_list[2]),
- LIST_HEAD_INIT(audit_filter_list[3]),
- LIST_HEAD_INIT(audit_filter_list[4]),
- LIST_HEAD_INIT(audit_filter_list[5]),
-#if AUDIT_NR_FILTERS != 6
-#error Fix audit_filter_list initialiser
-#endif
-};
+/* Audit filter lists */
+struct audit_flist audit_filter_list[AUDIT_NR_FILTERS];
+
+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_IN_WATCH IN_MOVE|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF
+#define AUDIT_IN_SELF IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT
+
+/* Flags for stale filterlist data */
+#define AUDIT_ENTRY_ADD 0x01 /* Rule entry addition in progress. */
+#define AUDIT_ENTRY_DEL 0x02 /* Rule entry deletion in progress. */
+#define AUDIT_PARENT_DEL 0x01 /* Parent deletion in progress. */
+
+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)) {
+ BUG_ON(!list_empty(&parent->watches));
+ 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)) {
+ BUG_ON(!list_empty(&watch->rules));
+ /* watches that were never added don't have a parent */
+ if (watch->parent)
+ audit_put_parent(watch->parent);
+ kfree(watch->path);
+ kfree(watch);
+ }
+}

static inline void audit_free_rule(struct audit_entry *e)
{
+ /* handle rules that don't have associated watches */
+ if (e->rule.watch)
+ audit_put_watch(e->rule.watch);
kfree(e->rule.fields);
kfree(e);
}
@@ -52,6 +91,190 @@ static inline void audit_free_rule_rcu(s
audit_free_rule(e);
}

+/* Remove all watches & rules associated with a parent that is going away. */
+static inline void audit_remove_parent_watches(struct audit_parent *parent)
+{
+ struct audit_watch *w, *nextw;
+ struct audit_krule *r, *nextr;
+ struct audit_entry *e;
+ struct audit_flist *flist = &audit_filter_list[AUDIT_FILTER_EXIT];
+
+ spin_lock(&flist->lock);
+ list_for_each_entry_safe(w, nextw, &parent->watches, wlist) {
+ list_for_each_entry_safe(r, nextr, &w->rules, rlist) {
+ e = container_of(r, struct audit_entry, rule);
+
+ /* make sure we have a valid copy */
+ while (e->replacement != NULL)
+ e = e->replacement;
+ if (e->flags & AUDIT_ENTRY_DEL)
+ continue;
+
+ list_del(&r->rlist);
+ list_del_rcu(&e->list);
+ e->flags |= AUDIT_ENTRY_DEL;
+ call_rcu(&e->rcu, audit_free_rule_rcu);
+ audit_log(NULL, GFP_ATOMIC, AUDIT_CONFIG_CHANGE,
+ "audit implicitly removed rule from list=%d\n",
+ AUDIT_FILTER_EXIT);
+ }
+ list_del(&w->wlist);
+ audit_put_watch(w);
+ }
+ spin_unlock(&flist->lock);
+}
+
+/* Actually remove the parent; inotify has acknowleged the removal. */
+static inline void audit_remove_parent(struct audit_parent *parent)
+{
+ BUG_ON(!list_empty(&parent->watches));
+ spin_lock(&master_parents_lock);
+ list_del(&parent->mlist);
+ audit_put_parent(parent);
+ spin_unlock(&master_parents_lock);
+}
+
+
+/* Register this parent's inotify watch. */
+static int audit_inotify_register(void *_data)
+{
+ void **data = _data;
+ struct audit_parent *parent;
+ char *path;
+ struct nameidata nd;
+ int err;
+ u32 wd;
+
+ parent = data[0];
+ path = data[1];
+ kfree(data);
+
+ err = path_lookup(path, LOOKUP_PARENT, &nd);
+ if (err)
+ goto handle_error;
+
+ wd = inotify_add_watch(audit_idev, nd.dentry->d_inode, AUDIT_IN_WATCH,
+ parent);
+ if (wd < 0)
+ goto handle_error;
+
+ spin_lock(&master_parents_lock);
+ parent->wd = wd;
+ parent->ino = nd.dentry->d_inode->i_ino;
+ spin_unlock(&master_parents_lock);
+
+ path_release(&nd);
+ audit_put_parent(parent);
+ return 0;
+
+handle_error:
+ path_release(&nd);
+ audit_remove_parent_watches(parent);
+ audit_remove_parent(parent);
+
+ audit_put_parent(parent);
+ return 0;
+}
+
+/* Unregister this parent's inotify watch. Generates an IN_IGNORED event. */
+static int audit_inotify_unregister(void *data)
+{
+ struct audit_parent *parent = data;
+ s32 wd;
+
+ spin_lock(&master_parents_lock);
+ wd = parent->wd;
+ spin_unlock(&master_parents_lock);
+
+ if (inotify_ignore(audit_idev, wd))
+ printk(KERN_ERR
+ "%s:%d: unable to remove inotify watch for inode %lu\n",
+ __FILE__, __LINE__, parent->ino);
+ audit_put_parent(parent);
+ return 0;
+}
+
+/* Initialize a parent watch entry. */
+static inline struct audit_parent *audit_init_parent(char *path,
+ unsigned long ino)
+{
+ struct audit_parent *parent;
+ void **data;
+ struct task_struct *task;
+
+ parent = kmalloc(sizeof(*parent), GFP_ATOMIC);
+ if (unlikely(!parent))
+ return ERR_PTR(-ENOMEM);
+
+ memset(parent, 0, sizeof(*parent));
+ INIT_LIST_HEAD(&parent->watches);
+ atomic_set(&parent->count, 0);
+ parent->ino = ino;
+ audit_get_parent(parent);
+
+ /* Spawn a thread to register the parent's inotify watch without
+ * the filterlist spinlock. */
+ data = kmalloc(2 * sizeof(void *), GFP_ATOMIC);
+ if (!data) {
+ audit_put_parent(parent);
+ return ERR_PTR(-ENOMEM);
+ }
+ data[0] = parent;
+ data[1] = path;
+ audit_get_parent(parent);
+ task = kthread_run(audit_inotify_register, data,
+ "audit_inotify_register");
+ if (IS_ERR(task)) {
+ audit_put_parent(parent);
+ return ERR_PTR(PTR_ERR(task));
+ }
+
+ 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);
+
+ memset(watch, 0, sizeof(*watch));
+ INIT_LIST_HEAD(&watch->rules);
+ atomic_set(&watch->count, 0);
+ watch->path = path;
+ audit_get_watch(watch);
+
+ return watch;
+}
+
+/* Initialize an audit filterlist entry. */
+static inline struct audit_entry *audit_init_entry(u32 field_count,
+ gfp_t gfp_mask)
+{
+ struct audit_entry *entry;
+ struct audit_field *fields;
+
+ entry = kmalloc(sizeof(*entry), gfp_mask);
+ if (unlikely(!entry))
+ return NULL;
+
+ fields = kmalloc(sizeof(*fields) * field_count, gfp_mask);
+ if (unlikely(!fields)) {
+ kfree(entry);
+ return NULL;
+ }
+
+ memset(entry, 0, sizeof(struct audit_entry));
+ memset(fields, 0, sizeof(struct audit_field) * field_count);
+
+ entry->rule.fields = fields;
+
+ return entry;
+}
+
/* Unpack a filter field's string representation from user-space
* buffer. */
static char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
@@ -79,12 +302,33 @@ static char *audit_unpack_string(void **
return str;
}

+/* Translate a watch string to kernel respresentation. */
+static int audit_to_watch(char *path, struct audit_krule *krule, int fidx)
+{
+ struct audit_field *f = &krule->fields[fidx];
+ struct audit_watch *watch;
+
+ if (path[0] != '/' || path[f->val-1] == '/' ||
+ krule->listnr != AUDIT_FILTER_EXIT ||
+ f->op & ~AUDIT_EQUAL)
+ return -EINVAL;
+
+ watch = audit_init_watch(path);
+ if (unlikely(IS_ERR(watch)))
+ return PTR_ERR(watch);
+
+ audit_get_watch(watch);
+ krule->watch = watch;
+ f->val = (unsigned int)-1;
+
+ return 0;
+}
+
/* Common user-space to kernel rule translation. */
static inline struct audit_entry *audit_to_entry_common(struct audit_rule *rule)
{
unsigned listnr;
struct audit_entry *entry;
- struct audit_field *fields;
int i, err;

err = -EINVAL;
@@ -108,23 +352,14 @@ static inline struct audit_entry *audit_
goto exit_err;

err = -ENOMEM;
- entry = kmalloc(sizeof(*entry), GFP_KERNEL);
- if (unlikely(!entry))
+ entry = audit_init_entry(rule->field_count, GFP_KERNEL);
+ if (!entry)
goto exit_err;
- fields = kmalloc(sizeof(*fields) * rule->field_count, GFP_KERNEL);
- if (unlikely(!fields)) {
- kfree(entry);
- goto exit_err;
- }
-
- memset(&entry->rule, 0, sizeof(struct audit_krule));
- memset(fields, 0, sizeof(struct audit_field));

entry->rule.flags = rule->flags & AUDIT_FILTER_PREPEND;
entry->rule.listnr = listnr;
entry->rule.action = rule->action;
entry->rule.field_count = rule->field_count;
- entry->rule.fields = fields;

for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
entry->rule.mask[i] = rule->mask[i];
@@ -150,15 +385,16 @@ static struct audit_entry *audit_rule_to
for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &entry->rule.fields[i];

- if (rule->fields[i] & AUDIT_UNUSED_BITS) {
- err = -EINVAL;
- goto exit_free;
- }
-
f->op = rule->fields[i] & (AUDIT_NEGATE|AUDIT_OPERATORS);
f->type = rule->fields[i] & ~(AUDIT_NEGATE|AUDIT_OPERATORS);
f->val = rule->values[i];

+ if (f->type & AUDIT_UNUSED_BITS ||
+ f->type == AUDIT_WATCH) {
+ err = -EINVAL;
+ goto exit_free;
+ }
+
entry->rule.vers_ops = (f->op & AUDIT_OPERATORS) ? 2 : 1;
if (f->op & AUDIT_NEGATE)
f->op |= AUDIT_NOT_EQUAL;
@@ -182,8 +418,9 @@ static struct audit_entry *audit_data_to
int err = 0;
struct audit_entry *entry;
void *bufp;
- /* size_t remain = datasz - sizeof(struct audit_rule_data); */
+ size_t remain = datasz - sizeof(struct audit_rule_data);
int i;
+ char *path;

entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
@@ -201,10 +438,20 @@ static struct audit_entry *audit_data_to

f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
+ f->val = data->values[i];
switch(f->type) {
- /* call type-specific conversion routines here */
- default:
- f->val = data->values[i];
+ case AUDIT_WATCH:
+ path = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(path))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = audit_to_watch(path, &entry->rule, i);
+ if (err) {
+ kfree(path);
+ goto exit_free;
+ }
+ break;
}
}

@@ -234,7 +481,8 @@ static struct audit_rule *audit_krule_to
struct audit_rule *rule;
int i;

- rule = kmalloc(sizeof(*rule), GFP_KERNEL);
+ /* use GFP_ATOMIC because we're under rcu_read_lock() */
+ rule = kmalloc(sizeof(*rule), GFP_ATOMIC);
if (unlikely(!rule))
return ERR_PTR(-ENOMEM);
memset(rule, 0, sizeof(*rule));
@@ -265,7 +513,8 @@ static struct audit_rule_data *audit_kru
void *bufp;
int i;

- data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
+ /* use GFP_ATOMIC because we're under rcu_read_lock() */
+ data = kmalloc(sizeof(*data) + krule->buflen, GFP_ATOMIC);
if (unlikely(!data))
return ERR_PTR(-ENOMEM);
memset(data, 0, sizeof(*data));
@@ -280,7 +529,10 @@ static struct audit_rule_data *audit_kru
data->fields[i] = f->type;
data->fieldflags[i] = f->op;
switch(f->type) {
- /* call type-specific conversion routines here */
+ case AUDIT_WATCH:
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, krule->watch->path);
+ break;
default:
data->values[i] = f->val;
}
@@ -290,6 +542,12 @@ static struct audit_rule_data *audit_kru
return data;
}

+/* Compare two watches. Considered success if rules don't match. */
+static inline int audit_compare_watch(struct audit_watch *a, struct audit_watch *b)
+{
+ return strcmp(a->path, b->path);
+}
+
/* Compare two rules in kernel format. Considered success if rules
* don't match. */
static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b)
@@ -308,7 +566,10 @@ static int audit_compare_rule(struct aud
return 1;

switch(a->fields[i].type) {
- /* call type-specific comparison routines here */
+ case AUDIT_WATCH:
+ if (audit_compare_watch(a->watch, b->watch))
+ return 1;
+ break;
default:
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -322,45 +583,264 @@ static int audit_compare_rule(struct aud
return 0;
}

+/* Copy an audit rule entry to be replaced.
+ * Caller must hold filterlist lock. */
+static inline struct audit_entry *audit_dupe_rule(struct audit_entry *old)
+{
+ u32 fcount = old->rule.field_count;
+ struct audit_entry *new;
+ struct audit_field *fields;
+ struct audit_watch *watch;
+
+ new = audit_init_entry(fcount, GFP_ATOMIC);
+ if (unlikely(!new))
+ return ERR_PTR(-ENOMEM);
+
+ fields = new->rule.fields;
+ memcpy(&new->rule, &old->rule, sizeof(struct audit_krule));
+ memcpy(fields, old->rule.fields, sizeof(struct audit_field) * fcount);
+ new->rule.fields = fields;
+
+ watch = old->rule.watch;
+ audit_get_watch(watch);
+ list_add(&new->rule.rlist, &watch->rules);
+ list_del(&old->rule.rlist);
+
+ return new;
+}
+
+/* Update an audit rule field. If the rule is part of a filterlist, caller
+ * must hold that filterlist's lock. */
+static void audit_update_rule(struct audit_krule *krule, u32 type, u32 val)
+{
+ int i;
+ struct audit_entry *old, *new;
+
+ for (i = 0; i < AUDIT_MAX_FIELDS; i++) {
+ if (krule->fields[i].type != type)
+ continue;
+
+ old = container_of(krule, struct audit_entry, rule);
+
+ /* rule is not in filterlist yet */
+ if (old->flags & AUDIT_ENTRY_ADD) {
+ krule->fields[i].val = val;
+ return;
+ }
+
+ /* make sure we have a valid copy */
+ while (old->replacement != NULL)
+ old = old->replacement;
+ if (old->flags & AUDIT_ENTRY_DEL)
+ return;
+
+ new = audit_dupe_rule(old);
+ if (unlikely(IS_ERR(new))) {
+ audit_panic("cannot allocate memory for rule update");
+ return;
+ }
+ new->rule.fields[i].val = val;
+
+ old->flags |= AUDIT_ENTRY_DEL;
+ old->replacement = new;
+ list_replace_rcu(&old->list, &new->list);
+ call_rcu(&old->rcu, audit_free_rule_rcu);
+ }
+}
+
+/* Find an existing parent entry for this watch, or create a new one.
+ * Caller must hold exit filterlist lock. */
+static inline struct audit_parent *audit_find_parent(char *path)
+{
+ int err;
+ struct nameidata nd;
+ struct audit_parent *p, *parent;
+ unsigned long ino;
+
+ err = path_lookup(path, LOOKUP_PARENT, &nd);
+ if (err) {
+ path_release(&nd);
+ parent = ERR_PTR(err);
+ goto out;
+ }
+
+ /* walk list locked for safe compare of ino field */
+ spin_lock(&master_parents_lock);
+ list_for_each_entry(p, &master_parents, mlist) {
+ if (p->ino != nd.dentry->d_inode->i_ino ||
+ p->flags & AUDIT_PARENT_DEL)
+ continue;
+
+ spin_unlock(&master_parents_lock);
+ path_release(&nd);
+ parent = p;
+ goto out;
+ }
+ ino = nd.dentry->d_inode->i_ino;
+ spin_unlock(&master_parents_lock);
+ path_release(&nd);
+
+ /* Initialize parent with this inode #; the registration thread will
+ * catch any changes. */
+ parent = audit_init_parent(path, ino);
+ if (unlikely(IS_ERR(parent)))
+ goto out;
+
+ 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.
+ * Caller must hold exit filterlist lock. */
+static inline int audit_add_watch(struct audit_krule *krule)
+{
+ struct audit_parent *parent;
+ struct audit_watch *w, *watch = krule->watch;
+ struct nameidata nd;
+
+ parent = audit_find_parent(watch->path);
+ if (IS_ERR(parent))
+ return PTR_ERR(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:
+ list_add(&krule->rlist, &watch->rules);
+
+ if (path_lookup(watch->path, 0, &nd) == 0)
+ audit_update_rule(krule, AUDIT_WATCH,
+ nd.dentry->d_inode->i_ino);
+ path_release(&nd);
+ return 0;
+}
+
/* Add rule to given filterlist if not a duplicate. Protected by
* audit_netlink_sem. */
static inline int audit_add_rule(struct audit_entry *entry,
- struct list_head *list)
+ struct audit_flist *flist)
{
struct audit_entry *e;
+ int err;
+
+ /* The *_rcu iterator is needed to protect from filterlist
+ * updates or removals. */
+ rcu_read_lock();
+ list_for_each_entry_rcu(e, &flist->head, list) {
+ if (e->flags & AUDIT_ENTRY_DEL)
+ continue;
+ if (!audit_compare_rule(&entry->rule, &e->rule)) {
+ err = -EEXIST;
+ rcu_read_unlock();
+ goto error;
+ }
+ }
+ rcu_read_unlock();
+
+ spin_lock(&flist->lock);
+ entry->flags |= AUDIT_ENTRY_ADD;

- /* 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;
+ 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);
+ list_add_rcu(&entry->list, &flist->head);
} else {
- list_add_tail_rcu(&entry->list, list);
+ list_add_tail_rcu(&entry->list, &flist->head);
}

+ entry->flags &= ~AUDIT_ENTRY_ADD;
+ spin_unlock(&flist->lock);
+
return 0;
+
+error:
+ if (entry->rule.watch)
+ audit_put_watch(entry->rule.watch);
+ return err;
+}
+
+/* Remove given krule from its associated watch's rules list and clean up any
+ * last instances of associated watch and parent.
+ * Caller must hold exit filterlist lock */
+static inline void audit_remove_watch(struct audit_krule *krule)
+{
+ struct audit_watch *watch = krule->watch;
+ struct audit_parent *parent = watch->parent;
+ struct task_struct *task;
+
+ list_del(&krule->rlist);
+ if (list_empty(&watch->rules)) {
+ list_del(&watch->wlist);
+ audit_put_watch(watch);
+
+ if (list_empty(&parent->watches)) {
+ /* This flag only read when user adds a watch,
+ * which is prevented by audit_netlink_sem. */
+ parent->flags |= AUDIT_PARENT_DEL;
+
+ /* Spawn a thread to unregister the parent's inotify
+ * watch without the filterlist spinlock. */
+ audit_get_parent(parent);
+ task = kthread_run(audit_inotify_unregister, parent,
+ "audit_inotify_unregister");
+ if (IS_ERR(task))
+ printk(KERN_ERR
+ "%s:%d: unable to remove inotify watch for inode %lu\n",
+ __FILE__, __LINE__, parent->ino);
+ }
+ }
}

/* Remove an existing rule from filterlist. Protected by
* audit_netlink_sem. */
static inline int audit_del_rule(struct audit_entry *entry,
- struct list_head *list)
+ struct audit_flist *flist)
{
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);
- call_rcu(&e->rcu, audit_free_rule_rcu);
- return 0;
+ spin_lock(&flist->lock);
+ list_for_each_entry(e, &flist->head, list) {
+ if (e->flags & AUDIT_ENTRY_DEL ||
+ audit_compare_rule(&entry->rule, &e->rule))
+ continue;
+
+ if (e->rule.watch) {
+ audit_remove_watch(&e->rule);
+ audit_put_watch(entry->rule.watch);
}
+
+ list_del_rcu(&e->list);
+ e->flags |= AUDIT_ENTRY_DEL;
+ call_rcu(&e->rcu, audit_free_rule_rcu);
+ spin_unlock(&flist->lock);
+
+ return ret;
}
+ spin_unlock(&flist->lock);
+ if (entry->rule.watch)
+ audit_put_watch(entry->rule.watch);
return -ENOENT; /* No matching rule */
}

@@ -379,10 +859,12 @@ 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].head,
+ list) {
struct audit_rule *rule;

rule = audit_krule_to_rule(&entry->rule);
@@ -392,6 +874,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);

@@ -413,19 +896,21 @@ 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].head, list) {
struct audit_rule_data *data;

data = audit_krule_to_data(&e->rule);
if (unlikely(!data))
break;
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
- data, sizeof(*data));
+ data, sizeof(*data) + data->buflen);
kfree(data);
}
+ rcu_read_unlock();
}
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);

@@ -516,6 +1001,58 @@ int audit_receive_filter(int type, int p
return err;
}

+/* Update inode numbers in audit rules based on filesystem event. */
+static inline void audit_update_ino(struct audit_parent *parent,
+ const char *dname, u32 ino)
+{
+ struct audit_watch *w;
+ struct audit_krule *r, *next;
+ struct audit_flist *flist = &audit_filter_list[AUDIT_FILTER_EXIT];
+ struct audit_buffer *ab;
+
+ spin_lock(&flist->lock);
+ list_for_each_entry(w, &parent->watches, wlist) {
+ if (audit_compare_dname_path(dname, w->path))
+ continue;
+
+ list_for_each_entry_safe(r, next, &w->rules, rlist)
+ audit_update_rule(r, AUDIT_WATCH, ino);
+
+ ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_CONFIG_CHANGE);
+ audit_log_format(ab, "audit updated rules specifying watch=");
+ audit_log_untrustedstring(ab, w->path);
+ audit_log_format(ab, " with ino=%u\n", ino);
+ audit_log_end(ab);
+ break;
+ }
+ spin_unlock(&flist->lock);
+}
+
+/**
+ * audit_handle_ievent - handler for Inotify events
+ * @event: information about the event
+ * @dname: dentry name associated with event
+ * @inode: inode associated with event
+ * @ptr: kernel's version of a watch descriptor
+ */
+void audit_handle_ievent(struct inotify_event *event, const char *dname,
+ struct inode *inode, void *ptr)
+{
+ struct audit_parent *parent = (struct audit_parent *)ptr;
+
+ if (event->mask & (IN_CREATE|IN_MOVED_TO) && inode)
+ audit_update_ino(parent, dname, (unsigned int)inode->i_ino);
+ else if (event->mask & (IN_DELETE|IN_MOVED_FROM))
+ audit_update_ino(parent, dname, (unsigned int)-1);
+ /* Note: Inotify doesn't remove the watch for the IN_MOVE_SELF event.
+ * Work around this by leaving the parent around with an empty
+ * watchlist. It will be re-used if new watches are added. */
+ else if (event->mask & (AUDIT_IN_SELF))
+ audit_remove_parent_watches(parent);
+ else if (event->mask & IN_IGNORED)
+ audit_remove_parent(parent);
+}
+
int audit_comparator(const u32 left, const u32 op, const u32 right)
{
switch (op) {
@@ -536,7 +1073,39 @@ int audit_comparator(const u32 left, con
}
}

+/* Compare given dentry name with last component in given path,
+ * return of 0 indicates a match. */
+int audit_compare_dname_path(const char *dname, const char *path)
+{
+ int dlen, plen;
+ const char *p;

+ if (!dname || !path)
+ return 1;
+
+ dlen = strlen(dname);
+ plen = strlen(path);
+ if (plen < dlen)
+ return 1;
+
+ /* disregard trailing slashes */
+ p = path + plen - 1;
+ while ((*p == '/') && (p > path))
+ p--;
+
+ /* find last path component */
+ p = p - dlen + 1;
+ if (p < path)
+ return 1;
+ else if (p > path) {
+ if (*--p != '/')
+ return 1;
+ else
+ p++;
+ }
+
+ return strncmp(p, dname, dlen);
+}

static int audit_filter_user_rules(struct netlink_skb_parms *cb,
struct audit_krule *rule,
@@ -581,7 +1150,8 @@ int audit_filter_user(struct netlink_skb
int ret = 1;

rcu_read_lock();
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER].head,
+ list) {
if (audit_filter_user_rules(cb, &e->rule, &state)) {
if (state == AUDIT_DISABLED)
ret = 0;
@@ -599,10 +1169,10 @@ int audit_filter_type(int type)
int result = 0;

rcu_read_lock();
- if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE]))
+ if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE].head))
goto unlock_and_return;

- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE],
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE].head,
list) {
int i;
for (i = 0; i < e->rule.field_count; i++) {
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 8ff51b3..4626c35 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -60,7 +60,7 @@

#include "audit.h"

-extern struct list_head audit_filter_list[];
+extern struct audit_flist audit_filter_list[];

/* No syscall auditing will take place unless audit_enabled != 0. */
extern int audit_enabled;
@@ -241,7 +241,8 @@ static int audit_filter_rules(struct tas
}
break;
case AUDIT_INODE:
- if (ctx) {
+ case AUDIT_WATCH:
+ if (ctx && f->val != (unsigned int)-1) {
for (j = 0; j < ctx->name_count; j++) {
if (audit_comparator(ctx->names[j].ino, f->op, f->val) ||
audit_comparator(ctx->names[j].pino, f->op, f->val)) {
@@ -286,7 +287,8 @@ static enum audit_state audit_filter_tas
enum audit_state state;

rcu_read_lock();
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list) {
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK].head,
+ list) {
if (audit_filter_rules(tsk, &e->rule, NULL, &state)) {
rcu_read_unlock();
return state;
@@ -342,7 +344,7 @@ static inline struct audit_context *audi

if (context->in_syscall && !context->auditable) {
enum audit_state state;
- state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_EXIT]);
+ state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_EXIT].head);
if (state == AUDIT_RECORD_CONTEXT)
context->auditable = 1;
}
@@ -789,7 +791,7 @@ void audit_syscall_entry(struct task_str

state = context->state;
if (state == AUDIT_SETUP_CONTEXT || state == AUDIT_BUILD_CONTEXT)
- state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_ENTRY]);
+ state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_ENTRY].head);
if (likely(state == AUDIT_DISABLED))
return;

@@ -1033,37 +1035,20 @@ void __audit_inode_child(const char *dna
return;

/* determine matching parent */
- if (dname)
- for (idx = 0; idx < context->name_count; idx++)
- if (context->names[idx].pino == pino) {
- const char *n;
- const char *name = context->names[idx].name;
- int dlen = strlen(dname);
- int nlen = name ? strlen(name) : 0;
-
- if (nlen < dlen)
- continue;
-
- /* disregard trailing slashes */
- n = name + nlen - 1;
- while ((*n == '/') && (n > name))
- n--;
-
- /* find last path component */
- n = n - dlen + 1;
- if (n < name)
- continue;
- else if (n > name) {
- if (*--n != '/')
- continue;
- else
- n++;
- }
+ if (!dname)
+ goto no_match;
+ for (idx = 0; idx < context->name_count; idx++)
+ if (context->names[idx].pino == pino) {
+ const char *name = context->names[idx].name;

- if (strncmp(n, dname, dlen) == 0)
- goto update_context;
- }
+ if (!name)
+ continue;
+
+ if (audit_compare_dname_path(dname, name) == 0)
+ goto update_context;
+ }

+no_match:
/* catch-all in case match not found */
idx = context->name_count++;
context->names[idx].name = NULL;
Peter Staubach
2006-02-24 20:42:22 UTC
Permalink
Post by Amy Griffis
diff --git a/kernel/audit.c b/kernel/audit.c
index 4eb97b6..82d926e 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -552,6 +561,14 @@ static void audit_receive(struct sock *s
/* Initialize audit support at boot time. */
static int __init audit_init(void)
{
+ int i;
+
+ /* must be initialized before any audit_log calls */
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ INIT_LIST_HEAD(&audit_filter_list[i].head);
+ spin_lock_init(&audit_filter_list[i].lock);
+ }
+
printk(KERN_INFO "audit: initializing netlink socket (%s)\n",
audit_default ? "enabled" : "disabled");
audit_sock = netlink_kernel_create(NETLINK_AUDIT, 0, audit_receive,
@@ -564,6 +581,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_ievent);
+ if (IS_ERR(audit_idev))
+ audit_panic("cannot initialize inotify device");
+
return 0;
}
__initcall(audit_init);
I don't mean to be too nit-picky, but what happens if the inotify_init()
call fails and audit_failure is anything but AUDIT_FAIL_PANIC? The same
sort of question, but with a little more immediacy, is also applicable
for the call to netlink_kernel_create(). If this routine fails, the
kernel may or may not panic, but if not, proceeds to use a NULL pointer
as if it were a valid pointer.

Perhaps while we're here, we could make the failure path a little more
defensive to keep the system from tipping over at some future point when
it tries use one of these pointers which are not supposed to be NULL,
but are?

Thanx...

ps
Amy Griffis
2006-02-24 21:00:39 UTC
Permalink
Post by Peter Staubach
Post by Amy Griffis
diff --git a/kernel/audit.c b/kernel/audit.c
index 4eb97b6..82d926e 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -552,6 +561,14 @@ static void audit_receive(struct sock *s
/* Initialize audit support at boot time. */
static int __init audit_init(void)
{
+ int i;
+
+ /* must be initialized before any audit_log calls */
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ INIT_LIST_HEAD(&audit_filter_list[i].head);
+ spin_lock_init(&audit_filter_list[i].lock);
+ }
+
printk(KERN_INFO "audit: initializing netlink socket (%s)\n",
audit_default ? "enabled" : "disabled");
audit_sock = netlink_kernel_create(NETLINK_AUDIT, 0,
audit_receive,
@@ -564,6 +581,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_ievent);
+ if (IS_ERR(audit_idev))
+ audit_panic("cannot initialize inotify device");
+
return 0;
}
__initcall(audit_init);
I don't mean to be too nit-picky, but what happens if the
inotify_init()
call fails and audit_failure is anything but AUDIT_FAIL_PANIC? The same
sort of question, but with a little more immediacy, is also applicable
for the call to netlink_kernel_create(). If this routine fails, the
kernel may or may not panic, but if not, proceeds to use a NULL pointer
as if it were a valid pointer.
Perhaps while we're here, we could make the failure path a little more
defensive to keep the system from tipping over at some future point when
it tries use one of these pointers which are not supposed to be NULL,
but are?
Agreed, thanks.

Which reminds me of the need to add the INOTIFY dependency to
AUDITSYSCALL in init/Kconfig.
Timothy R. Chavez
2006-02-24 21:07:30 UTC
Permalink
Post by Amy Griffis
Hello,
This patch provides the functionality which allows a user to specify
audit rules targeting specific filesystem locations. It is an update
of the previously posted patch which provided functionality solely for
<snip>
Post by Amy Griffis
Regards,
Amy
Hi Amy,

Just took a cursory glance and noted a few little things.

-tim

<snip>
Post by Amy Griffis
+/* Inotify events we care about. */
+#define AUDIT_IN_WATCH IN_MOVE|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF
+#define AUDIT_IN_SELF IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT
+
+/* Flags for stale filterlist data */
+#define AUDIT_ENTRY_ADD 0x01 /* Rule entry addition in progress. */
+#define AUDIT_ENTRY_DEL 0x02 /* Rule entry deletion in progress. */
+#define AUDIT_PARENT_DEL 0x01 /* Parent deletion in progress. */
AUDIT_ENTRY_ADD and AUDIT_PARENT_DEL are the same? I'm assuming since
this is a mask, that it probably makes sense to make 0x04, no?

[..]
Post by Amy Griffis
+
+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)) {
+ BUG_ON(!list_empty(&parent->watches));
+ 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)) {
+ BUG_ON(!list_empty(&watch->rules));
+ /* watches that were never added don't have a parent */
+ if (watch->parent)
+ audit_put_parent(watch->parent);
+ kfree(watch->path);
+ kfree(watch);
+ }
+}
static inline void audit_free_rule(struct audit_entry *e)
{
+ /* handle rules that don't have associated watches */
+ if (e->rule.watch)
+ audit_put_watch(e->rule.watch);
kfree(e->rule.fields);
kfree(e);
}
@@ -52,6 +91,190 @@ static inline void audit_free_rule_rcu(s
audit_free_rule(e);
}
+/* Remove all watches & rules associated with a parent that is going away. */
+static inline void audit_remove_parent_watches(struct audit_parent *parent)
+{
+ struct audit_watch *w, *nextw;
+ struct audit_krule *r, *nextr;
+ struct audit_entry *e;
+ struct audit_flist *flist = &audit_filter_list[AUDIT_FILTER_EXIT];
+
+ spin_lock(&flist->lock);
+ list_for_each_entry_safe(w, nextw, &parent->watches, wlist) {
+ list_for_each_entry_safe(r, nextr, &w->rules, rlist) {
+ e = container_of(r, struct audit_entry, rule);
+
+ /* make sure we have a valid copy */
+ while (e->replacement != NULL)
+ e = e->replacement;
+ if (e->flags & AUDIT_ENTRY_DEL)
+ continue;
+
+ list_del(&r->rlist);
+ list_del_rcu(&e->list);
+ e->flags |= AUDIT_ENTRY_DEL;
+ call_rcu(&e->rcu, audit_free_rule_rcu);
+ audit_log(NULL, GFP_ATOMIC, AUDIT_CONFIG_CHANGE,
+ "audit implicitly removed rule from list=%d\n",
+ AUDIT_FILTER_EXIT);
+ }
+ list_del(&w->wlist);
+ audit_put_watch(w);
+ }
+ spin_unlock(&flist->lock);
+}
Why not rcu locking here? You can iterate over the list w/ rcu_read_lock/unlock
and then removals from the list via admin action can be defferred using list_del_rcu?
Doesn't the same apply for additions to the flist as well..

Admittedly, I haven't looked extensively at the code though :/

[..]
Post by Amy Griffis
+
+/* Actually remove the parent; inotify has acknowleged the removal. */
+static inline void audit_remove_parent(struct audit_parent *parent)
+{
+ BUG_ON(!list_empty(&parent->watches));
+ spin_lock(&master_parents_lock);
+ list_del(&parent->mlist);
+ audit_put_parent(parent);
+ spin_unlock(&master_parents_lock);
+}
+
+
+/* Register this parent's inotify watch. */
+static int audit_inotify_register(void *_data)
+{
+ void **data = _data;
+ struct audit_parent *parent;
+ char *path;
+ struct nameidata nd;
+ int err;
+ u32 wd;
+
+ parent = data[0];
+ path = data[1];
+ kfree(data);
+
+ err = path_lookup(path, LOOKUP_PARENT, &nd);
+ if (err)
+ goto handle_error;
+
+ wd = inotify_add_watch(audit_idev, nd.dentry->d_inode, AUDIT_IN_WATCH,
+ parent);
+ if (wd < 0)
+ goto handle_error;
+
+ spin_lock(&master_parents_lock);
+ parent->wd = wd;
+ parent->ino = nd.dentry->d_inode->i_ino;
+ spin_unlock(&master_parents_lock);
+
+ path_release(&nd);
+ audit_put_parent(parent);
+ return 0;
+
+ path_release(&nd);
+ audit_remove_parent_watches(parent);
+ audit_remove_parent(parent);
+
+ audit_put_parent(parent);
+ return 0;
Hm... on error you return 0? That's what you return on success too.

[..]
Post by Amy Griffis
+}
+
+/* Unregister this parent's inotify watch. Generates an IN_IGNORED event. */
+static int audit_inotify_unregister(void *data)
+{
+ struct audit_parent *parent = data;
+ s32 wd;
+
+ spin_lock(&master_parents_lock);
+ wd = parent->wd;
+ spin_unlock(&master_parents_lock);
+
+ if (inotify_ignore(audit_idev, wd))
+ printk(KERN_ERR
+ "%s:%d: unable to remove inotify watch for inode %lu\n",
+ __FILE__, __LINE__, parent->ino);
This is benign?

[..]
Post by Amy Griffis
+ audit_put_parent(parent);
+ return 0;
+}
+
+/* Initialize a parent watch entry. */
+static inline struct audit_parent *audit_init_parent(char *path,
+ unsigned long ino)
+{
+ struct audit_parent *parent;
+ void **data;
+ struct task_struct *task;
+
+ parent = kmalloc(sizeof(*parent), GFP_ATOMIC);
+ if (unlikely(!parent))
+ return ERR_PTR(-ENOMEM);
+
+ memset(parent, 0, sizeof(*parent));
+ INIT_LIST_HEAD(&parent->watches);
+ atomic_set(&parent->count, 0);
+ parent->ino = ino;
+ audit_get_parent(parent);
+
+ /* Spawn a thread to register the parent's inotify watch without
+ * the filterlist spinlock. */
+ data = kmalloc(2 * sizeof(void *), GFP_ATOMIC);
+ if (!data) {
+ audit_put_parent(parent);
+ return ERR_PTR(-ENOMEM);
+ }
+ data[0] = parent;
+ data[1] = path;
+ audit_get_parent(parent);
+ task = kthread_run(audit_inotify_register, data,
+ "audit_inotify_register");
+ if (IS_ERR(task)) {
+ audit_put_parent(parent);
+ return ERR_PTR(PTR_ERR(task));
Redundant? return ERR_PTR(task), right?

[..]
Post by Amy Griffis
+ }
+
+ 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);
+
+ memset(watch, 0, sizeof(*watch));
+ INIT_LIST_HEAD(&watch->rules);
+ atomic_set(&watch->count, 0);
+ watch->path = path;
+ audit_get_watch(watch);
+
Why not just atomic_set(&watch->count, 1), rather than making two atomic calls?

[..]
Post by Amy Griffis
+ return watch;
+}
+
+/* Initialize an audit filterlist entry. */
+static inline struct audit_entry *audit_init_entry(u32 field_count,
+ gfp_t gfp_mask)
+{
+ struct audit_entry *entry;
+ struct audit_field *fields;
+
+ entry = kmalloc(sizeof(*entry), gfp_mask);
+ if (unlikely(!entry))
+ return NULL;
+
+ fields = kmalloc(sizeof(*fields) * field_count, gfp_mask);
+ if (unlikely(!fields)) {
+ kfree(entry);
+ return NULL;
+ }
+
+ memset(entry, 0, sizeof(struct audit_entry));
+ memset(fields, 0, sizeof(struct audit_field) * field_count);
+
+ entry->rule.fields = fields;
+
+ return entry;
+}
+
/* Unpack a filter field's string representation from user-space
* buffer. */
static char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
@@ -79,12 +302,33 @@ static char *audit_unpack_string(void **
return str;
}
+/* Translate a watch string to kernel respresentation. */
+static int audit_to_watch(char *path, struct audit_krule *krule, int fidx)
+{
+ struct audit_field *f = &krule->fields[fidx];
+ struct audit_watch *watch;
+
+ if (path[0] != '/' || path[f->val-1] == '/' ||
+ krule->listnr != AUDIT_FILTER_EXIT ||
+ f->op & ~AUDIT_EQUAL)
+ return -EINVAL;
+
+ watch = audit_init_watch(path);
+ if (unlikely(IS_ERR(watch)))
+ return PTR_ERR(watch);
+
+ audit_get_watch(watch);
+ krule->watch = watch;
+ f->val = (unsigned int)-1;
+
+ return 0;
+}
+
/* Common user-space to kernel rule translation. */
static inline struct audit_entry *audit_to_entry_common(struct audit_rule *rule)
{
unsigned listnr;
struct audit_entry *entry;
- struct audit_field *fields;
int i, err;
err = -EINVAL;
@@ -108,23 +352,14 @@ static inline struct audit_entry *audit_
goto exit_err;
err = -ENOMEM;
- entry = kmalloc(sizeof(*entry), GFP_KERNEL);
- if (unlikely(!entry))
+ entry = audit_init_entry(rule->field_count, GFP_KERNEL);
+ if (!entry)
goto exit_err;
- fields = kmalloc(sizeof(*fields) * rule->field_count, GFP_KERNEL);
- if (unlikely(!fields)) {
- kfree(entry);
- goto exit_err;
- }
-
- memset(&entry->rule, 0, sizeof(struct audit_krule));
- memset(fields, 0, sizeof(struct audit_field));
entry->rule.flags = rule->flags & AUDIT_FILTER_PREPEND;
entry->rule.listnr = listnr;
entry->rule.action = rule->action;
entry->rule.field_count = rule->field_count;
- entry->rule.fields = fields;
for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
entry->rule.mask[i] = rule->mask[i];
@@ -150,15 +385,16 @@ static struct audit_entry *audit_rule_to
for (i = 0; i < rule->field_count; i++) {
struct audit_field *f = &entry->rule.fields[i];
- if (rule->fields[i] & AUDIT_UNUSED_BITS) {
- err = -EINVAL;
- goto exit_free;
- }
-
f->op = rule->fields[i] & (AUDIT_NEGATE|AUDIT_OPERATORS);
f->type = rule->fields[i] & ~(AUDIT_NEGATE|AUDIT_OPERATORS);
f->val = rule->values[i];
+ if (f->type & AUDIT_UNUSED_BITS ||
+ f->type == AUDIT_WATCH) {
+ err = -EINVAL;
+ goto exit_free;
+ }
+
entry->rule.vers_ops = (f->op & AUDIT_OPERATORS) ? 2 : 1;
if (f->op & AUDIT_NEGATE)
f->op |= AUDIT_NOT_EQUAL;
@@ -182,8 +418,9 @@ static struct audit_entry *audit_data_to
int err = 0;
struct audit_entry *entry;
void *bufp;
- /* size_t remain = datasz - sizeof(struct audit_rule_data); */
+ size_t remain = datasz - sizeof(struct audit_rule_data);
int i;
+ char *path;
entry = audit_to_entry_common((struct audit_rule *)data);
if (IS_ERR(entry))
@@ -201,10 +438,20 @@ static struct audit_entry *audit_data_to
f->op = data->fieldflags[i] & AUDIT_OPERATORS;
f->type = data->fields[i];
+ f->val = data->values[i];
switch(f->type) {
- /* call type-specific conversion routines here */
- f->val = data->values[i];
+ path = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(path))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = audit_to_watch(path, &entry->rule, i);
+ if (err) {
+ kfree(path);
+ goto exit_free;
+ }
+ break;
}
}
@@ -234,7 +481,8 @@ static struct audit_rule *audit_krule_to
struct audit_rule *rule;
int i;
- rule = kmalloc(sizeof(*rule), GFP_KERNEL);
+ /* use GFP_ATOMIC because we're under rcu_read_lock() */
+ rule = kmalloc(sizeof(*rule), GFP_ATOMIC);
if (unlikely(!rule))
return ERR_PTR(-ENOMEM);
memset(rule, 0, sizeof(*rule));
@@ -265,7 +513,8 @@ static struct audit_rule_data *audit_kru
void *bufp;
int i;
- data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
+ /* use GFP_ATOMIC because we're under rcu_read_lock() */
+ data = kmalloc(sizeof(*data) + krule->buflen, GFP_ATOMIC);
if (unlikely(!data))
return ERR_PTR(-ENOMEM);
memset(data, 0, sizeof(*data));
@@ -280,7 +529,10 @@ static struct audit_rule_data *audit_kru
data->fields[i] = f->type;
data->fieldflags[i] = f->op;
switch(f->type) {
- /* call type-specific conversion routines here */
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, krule->watch->path);
+ break;
data->values[i] = f->val;
}
@@ -290,6 +542,12 @@ static struct audit_rule_data *audit_kru
return data;
}
+/* Compare two watches. Considered success if rules don't match. */
+static inline int audit_compare_watch(struct audit_watch *a, struct audit_watch *b)
+{
+ return strcmp(a->path, b->path);
+}
+
/* Compare two rules in kernel format. Considered success if rules
* don't match. */
static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b)
@@ -308,7 +566,10 @@ static int audit_compare_rule(struct aud
return 1;
switch(a->fields[i].type) {
- /* call type-specific comparison routines here */
+ if (audit_compare_watch(a->watch, b->watch))
+ return 1;
+ break;
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -322,45 +583,264 @@ static int audit_compare_rule(struct aud
return 0;
}
+/* Copy an audit rule entry to be replaced.
+ * Caller must hold filterlist lock. */
+static inline struct audit_entry *audit_dupe_rule(struct audit_entry *old)
+{
+ u32 fcount = old->rule.field_count;
+ struct audit_entry *new;
+ struct audit_field *fields;
+ struct audit_watch *watch;
+
+ new = audit_init_entry(fcount, GFP_ATOMIC);
+ if (unlikely(!new))
+ return ERR_PTR(-ENOMEM);
+
+ fields = new->rule.fields;
+ memcpy(&new->rule, &old->rule, sizeof(struct audit_krule));
+ memcpy(fields, old->rule.fields, sizeof(struct audit_field) * fcount);
+ new->rule.fields = fields;
+
+ watch = old->rule.watch;
+ audit_get_watch(watch);
+ list_add(&new->rule.rlist, &watch->rules);
+ list_del(&old->rule.rlist);
+
+ return new;
+}
+
+/* Update an audit rule field. If the rule is part of a filterlist, caller
+ * must hold that filterlist's lock. */
+static void audit_update_rule(struct audit_krule *krule, u32 type, u32 val)
+{
+ int i;
+ struct audit_entry *old, *new;
+
+ for (i = 0; i < AUDIT_MAX_FIELDS; i++) {
+ if (krule->fields[i].type != type)
+ continue;
+
+ old = container_of(krule, struct audit_entry, rule);
+
+ /* rule is not in filterlist yet */
+ if (old->flags & AUDIT_ENTRY_ADD) {
+ krule->fields[i].val = val;
+ return;
+ }
+
+ /* make sure we have a valid copy */
+ while (old->replacement != NULL)
+ old = old->replacement;
+ if (old->flags & AUDIT_ENTRY_DEL)
+ return;
+
+ new = audit_dupe_rule(old);
+ if (unlikely(IS_ERR(new))) {
+ audit_panic("cannot allocate memory for rule update");
+ return;
+ }
+ new->rule.fields[i].val = val;
+
+ old->flags |= AUDIT_ENTRY_DEL;
+ old->replacement = new;
+ list_replace_rcu(&old->list, &new->list);
+ call_rcu(&old->rcu, audit_free_rule_rcu);
+ }
+}
+
+/* Find an existing parent entry for this watch, or create a new one.
+ * Caller must hold exit filterlist lock. */
+static inline struct audit_parent *audit_find_parent(char *path)
+{
+ int err;
+ struct nameidata nd;
+ struct audit_parent *p, *parent;
+ unsigned long ino;
+
+ err = path_lookup(path, LOOKUP_PARENT, &nd);
+ if (err) {
+ path_release(&nd);
+ parent = ERR_PTR(err);
+ goto out;
+ }
+
+ /* walk list locked for safe compare of ino field */
+ spin_lock(&master_parents_lock);
+ list_for_each_entry(p, &master_parents, mlist) {
+ if (p->ino != nd.dentry->d_inode->i_ino ||
+ p->flags & AUDIT_PARENT_DEL)
+ continue;
+
+ spin_unlock(&master_parents_lock);
+ path_release(&nd);
+ parent = p;
+ goto out;
+ }
+ ino = nd.dentry->d_inode->i_ino;
+ spin_unlock(&master_parents_lock);
+ path_release(&nd);
+
+ /* Initialize parent with this inode #; the registration thread will
+ * catch any changes. */
+ parent = audit_init_parent(path, ino);
+ if (unlikely(IS_ERR(parent)))
+ goto out;
+
+ spin_lock(&master_parents_lock);
+ list_add(&parent->mlist, &master_parents);
+ spin_unlock(&master_parents_lock);
+
+ return parent;
+}
+
+/* Find a matching watch entry, or add this one.
+ * Caller must hold exit filterlist lock. */
+static inline int audit_add_watch(struct audit_krule *krule)
+{
+ struct audit_parent *parent;
+ struct audit_watch *w, *watch = krule->watch;
+ struct nameidata nd;
+
+ parent = audit_find_parent(watch->path);
+ if (IS_ERR(parent))
+ return PTR_ERR(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);
+
+ list_add(&krule->rlist, &watch->rules);
+
+ if (path_lookup(watch->path, 0, &nd) == 0)
+ audit_update_rule(krule, AUDIT_WATCH,
+ nd.dentry->d_inode->i_ino);
+ path_release(&nd);
+ return 0;
+}
+
/* Add rule to given filterlist if not a duplicate. Protected by
* audit_netlink_sem. */
static inline int audit_add_rule(struct audit_entry *entry,
- struct list_head *list)
+ struct audit_flist *flist)
{
struct audit_entry *e;
+ int err;
+
+ /* The *_rcu iterator is needed to protect from filterlist
+ * updates or removals. */
+ rcu_read_lock();
+ list_for_each_entry_rcu(e, &flist->head, list) {
+ if (e->flags & AUDIT_ENTRY_DEL)
+ continue;
+ if (!audit_compare_rule(&entry->rule, &e->rule)) {
+ err = -EEXIST;
+ rcu_read_unlock();
+ goto error;
+ }
+ }
+ rcu_read_unlock();
+
+ spin_lock(&flist->lock);
+ entry->flags |= AUDIT_ENTRY_ADD;
- /* 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;
+ 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);
+ list_add_rcu(&entry->list, &flist->head);
} else {
- list_add_tail_rcu(&entry->list, list);
+ list_add_tail_rcu(&entry->list, &flist->head);
}
+ entry->flags &= ~AUDIT_ENTRY_ADD;
+ spin_unlock(&flist->lock);
+
I guess I should look over this more closely... not getting the use of flist->lock.

[..]
Post by Amy Griffis
return 0;
+
+ if (entry->rule.watch)
+ audit_put_watch(entry->rule.watch);
+ return err;
+}
+
+/* Remove given krule from its associated watch's rules list and clean up any
+ * last instances of associated watch and parent.
+ * Caller must hold exit filterlist lock */
+static inline void audit_remove_watch(struct audit_krule *krule)
+{
+ struct audit_watch *watch = krule->watch;
+ struct audit_parent *parent = watch->parent;
+ struct task_struct *task;
+
+ list_del(&krule->rlist);
+ if (list_empty(&watch->rules)) {
+ list_del(&watch->wlist);
+ audit_put_watch(watch);
+
+ if (list_empty(&parent->watches)) {
+ /* This flag only read when user adds a watch,
+ * which is prevented by audit_netlink_sem. */
+ parent->flags |= AUDIT_PARENT_DEL;
+
+ /* Spawn a thread to unregister the parent's inotify
+ * watch without the filterlist spinlock. */
+ audit_get_parent(parent);
+ task = kthread_run(audit_inotify_unregister, parent,
+ "audit_inotify_unregister");
+ if (IS_ERR(task))
+ printk(KERN_ERR
+ "%s:%d: unable to remove inotify watch for inode %lu\n",
+ __FILE__, __LINE__, parent->ino);
+ }
+ }
}
/* Remove an existing rule from filterlist. Protected by
* audit_netlink_sem. */
static inline int audit_del_rule(struct audit_entry *entry,
- struct list_head *list)
+ struct audit_flist *flist)
{
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);
- call_rcu(&e->rcu, audit_free_rule_rcu);
- return 0;
+ spin_lock(&flist->lock);
+ list_for_each_entry(e, &flist->head, list) {
+ if (e->flags & AUDIT_ENTRY_DEL ||
+ audit_compare_rule(&entry->rule, &e->rule))
+ continue;
+
+ if (e->rule.watch) {
+ audit_remove_watch(&e->rule);
+ audit_put_watch(entry->rule.watch);
}
+
+ list_del_rcu(&e->list);
+ e->flags |= AUDIT_ENTRY_DEL;
+ call_rcu(&e->rcu, audit_free_rule_rcu);
+ spin_unlock(&flist->lock);
+
+ return ret;
}
+ spin_unlock(&flist->lock);
+ if (entry->rule.watch)
+ audit_put_watch(entry->rule.watch);
return -ENOENT; /* No matching rule */
}
@@ -379,10 +859,12 @@ 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].head,
+ list) {
struct audit_rule *rule;
rule = audit_krule_to_rule(&entry->rule);
@@ -392,6 +874,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);
@@ -413,19 +896,21 @@ 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].head, list) {
struct audit_rule_data *data;
data = audit_krule_to_data(&e->rule);
if (unlikely(!data))
break;
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
- data, sizeof(*data));
+ data, sizeof(*data) + data->buflen);
kfree(data);
}
+ rcu_read_unlock();
}
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
@@ -516,6 +1001,58 @@ int audit_receive_filter(int type, int p
return err;
}
+/* Update inode numbers in audit rules based on filesystem event. */
+static inline void audit_update_ino(struct audit_parent *parent,
+ const char *dname, u32 ino)
+{
+ struct audit_watch *w;
+ struct audit_krule *r, *next;
+ struct audit_flist *flist = &audit_filter_list[AUDIT_FILTER_EXIT];
+ struct audit_buffer *ab;
+
+ spin_lock(&flist->lock);
+ list_for_each_entry(w, &parent->watches, wlist) {
+ if (audit_compare_dname_path(dname, w->path))
+ continue;
+
+ list_for_each_entry_safe(r, next, &w->rules, rlist)
+ audit_update_rule(r, AUDIT_WATCH, ino);
+
+ ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_CONFIG_CHANGE);
+ audit_log_format(ab, "audit updated rules specifying watch=");
+ audit_log_untrustedstring(ab, w->path);
+ audit_log_format(ab, " with ino=%u\n", ino);
+ audit_log_end(ab);
+ break;
+ }
+ spin_unlock(&flist->lock);
+}
+
+/**
+ * audit_handle_ievent - handler for Inotify events
+ */
+void audit_handle_ievent(struct inotify_event *event, const char *dname,
+ struct inode *inode, void *ptr)
+{
+ struct audit_parent *parent = (struct audit_parent *)ptr;
+
+ if (event->mask & (IN_CREATE|IN_MOVED_TO) && inode)
+ audit_update_ino(parent, dname, (unsigned int)inode->i_ino);
+ else if (event->mask & (IN_DELETE|IN_MOVED_FROM))
+ audit_update_ino(parent, dname, (unsigned int)-1);
+ /* Note: Inotify doesn't remove the watch for the IN_MOVE_SELF event.
+ * Work around this by leaving the parent around with an empty
+ * watchlist. It will be re-used if new watches are added. */
+ else if (event->mask & (AUDIT_IN_SELF))
+ audit_remove_parent_watches(parent);
+ else if (event->mask & IN_IGNORED)
+ audit_remove_parent(parent);
+}
+
int audit_comparator(const u32 left, const u32 op, const u32 right)
{
switch (op) {
@@ -536,7 +1073,39 @@ int audit_comparator(const u32 left, con
}
}
+/* Compare given dentry name with last component in given path,
+ * return of 0 indicates a match. */
+int audit_compare_dname_path(const char *dname, const char *path)
+{
+ int dlen, plen;
+ const char *p;
+ if (!dname || !path)
+ return 1;
+
+ dlen = strlen(dname);
+ plen = strlen(path);
+ if (plen < dlen)
+ return 1;
+
+ /* disregard trailing slashes */
+ p = path + plen - 1;
+ while ((*p == '/') && (p > path))
+ p--;
+
+ /* find last path component */
Why not path_lookup with LOOKUP_PARENT, then d_lookup?
Post by Amy Griffis
+ p = p - dlen + 1;
+ if (p < path)
+ return 1;
+ else if (p > path) {
+ if (*--p != '/')
+ return 1;
+ else
+ p++;
+ }
+
+ return strncmp(p, dname, dlen);
+}
static int audit_filter_user_rules(struct netlink_skb_parms *cb,
struct audit_krule *rule,
@@ -581,7 +1150,8 @@ int audit_filter_user(struct netlink_skb
int ret = 1;
rcu_read_lock();
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER].head,
+ list) {
if (audit_filter_user_rules(cb, &e->rule, &state)) {
if (state == AUDIT_DISABLED)
ret = 0;
@@ -599,10 +1169,10 @@ int audit_filter_type(int type)
int result = 0;
rcu_read_lock();
- if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE]))
+ if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE].head))
goto unlock_and_return;
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE],
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE].head,
list) {
int i;
for (i = 0; i < e->rule.field_count; i++) {
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 8ff51b3..4626c35 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -60,7 +60,7 @@
#include "audit.h"
-extern struct list_head audit_filter_list[];
+extern struct audit_flist audit_filter_list[];
/* No syscall auditing will take place unless audit_enabled != 0. */
extern int audit_enabled;
@@ -241,7 +241,8 @@ static int audit_filter_rules(struct tas
}
break;
- if (ctx) {
+ if (ctx && f->val != (unsigned int)-1) {
for (j = 0; j < ctx->name_count; j++) {
if (audit_comparator(ctx->names[j].ino, f->op, f->val) ||
audit_comparator(ctx->names[j].pino, f->op, f->val)) {
@@ -286,7 +287,8 @@ static enum audit_state audit_filter_tas
enum audit_state state;
rcu_read_lock();
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list) {
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK].head,
+ list) {
if (audit_filter_rules(tsk, &e->rule, NULL, &state)) {
rcu_read_unlock();
return state;
@@ -342,7 +344,7 @@ static inline struct audit_context *audi
if (context->in_syscall && !context->auditable) {
enum audit_state state;
- state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_EXIT]);
+ state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_EXIT].head);
if (state == AUDIT_RECORD_CONTEXT)
context->auditable = 1;
}
@@ -789,7 +791,7 @@ void audit_syscall_entry(struct task_str
state = context->state;
if (state == AUDIT_SETUP_CONTEXT || state == AUDIT_BUILD_CONTEXT)
- state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_ENTRY]);
+ state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_ENTRY].head);
if (likely(state == AUDIT_DISABLED))
return;
@@ -1033,37 +1035,20 @@ void __audit_inode_child(const char *dna
return;
/* determine matching parent */
- if (dname)
- for (idx = 0; idx < context->name_count; idx++)
- if (context->names[idx].pino == pino) {
- const char *n;
- const char *name = context->names[idx].name;
- int dlen = strlen(dname);
- int nlen = name ? strlen(name) : 0;
-
- if (nlen < dlen)
- continue;
-
- /* disregard trailing slashes */
- n = name + nlen - 1;
- while ((*n == '/') && (n > name))
- n--;
-
- /* find last path component */
- n = n - dlen + 1;
- if (n < name)
- continue;
- else if (n > name) {
- if (*--n != '/')
- continue;
- else
- n++;
- }
+ if (!dname)
+ goto no_match;
+ for (idx = 0; idx < context->name_count; idx++)
+ if (context->names[idx].pino == pino) {
+ const char *name = context->names[idx].name;
- if (strncmp(n, dname, dlen) == 0)
- goto update_context;
- }
+ if (!name)
+ continue;
+
+ if (audit_compare_dname_path(dname, name) == 0)
+ goto update_context;
+ }
/* catch-all in case match not found */
idx = context->name_count++;
context->names[idx].name = NULL;
Timothy R. Chavez
2006-02-24 21:20:35 UTC
Permalink
Post by Timothy R. Chavez
Post by Amy Griffis
+ if (IS_ERR(task)) {
+ audit_put_parent(parent);
+ return ERR_PTR(PTR_ERR(task));
Redundant? return ERR_PTR(task), right?
[..]
er return task;, my bad.

-tim
Amy Griffis
2006-03-09 23:05:18 UTC
Permalink
Post by Timothy R. Chavez
Post by Amy Griffis
+/* Inotify events we care about. */
+#define AUDIT_IN_WATCH IN_MOVE|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF
+#define AUDIT_IN_SELF IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT
+
+/* Flags for stale filterlist data */
+#define AUDIT_ENTRY_ADD 0x01 /* Rule entry addition in progress. */
+#define AUDIT_ENTRY_DEL 0x02 /* Rule entry deletion in progress. */
+#define AUDIT_PARENT_DEL 0x01 /* Parent deletion in progress. */
AUDIT_ENTRY_ADD and AUDIT_PARENT_DEL are the same? I'm assuming since
this is a mask, that it probably makes sense to make 0x04, no?
AUDIT_ENTRY_* and AUDIT_PARENT_* were two different masks in the
audit_entry and audit_parent structs. I've re-written the code
though, so neither of them is needed now.
Post by Timothy R. Chavez
Post by Amy Griffis
+/* Remove all watches & rules associated with a parent that is going away. */
+static inline void audit_remove_parent_watches(struct audit_parent *parent)
+{
+ struct audit_watch *w, *nextw;
+ struct audit_krule *r, *nextr;
+ struct audit_entry *e;
+ struct audit_flist *flist = &audit_filter_list[AUDIT_FILTER_EXIT];
+
+ spin_lock(&flist->lock);
+ list_for_each_entry_safe(w, nextw, &parent->watches, wlist) {
+ list_for_each_entry_safe(r, nextr, &w->rules, rlist) {
+ e = container_of(r, struct audit_entry, rule);
+
+ /* make sure we have a valid copy */
+ while (e->replacement != NULL)
+ e = e->replacement;
+ if (e->flags & AUDIT_ENTRY_DEL)
+ continue;
+
+ list_del(&r->rlist);
+ list_del_rcu(&e->list);
+ e->flags |= AUDIT_ENTRY_DEL;
+ call_rcu(&e->rcu, audit_free_rule_rcu);
+ audit_log(NULL, GFP_ATOMIC, AUDIT_CONFIG_CHANGE,
+ "audit implicitly removed rule from list=%d\n",
+ AUDIT_FILTER_EXIT);
+ }
+ list_del(&w->wlist);
+ audit_put_watch(w);
+ }
+ spin_unlock(&flist->lock);
+}
Why not rcu locking here? You can iterate over the list w/
rcu_read_lock/unlock and then removals from the list via admin
action can be defferred using list_del_rcu? Doesn't the same apply
for additions to the flist as well..
I am using rcu locking, but you need an additional lock to protect
against concurrent list writes.
Post by Timothy R. Chavez
Post by Amy Griffis
+/* Register this parent's inotify watch. */
+static int audit_inotify_register(void *_data)
+{
+ void **data = _data;
+ struct audit_parent *parent;
+ char *path;
+ struct nameidata nd;
+ int err;
+ u32 wd;
+
+ parent = data[0];
+ path = data[1];
+ kfree(data);
+
+ err = path_lookup(path, LOOKUP_PARENT, &nd);
+ if (err)
+ goto handle_error;
+
+ wd = inotify_add_watch(audit_idev, nd.dentry->d_inode, AUDIT_IN_WATCH,
+ parent);
+ if (wd < 0)
+ goto handle_error;
+
+ spin_lock(&master_parents_lock);
+ parent->wd = wd;
+ parent->ino = nd.dentry->d_inode->i_ino;
+ spin_unlock(&master_parents_lock);
+
+ path_release(&nd);
+ audit_put_parent(parent);
+ return 0;
+
+ path_release(&nd);
+ audit_remove_parent_watches(parent);
+ audit_remove_parent(parent);
+
+ audit_put_parent(parent);
+ return 0;
Hm... on error you return 0? That's what you return on success too.
Yeah, the function was supposed to contain the error by cleaning up
after itself. It's a little different in the latest iteration...
Post by Timothy R. Chavez
Post by Amy Griffis
+/* Unregister this parent's inotify watch. Generates an IN_IGNORED event. */
+static int audit_inotify_unregister(void *data)
+{
+ struct audit_parent *parent = data;
+ s32 wd;
+
+ spin_lock(&master_parents_lock);
+ wd = parent->wd;
+ spin_unlock(&master_parents_lock);
+
+ if (inotify_ignore(audit_idev, wd))
+ printk(KERN_ERR
+ "%s:%d: unable to remove inotify watch for inode %lu\n",
+ __FILE__, __LINE__, parent->ino);
This is benign?
Not really benign, but a way to signal an un-recoverable error. This
situation has been removed by not using kthread_run.
Post by Timothy R. Chavez
Post by Amy Griffis
+/* Initialize a parent watch entry. */
+static inline struct audit_parent *audit_init_parent(char *path,
+ unsigned long ino)
+{
+ struct audit_parent *parent;
+ void **data;
+ struct task_struct *task;
+
+ parent = kmalloc(sizeof(*parent), GFP_ATOMIC);
+ if (unlikely(!parent))
+ return ERR_PTR(-ENOMEM);
+
+ memset(parent, 0, sizeof(*parent));
+ INIT_LIST_HEAD(&parent->watches);
+ atomic_set(&parent->count, 0);
+ parent->ino = ino;
+ audit_get_parent(parent);
+
+ /* Spawn a thread to register the parent's inotify watch without
+ * the filterlist spinlock. */
+ data = kmalloc(2 * sizeof(void *), GFP_ATOMIC);
+ if (!data) {
+ audit_put_parent(parent);
+ return ERR_PTR(-ENOMEM);
+ }
+ data[0] = parent;
+ data[1] = path;
+ audit_get_parent(parent);
+ task = kthread_run(audit_inotify_register, data,
+ "audit_inotify_register");
+ if (IS_ERR(task)) {
+ audit_put_parent(parent);
+ return ERR_PTR(PTR_ERR(task));
Redundant? return ERR_PTR(task), right?
The function returns an audit_parent struct, and this is a task
struct. Anyway, it's been removed as a result of other changes. :-)
Post by Timothy R. Chavez
Post by Amy Griffis
+/* 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);
+
+ memset(watch, 0, sizeof(*watch));
+ INIT_LIST_HEAD(&watch->rules);
+ atomic_set(&watch->count, 0);
+ watch->path = path;
+ audit_get_watch(watch);
+
Why not just atomic_set(&watch->count, 1), rather than making two atomic calls?
Done.
Post by Timothy R. Chavez
Post by Amy Griffis
+/* Compare given dentry name with last component in given path,
+ * return of 0 indicates a match. */
+int audit_compare_dname_path(const char *dname, const char *path)
+{
+ int dlen, plen;
+ const char *p;
+ if (!dname || !path)
+ return 1;
+
+ dlen = strlen(dname);
+ plen = strlen(path);
+ if (plen < dlen)
+ return 1;
+
+ /* disregard trailing slashes */
+ p = path + plen - 1;
+ while ((*p == '/') && (p > path))
+ p--;
+
+ /* find last path component */
Why not path_lookup with LOOKUP_PARENT, then d_lookup?
I was hoping to avoid doing path_lookup here.
Post by Timothy R. Chavez
Post by Amy Griffis
+ p = p - dlen + 1;
+ if (p < path)
+ return 1;
+ else if (p > path) {
+ if (*--p != '/')
+ return 1;
+ else
+ p++;
+ }
+
+ return strncmp(p, dname, dlen);
+}
Alexander Viro
2006-02-24 21:31:06 UTC
Permalink
* path_lookup() blocks. Don't do it under spinlocks; possible
approach is to do it early and pass pointer to nameidata down into
critical area.
* path_release() blocks. Dealing with path_lookup() will deal
with that - you simply do it after leaving the critical area.
* failing path_lookup() => no path_release() is needed in cleanup.
* spawning a thread blocks; FWIW, I'd try passing a list to
audit_remove_watch() and instead of kthread_... put on that list for
post-processing.
* inode number alone is not enough to compare fs objects;
several filesystems easily can have inodes with the same inode numbers.
That, BTW, gives a useful test - create a filesystem, copy it to device
of the same size with dd(1), then mount both. And try to mix watching
the corresponding places on both filesystems.
Steve Grubb
2006-02-27 14:24:25 UTC
Permalink
Post by Amy Griffis
TIA for reviewing this patch.
I wasn't able to put this into the lspp.10 kernel cause there was too many
patching failures. For example,
Post by Amy Griffis
diff --git a/include/linux/audit.h b/include/linux/audit.h
index c208554..d76fa58 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -148,6 +148,7 @@
#define AUDIT_INODE 102
#define AUDIT_EXIT 103
#define AUDIT_SUCCESS 104 /* exit >= 0; value ignored */
+#define AUDIT_WATCH 105
#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
The above code was in your patch from:

https://www.redhat.com/archives/linux-audit/2006-February/msg00034.html


I deleted the above patch and applied with this as a result:

patching file kernel/audit.c
patching file kernel/audit.h
Hunk #2 FAILED at 54.
Hunk #3 FAILED at 86.
2 out of 3 hunks FAILED -- saving rejects to file kernel/audit.h.rej
patching file kernel/auditfilter.c
Hunk #1 FAILED at 22.
Hunk #2 succeeded at 101 with fuzz 2 (offset 10 lines).
Hunk #3 succeeded at 329 with fuzz 1 (offset 27 lines).
Hunk #4 succeeded at 362 (offset 10 lines).
Hunk #5 FAILED at 395.
Hunk #6 FAILED at 428.
Hunk #7 FAILED at 448.
Hunk #8 succeeded at 526 (offset 45 lines).
Hunk #9 succeeded at 523 (offset 10 lines).
Hunk #10 FAILED at 539.
Hunk #11 succeeded at 596 with fuzz 1 (offset 54 lines).
Hunk #12 FAILED at 620.
Hunk #13 FAILED at 637.
Hunk #14 succeeded at 910 (offset 51 lines).
Hunk #15 succeeded at 928 (offset 54 lines).
Hunk #16 FAILED at 950.
Hunk #17 succeeded at 1059 (offset 58 lines).
Hunk #18 succeeded at 1125 (offset 52 lines).
Hunk #19 succeeded at 1208 (offset 58 lines).
Hunk #20 succeeded at 1221 (offset 52 lines).
8 out of 20 hunks FAILED -- saving rejects to file kernel/auditfilter.c.rej
patching file kernel/auditsc.c
Hunk #2 FAILED at 241.
Hunk #3 succeeded at 288 (offset 1 line).
Hunk #5 succeeded at 796 (offset 5 lines).
1 out of 6 hunks FAILED -- saving rejects to file kernel/auditsc.c.rej

I think we are out of sync somewhere.

-Steve
Amy Griffis
2006-02-27 16:30:55 UTC
Permalink
Post by Steve Grubb
Post by Amy Griffis
TIA for reviewing this patch.
I wasn't able to put this into the lspp.10 kernel cause there was too many
patching failures. For example,
Post by Amy Griffis
diff --git a/include/linux/audit.h b/include/linux/audit.h
index c208554..d76fa58 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -148,6 +148,7 @@
#define AUDIT_INODE 102
#define AUDIT_EXIT 103
#define AUDIT_SUCCESS 104 /* exit >= 0; value ignored */
+#define AUDIT_WATCH 105
#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
https://www.redhat.com/archives/linux-audit/2006-February/msg00034.html
patching file kernel/audit.c
patching file kernel/audit.h
Hunk #2 FAILED at 54.
Hunk #3 FAILED at 86.
2 out of 3 hunks FAILED -- saving rejects to file kernel/audit.h.rej
patching file kernel/auditfilter.c
Hunk #1 FAILED at 22.
Hunk #2 succeeded at 101 with fuzz 2 (offset 10 lines).
Hunk #3 succeeded at 329 with fuzz 1 (offset 27 lines).
Hunk #4 succeeded at 362 (offset 10 lines).
Hunk #5 FAILED at 395.
Hunk #6 FAILED at 428.
Hunk #7 FAILED at 448.
Hunk #8 succeeded at 526 (offset 45 lines).
Hunk #9 succeeded at 523 (offset 10 lines).
Hunk #10 FAILED at 539.
Hunk #11 succeeded at 596 with fuzz 1 (offset 54 lines).
Hunk #12 FAILED at 620.
Hunk #13 FAILED at 637.
Hunk #14 succeeded at 910 (offset 51 lines).
Hunk #15 succeeded at 928 (offset 54 lines).
Hunk #16 FAILED at 950.
Hunk #17 succeeded at 1059 (offset 58 lines).
Hunk #18 succeeded at 1125 (offset 52 lines).
Hunk #19 succeeded at 1208 (offset 58 lines).
Hunk #20 succeeded at 1221 (offset 52 lines).
8 out of 20 hunks FAILED -- saving rejects to file kernel/auditfilter.c.rej
patching file kernel/auditsc.c
Hunk #2 FAILED at 241.
Hunk #3 succeeded at 288 (offset 1 line).
Hunk #5 succeeded at 796 (offset 5 lines).
1 out of 6 hunks FAILED -- saving rejects to file kernel/auditsc.c.rej
I think we are out of sync somewhere.
I based this patch off of the git tree, and I suspect the order in
which you are applying the patches does not follow the git tree.

Looking at the spec file for the lspp.9 kernel, I see:

Patch20010: linux-2.6-audit-git.patch
Patch20011: linux-2.6-audit-promisc.patch
Patch20012: linux-2.6-audit-tty.patch
Patch20013: linux-2.6-vm86-audit_syscall_exit.patch
Patch20014: linux-2.6-audit-string-1.patch
Patch20015: linux-2.6-audit-string-2.patch
Patch20016: linux-2.6-audit-inotify-api.patch
Patch20017: linux-2.6-audit-rule-log.patch
Patch20018: linux-2.6-audit_log_exit-gfp_mask.patch
Patch20019: linux-2.6-audit-fix-operators.patch

In the git tree master.b1 branch, I see a different ordering:

Patch20013: linux-2.6-vm86-audit_syscall_exit.patch
Patch20018: linux-2.6-audit_log_exit-gfp_mask.patch
Patch20014: linux-2.6-audit-string-1.patch
Patch20015: linux-2.6-audit-string-2.patch
Patch20016: linux-2.6-audit-inotify-api.patch
Patch20017: linux-2.6-audit-rule-log.patch
Patch20012: linux-2.6-audit-tty.patch

And these patches aren't included:

Patch20011: linux-2.6-audit-promisc.patch
Patch20019: linux-2.6-audit-fix-operators.patch

The last patch I posted was supposed to replace audit-string-2.patch
(as audit-watch.patch or something similar). I think if you used the
git tree ordering, you wouldn't see as many patch collisions.

Regards,
Amy
Steve Grubb
2006-02-27 16:42:23 UTC
Permalink
Post by Amy Griffis
The last patch I posted was supposed to replace audit-string-2.patch
I must have missed that in the text somewhere...
Post by Amy Griffis
(as audit-watch.patch or something similar).  I think if you used the
git tree ordering, you wouldn't see as many patch collisions.
At this point, your patch is the last one in, so shouldn't it should be
against what's already in the tree? Its really a lot of trouble to take a
change in a patch that's in the middle of the stack.

-Steve
Amy Griffis
2006-02-27 17:06:05 UTC
Permalink
Post by Steve Grubb
Post by Amy Griffis
The last patch I posted was supposed to replace audit-string-2.patch
I must have missed that in the text somewhere...
Post by Amy Griffis
(as audit-watch.patch or something similar). ?I think if you used the
git tree ordering, you wouldn't see as many patch collisions.
At this point, your patch is the last one in, so shouldn't it should be
against what's already in the tree? Its really a lot of trouble to
take a change in a patch that's in the middle of the stack.
The patch I was updating was already present in both the audit.b1 and
master.b1 branches in the git tree. I didn't see other options for
basing the patch, other than to branch off of the last commit prior to
the commit for this patch.

What is the preferred way to update a patch which has already been
merged?

Thanks,
Amy
Steve Grubb
2006-02-27 17:24:35 UTC
Permalink
Post by Amy Griffis
What is the preferred way to update a patch which has already been
merged?
In the past, we've applied patches on top of patches when they've been out for
some time. If its recent, like within a week, we've corrected patches since
there's not much that is counting on it being a certain way. An example,
we've got 2 patches against Dustin's patch that adds contexts to syscall
records.

Under normal circumstances this normally isn't a problem. Right now we have a
5 month backlog of patches that haven't gone upstream. That represents about
17-20 individual patches. Each patch added makes it more and more fragile to
changes that upstream is making.

The functionality that you just added, the audit client, really does stand on
its own. I don't think you need to merge it with the string2 patch.

-Steve
Amy Griffis
2006-02-27 18:44:14 UTC
Permalink
Post by Steve Grubb
Post by Amy Griffis
What is the preferred way to update a patch which has already been
merged?
In the past, we've applied patches on top of patches when they've
been out for some time. If its recent, like within a week, we've
corrected patches since there's not much that is counting on it
being a certain way. An example, we've got 2 patches against
Dustin's patch that adds contexts to syscall records.
I understand why you want to do things this way. But I don't think it
makes it very easy for someone upstream to review our list of patches.

I thought that the branches in the git tree could handle this. Would
it be possible to have a new branch that moves the inotify kernel api
patch and the audit watch patch to the end of the list?

If this isn't possible, then I'm afraid I don't understand the point
of the 'amg' branch.
Post by Steve Grubb
Under normal circumstances this normally isn't a problem. Right now
we have a 5 month backlog of patches that haven't gone upstream.
That represents about 17-20 individual patches. Each patch added
makes it more and more fragile to changes that upstream is making.
The functionality that you just added, the audit client, really does
stand on its own. I don't think you need to merge it with the
string2 patch.
The patch I just posted includes all of the AUDIT_WATCH functionality.
The previous patch (aka string2) only included functionality for
add/remove/listing rules. The purpose of separating this piece out
was to have something stable that would enable work on the interface
changes from the userspace side.

If the AUDIT_WATCH functionality is to be reviewed by anyone further
upstream, as I expect it to be, it should be reviewed as a single
patch. The first patch adds a chunk of code that is then removed by
the second patch. It's pointless for anyone upstream to review that
intermediate work.

This is also going to be a problem with the inotify kernel API patch,
as it needs a re-write before it can be proposed upstream.

Regards,
Amy
Amy Griffis
2006-03-09 23:08:19 UTC
Permalink
Here is another iteration of the filesystem location based auditing patch,
previous version was posted here:

https://www.redhat.com/archives/linux-audit/2006-February/msg00152.html

The patch is based off of the current audit git tree plus the latest versions
of the context based audit filtering work, posted here:

https://www.redhat.com/archives/linux-audit/2006-February/msg00160.html
https://www.redhat.com/archives/linux-audit/2006-March/msg00071.html

I believe I've addressed all of the feedback I received last round. Here is a
laundry list:

- protect against uninitialized audit_idev (similar fix for audit_sock in
separate patch)
- add INOTIFY dependency to AUDITSYSCALL in Kconfig
- use atomic_set 1 for struct initialization
- move path_lookup and path_release out from under spinlock
- don't call path_release on path_lookup failure
- post-process inotify registration/de-registration instead of using
kthread_run
- remove no longer needed AUDIT_PARENT_DEL and AUDIT_ENTRY_ADD flags
- use a combination of superblock dev and inode # to distinguish watches and
parents; this results in replacing the watch in addition to the watch's rules
in audit_update_watch()
- fix memory corruption due to list_add while traversing same list
- incorporate selinux field copy into audit_dupe_rule()
- modify selinux_audit_rule_update() to use audit_dupe_rule() and do
necessary watch.rules list manipulation
- modify selinux_audit_rule_update() to use filterlist locks instead of
audit_netlink_mutex
- remove the replacement pointer from audit_entry; it doesn't seem to be needed
since we always search for list elements under lock

I'm a bit uncertain about the per-filterlist spinlocks. It may suffice to just use the audit_netlink_mutex as Darrel has been doing. Any advice?

If ok, please include this patch in the next version of the lspp test kernel,
along with the latest inotify kernel API patch. Please don't include either of these patches in -mm yet.

Thanks,
Amy

---

include/linux/audit.h | 1
init/Kconfig | 2
kernel/audit.c | 24 +
kernel/audit.h | 37 +-
kernel/auditfilter.c | 886 ++++++++++++++++++++++++++++++++++++++++++--------
kernel/auditsc.c | 76 ++--
6 files changed, 848 insertions(+), 178 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 76feae3..4fb7fbd 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -159,6 +159,7 @@
#define AUDIT_INODE 102
#define AUDIT_EXIT 103
#define AUDIT_SUCCESS 104 /* exit >= 0; value ignored */
+#define AUDIT_WATCH 105

#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
diff --git a/init/Kconfig b/init/Kconfig
index 38416a1..7fc7b20 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -177,7 +177,7 @@ config AUDIT

config AUDITSYSCALL
bool "Enable system-call auditing support"
- depends on AUDIT && (X86 || PPC || PPC64 || S390 || IA64 || UML || SPARC64)
+ depends on AUDIT && INOTIFY && (X86 || PPC || PPC64 || S390 || IA64 || UML || SPARC64)
default y if SECURITY_SELINUX
help
Enable low-overhead system-call auditing infrastructure that
diff --git a/kernel/audit.c b/kernel/audit.c
index 6a44e0a..b0d7fb7 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,12 @@ static atomic_t audit_lost = ATOMIC_I
/* The netlink socket. */
static struct sock *audit_sock;

+/* Inotify device. */
+struct inotify_device *audit_idev;
+
+/* Audit filter lists, initialized in audit_init() */
+extern struct audit_flist audit_filter_list[];
+
/* 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). */
@@ -552,6 +561,14 @@ static void audit_receive(struct sock *s
/* Initialize audit support at boot time. */
static int __init audit_init(void)
{
+ int i;
+
+ /* must be initialized before any audit_log calls */
+ for (i = 0; i < AUDIT_NR_FILTERS; i++) {
+ INIT_LIST_HEAD(&audit_filter_list[i].head);
+ spin_lock_init(&audit_filter_list[i].lock);
+ }
+
printk(KERN_INFO "audit: initializing netlink socket (%s)\n",
audit_default ? "enabled" : "disabled");
audit_sock = netlink_kernel_create(NETLINK_AUDIT, 0, audit_receive,
@@ -564,6 +581,13 @@ static int __init audit_init(void)
audit_initialized = 1;
audit_enabled = audit_default;
audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized");
+
+#ifdef CONFIG_AUDITSYSCALL
+ audit_idev = inotify_init(audit_handle_ievent);
+ if (IS_ERR(audit_idev))
+ audit_panic("cannot initialize inotify device");
+#endif
+
return 0;
}
__initcall(audit_init);
diff --git a/kernel/audit.h b/kernel/audit.h
index 051ac2a..795fb72 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -23,6 +23,8 @@
#include <linux/fs.h>
#include <linux/audit.h>

+struct inotify_event;
+
/* 0 = no checking
1 = put_count checking
2 = verbose put_count checking
@@ -53,6 +55,27 @@ enum audit_state {
};

/* Rule lists */
+struct audit_parent {
+ atomic_t count; /* reference count */
+ unsigned int flags; /* flag in-process removals */
+ u32 wd; /* inotify watch descriptor */
+ dev_t dev; /* associated superblock device */
+ unsigned long ino; /* associated inode number */
+ struct list_head mlist; /* entry in master_parents */
+ struct list_head ilist; /* entry in inotify registration list*/
+ struct list_head watches; /* associated watches */
+};
+
+struct audit_watch {
+ atomic_t count; /* reference count */
+ char *path; /* watch insertion path */
+ dev_t dev; /* associated superblock device */
+ unsigned long ino; /* associated inode number */
+ struct audit_parent *parent; /* associated parent */
+ struct list_head wlist; /* entry in audit_parent.watches list*/
+ struct list_head rules; /* associated rules */
+};
+
struct audit_field {
u32 type;
u32 val;
@@ -70,18 +93,28 @@ struct audit_krule {
u32 buflen; /* for data alloc on list rules */
u32 field_count;
struct audit_field *fields;
+ struct audit_watch *watch; /* associated watch */
+ struct list_head rlist; /* entry in audit_watch.rules list */
};

struct audit_entry {
struct list_head list;
struct rcu_head rcu;
- struct audit_krule rule;
+ unsigned int flags; /* flag list manips in progress */
+ struct audit_krule rule; /* audit rule data */
};

+struct audit_flist {
+ struct list_head head;
+ spinlock_t lock; /* syncs filter data manipulation */
+};

extern int audit_pid;
extern int audit_comparator(const u32 left, const u32 op, const u32 right);
-
+extern int audit_compare_dname_path(const char *dname, const char *path);
+extern void audit_handle_ievent(struct inotify_event *event,
+ const char *dname, 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 b115f51..9ae8be9 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -22,28 +22,66 @@
#include <linux/kernel.h>
#include <linux/audit.h>
#include <linux/kthread.h>
+#include <linux/fs.h>
+#include <linux/namei.h>
#include <linux/netlink.h>
+#include <linux/inotify.h>
#include <linux/selinux.h>
#include "audit.h"

-/* There are three lists of rules -- one to search at task creation
- * time, one to search at syscall entry time, and another to search at
- * syscall exit time. */
-struct list_head audit_filter_list[AUDIT_NR_FILTERS] = {
- LIST_HEAD_INIT(audit_filter_list[0]),
- LIST_HEAD_INIT(audit_filter_list[1]),
- LIST_HEAD_INIT(audit_filter_list[2]),
- LIST_HEAD_INIT(audit_filter_list[3]),
- LIST_HEAD_INIT(audit_filter_list[4]),
- LIST_HEAD_INIT(audit_filter_list[5]),
-#if AUDIT_NR_FILTERS != 6
-#error Fix audit_filter_list initialiser
-#endif
-};
+/* Audit filter lists */
+struct audit_flist audit_filter_list[AUDIT_NR_FILTERS];
+
+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_IN_WATCH IN_MOVE|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF
+#define AUDIT_IN_SELF IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT
+
+/* Flags for stale filterlist data */
+#define AUDIT_ENTRY_DEL 0x01 /* Rule entry deletion in progress. */
+
+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)) {
+ BUG_ON(!list_empty(&parent->watches));
+ 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)) {
+ BUG_ON(!list_empty(&watch->rules));
+ /* watches that were never added don't have a parent */
+ if (watch->parent)
+ audit_put_parent(watch->parent);
+ kfree(watch->path);
+ kfree(watch);
+ }
+}

static inline void audit_free_rule(struct audit_entry *e)
{
int i;
+
+ /* some rules don't have associated watches */
+ if (e->rule.watch)
+ audit_put_watch(e->rule.watch);
if (e->rule.fields)
for (i = 0; i < e->rule.field_count; i++) {
struct audit_field *f = &e->rule.fields[i];
@@ -60,6 +98,65 @@ static inline void audit_free_rule_rcu(s
audit_free_rule(e);
}

+/* Initialize a parent watch entry. */
+static inline struct audit_parent *audit_init_parent(void)
+{
+ struct audit_parent *parent;
+
+ parent = kzalloc(sizeof(*parent), GFP_ATOMIC);
+ if (unlikely(!parent))
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&parent->watches);
+ atomic_set(&parent->count, 1);
+
+ spin_lock(&master_parents_lock);
+ list_add(&parent->mlist, &master_parents);
+ spin_unlock(&master_parents_lock);
+
+ return parent;
+}
+
+/* Initialize a watch entry. */
+static inline struct audit_watch *audit_init_watch(char *path,
+ gfp_t gfp_mask)
+{
+ struct audit_watch *watch;
+
+ watch = kzalloc(sizeof(*watch), gfp_mask);
+ if (unlikely(!watch))
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&watch->rules);
+ atomic_set(&watch->count, 1);
+ watch->path = path;
+ watch->dev = (dev_t)-1;
+ watch->ino = (unsigned long)-1;
+
+ return watch;
+}
+
+/* Initialize an audit filterlist entry. */
+static inline struct audit_entry *audit_init_entry(u32 field_count,
+ gfp_t gfp_mask)
+{
+ struct audit_entry *entry;
+ struct audit_field *fields;
+
+ entry = kzalloc(sizeof(*entry), gfp_mask);
+ if (unlikely(!entry))
+ return NULL;
+
+ fields = kzalloc(sizeof(*fields) * field_count, gfp_mask);
+ if (unlikely(!fields)) {
+ kfree(entry);
+ return NULL;
+ }
+ entry->rule.fields = fields;
+
+ return entry;
+}
+
/* Unpack a filter field's string representation from user-space
* buffer. */
static char *audit_unpack_string(void **bufp, size_t *remain, size_t len)
@@ -87,12 +184,32 @@ static char *audit_unpack_string(void **
return str;
}

+/* Translate a watch string to kernel respresentation. */
+static int audit_to_watch(char *path, struct audit_krule *krule, int fidx)
+{
+ struct audit_field *f = &krule->fields[fidx];
+ struct audit_watch *watch;
+
+ if (path[0] != '/' || path[f->val-1] == '/' ||
+ krule->listnr != AUDIT_FILTER_EXIT ||
+ f->op & ~AUDIT_EQUAL)
+ return -EINVAL;
+
+ watch = audit_init_watch(path, GFP_KERNEL);
+ if (unlikely(IS_ERR(watch)))
+ return PTR_ERR(watch);
+
+ audit_get_watch(watch);
+ krule->watch = watch;
+
+ return 0;
+}
+
/* Common user-space to kernel rule translation. */
static inline struct audit_entry *audit_to_entry_common(struct audit_rule *rule)
{
unsigned listnr;
struct audit_entry *entry;
- struct audit_field *fields;
int i, err;

err = -EINVAL;
@@ -116,23 +233,14 @@ static inline struct audit_entry *audit_
goto exit_err;

err = -ENOMEM;
- entry = kmalloc(sizeof(*entry), GFP_KERNEL);
- if (unlikely(!entry))
+ entry = audit_init_entry(rule->field_count, GFP_KERNEL);
+ if (!entry)
goto exit_err;
- fields = kmalloc(sizeof(*fields) * rule->field_count, GFP_KERNEL);
- if (unlikely(!fields)) {
- kfree(entry);
- goto exit_err;
- }
-
- memset(&entry->rule, 0, sizeof(struct audit_krule));
- memset(fields, 0, sizeof(struct audit_field));

entry->rule.flags = rule->flags & AUDIT_FILTER_PREPEND;
entry->rule.listnr = listnr;
entry->rule.action = rule->action;
entry->rule.field_count = rule->field_count;
- entry->rule.fields = fields;

for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
entry->rule.mask[i] = rule->mask[i];
@@ -167,7 +275,8 @@ static struct audit_entry *audit_rule_to
f->type == AUDIT_SE_ROLE ||
f->type == AUDIT_SE_TYPE ||
f->type == AUDIT_SE_SEN ||
- f->type == AUDIT_SE_CLR) {
+ f->type == AUDIT_SE_CLR ||
+ f->type == AUDIT_WATCH) {
err = -EINVAL;
goto exit_free;
}
@@ -249,6 +358,18 @@ static struct audit_entry *audit_data_to
} else
f->se_str = str;
break;
+ case AUDIT_WATCH:
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = audit_to_watch(str, &entry->rule, i);
+ if (err) {
+ kfree(str);
+ goto exit_free;
+ }
+ break;
}
}

@@ -278,7 +399,8 @@ static struct audit_rule *audit_krule_to
struct audit_rule *rule;
int i;

- rule = kmalloc(sizeof(*rule), GFP_KERNEL);
+ /* use GFP_ATOMIC because we're under rcu_read_lock() */
+ rule = kmalloc(sizeof(*rule), GFP_ATOMIC);
if (unlikely(!rule))
return ERR_PTR(-ENOMEM);
memset(rule, 0, sizeof(*rule));
@@ -309,7 +431,8 @@ static struct audit_rule_data *audit_kru
void *bufp;
int i;

- data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
+ /* use GFP_ATOMIC because we're under rcu_read_lock() */
+ data = kmalloc(sizeof(*data) + krule->buflen, GFP_ATOMIC);
if (unlikely(!data))
return ERR_PTR(-ENOMEM);
memset(data, 0, sizeof(*data));
@@ -332,6 +455,10 @@ static struct audit_rule_data *audit_kru
data->buflen += data->values[i] =
audit_pack_string(&bufp, f->se_str);
break;
+ case AUDIT_WATCH:
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, krule->watch->path);
+ break;
default:
data->values[i] = f->val;
}
@@ -341,6 +468,12 @@ static struct audit_rule_data *audit_kru
return data;
}

+/* Compare two watches. Considered success if rules don't match. */
+static inline int audit_compare_watch(struct audit_watch *a, struct audit_watch *b)
+{
+ return strcmp(a->path, b->path);
+}
+
/* Compare two rules in kernel format. Considered success if rules
* don't match. */
static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b)
@@ -367,6 +500,10 @@ static int audit_compare_rule(struct aud
if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
return 1;
break;
+ case AUDIT_WATCH:
+ if (audit_compare_watch(a->watch, b->watch))
+ return 1;
+ break;
default:
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -380,22 +517,416 @@ static int audit_compare_rule(struct aud
return 0;
}

+/* Duplicate the given audit watch. The new watch's rules list is initialized
+ * to an empty list and wlist is undefined. */
+static inline struct audit_watch *audit_dupe_watch(struct audit_watch *old)
+{
+ char *path;
+ struct audit_watch *new;
+
+ path = kstrdup(old->path, GFP_ATOMIC);
+ if (unlikely(!path))
+ return ERR_PTR(-ENOMEM);
+
+ new = audit_init_watch(path, GFP_ATOMIC);
+ if (unlikely(!new)) {
+ kfree(path);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ new->dev = old->dev;
+ new->ino = old->ino;
+ audit_get_parent(old->parent);
+ new->parent = old->parent;
+
+ return new;
+}
+
+/* Duplicate selinux field information. The se_rule is opaque, so must be
+ * re-initialized. */
+static inline int audit_dupe_selinux_field(struct audit_field *df,
+ struct audit_field *sf)
+{
+ int ret = 0;
+ char *se_str;
+
+ /* our own copy of se_str */
+ se_str = kstrdup(sf->se_str, GFP_ATOMIC);
+ if (unlikely(IS_ERR(se_str)))
+ return -ENOMEM;
+ df->se_str = se_str;
+
+ /* our own (refreshed) copy of se_rule */
+ ret = selinux_audit_rule_init(df->type, df->op, df->se_str,
+ &df->se_rule);
+ /* Keep currently invalid fields around in case they
+ * become valid after a policy reload. */
+ if (ret == -EINVAL) {
+ printk(KERN_WARNING "audit rule for selinux \'%s\' is invalid\n",
+ df->se_str);
+ ret = 0;
+ }
+
+ return ret;
+}
+
+/* Duplicate an audit rule. This will be a deep copy with the exception
+ * of the watch - that pointer is carried over. The selinux specific fields
+ * will be updated in the copy. The point is to be able to replace the old
+ * rule with the new rule in the filterlist, then free the old rule. The rlist
+ * element is undefined; list manipulations should happen elsewhere. */
+static struct audit_entry *audit_dupe_rule(struct audit_krule *old,
+ struct audit_watch *watch)
+{
+ u32 fcount = old->field_count;
+ struct audit_entry *entry;
+ struct audit_krule *new;
+ int i, err = 0;
+
+ entry = audit_init_entry(fcount, GFP_ATOMIC);
+ if (unlikely(!entry))
+ return ERR_PTR(-ENOMEM);
+
+ new = &entry->rule;
+ new->vers_ops = old->vers_ops;
+ new->flags = old->flags;
+ new->listnr = old->listnr;
+ new->action = old->action;
+ for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
+ new->mask[i] = old->mask[i];
+ new->buflen = old->buflen;
+ new->watch = NULL;
+ new->field_count = old->field_count;
+ memcpy(new->fields, old->fields, sizeof(struct audit_field) * fcount);
+
+ /* deep copy this information, updating the se_rule fields, because
+ the originals will all be freed when the old rule is freed. */
+ for (i = 0; i < fcount; i++) {
+ switch (new->fields[i].type) {
+ case AUDIT_SE_USER:
+ case AUDIT_SE_ROLE:
+ case AUDIT_SE_TYPE:
+ case AUDIT_SE_SEN:
+ case AUDIT_SE_CLR:
+ err = audit_dupe_selinux_field(&new->fields[i],
+ &old->fields[i]);
+ }
+ if (err) {
+ audit_free_rule(entry);
+ return ERR_PTR(err);
+ }
+ }
+
+ if (watch) {
+ audit_get_watch(watch);
+ new->watch = watch;
+ }
+
+ return entry;
+}
+
+/* Update inode numbers in audit rules based on filesystem event. */
+static inline void audit_update_watch(struct audit_parent *parent,
+ const char *dname, dev_t dev,
+ unsigned long ino)
+{
+ struct audit_flist *flist = &audit_filter_list[AUDIT_FILTER_EXIT];
+ struct audit_watch *owatch, *nwatch, *nextw;
+ struct audit_krule *r, *nextr;
+ struct audit_entry *oentry, *nentry;
+ struct audit_buffer *ab;
+
+ spin_lock(&flist->lock);
+ list_for_each_entry_safe(owatch, nextw, &parent->watches, wlist) {
+ if (audit_compare_dname_path(dname, owatch->path))
+ continue;
+
+ nwatch = audit_dupe_watch(owatch);
+ if (unlikely(IS_ERR(nwatch))) {
+ audit_panic("error updating watch");
+ ab = audit_log_start(NULL, GFP_ATOMIC,
+ AUDIT_CONFIG_CHANGE);
+ audit_log_format(ab,
+ "audit skipped update for rules specifying watch=");
+ audit_log_untrustedstring(ab, owatch->path);
+ audit_log_format(ab, " with ino=%lu\n", ino);
+ audit_log_end(ab);
+ return;
+ }
+ nwatch->dev = dev;
+ nwatch->ino = ino;
+
+ list_for_each_entry_safe(r, nextr, &owatch->rules, rlist) {
+ oentry = container_of(r, struct audit_entry, rule);
+ if (oentry->flags & AUDIT_ENTRY_DEL)
+ continue;
+
+ nentry = audit_dupe_rule(&oentry->rule, nwatch);
+ if (unlikely(IS_ERR(nentry))) {
+ audit_panic("error updating watch");
+ audit_log(NULL, GFP_ATOMIC, AUDIT_CONFIG_CHANGE,
+ "audit removed rule to be updated\n");
+ list_del(&oentry->rule.rlist);
+ list_del_rcu(&oentry->list);
+ } else {
+ list_add(&nentry->rule.rlist, &nwatch->rules);
+ list_del(&oentry->rule.rlist);
+ list_replace_rcu(&oentry->list, &nentry->list);
+ }
+ oentry->flags |= AUDIT_ENTRY_DEL;
+ call_rcu(&oentry->rcu, audit_free_rule_rcu);
+ }
+
+ ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_CONFIG_CHANGE);
+ audit_log_format(ab, "audit updated rules specifying watch=");
+ audit_log_untrustedstring(ab, owatch->path);
+ audit_log_format(ab, " with ino=%lu\n", ino);
+ audit_log_end(ab);
+
+ list_del(&owatch->wlist);
+ audit_put_watch(owatch);
+ goto add_watch_to_parent; /* event applies to a single watch */
+ }
+ spin_unlock(&flist->lock);
+ return;
+
+add_watch_to_parent:
+ list_add(&nwatch->wlist, &parent->watches);
+ spin_unlock(&flist->lock);
+ return;
+}
+
+/* Remove all watches & rules associated with a parent that is going away. */
+static inline void audit_remove_parent_watches(struct audit_parent *parent)
+{
+ struct audit_watch *w, *nextw;
+ struct audit_krule *r, *nextr;
+ struct audit_entry *e;
+ struct audit_flist *flist = &audit_filter_list[AUDIT_FILTER_EXIT];
+
+ spin_lock(&flist->lock);
+ list_for_each_entry_safe(w, nextw, &parent->watches, wlist) {
+ list_for_each_entry_safe(r, nextr, &w->rules, rlist) {
+ e = container_of(r, struct audit_entry, rule);
+ if (e->flags & AUDIT_ENTRY_DEL)
+ continue;
+
+ list_del(&r->rlist);
+ list_del_rcu(&e->list);
+ e->flags |= AUDIT_ENTRY_DEL;
+ call_rcu(&e->rcu, audit_free_rule_rcu);
+ audit_log(NULL, GFP_ATOMIC, AUDIT_CONFIG_CHANGE,
+ "audit implicitly removed rule from list=%d\n",
+ AUDIT_FILTER_EXIT);
+ }
+ list_del(&w->wlist);
+ audit_put_watch(w);
+ }
+ spin_unlock(&flist->lock);
+}
+
+/* Actually remove the parent; inotify has acknowleged the removal. */
+static inline void audit_remove_parent(struct audit_parent *parent)
+{
+ BUG_ON(!list_empty(&parent->watches));
+ spin_lock(&master_parents_lock);
+ list_del(&parent->mlist);
+ audit_put_parent(parent);
+ spin_unlock(&master_parents_lock);
+}
+
+/* Register inotify watches for parents on in_list. */
+static int audit_inotify_register(struct nameidata *nd,
+ struct list_head *in_list)
+{
+ struct audit_parent *p;
+ s32 wd;
+ int ret = 0;
+
+ list_for_each_entry(p, in_list, ilist) {
+ audit_get_parent(p);
+
+ if (!audit_idev)
+ wd = -EOPNOTSUPP;
+ else
+ wd = inotify_add_watch(audit_idev, nd->dentry->d_inode,
+ AUDIT_IN_WATCH, p);
+ if (wd < 0) {
+ audit_remove_parent_watches(p);
+ audit_remove_parent(p);
+ audit_put_parent(p);
+ ret = wd;
+ } else {
+ /* These values only read during operations which are
+ * currently prevented by audit_netlink_sem. */
+ struct inode *inode = nd->dentry->d_inode;
+ p->wd = wd;
+ p->dev = inode->i_sb->s_dev;
+ p->ino = inode->i_ino;
+ }
+
+ audit_put_parent(p);
+ }
+
+ return ret;
+}
+
+/* Unregister inotify watches for parents on in_list.
+ * Generates an IN_IGNORED event. */
+static void audit_inotify_unregister(struct list_head *in_list)
+{
+ struct audit_parent *p;
+
+ list_for_each_entry(p, in_list, ilist) {
+ if (audit_idev)
+ inotify_ignore(audit_idev, p->wd);
+ audit_put_parent(p);
+ }
+}
+
+/* Get path information necessary for adding watches. */
+static int audit_get_nd(char *path, struct nameidata **ndp,
+ struct nameidata **ndw)
+{
+ struct nameidata *ndparent, *ndwatch;
+ int err;
+
+ ndparent = kmalloc(sizeof(*ndparent), GFP_KERNEL);
+ if (unlikely(!ndparent))
+ return -ENOMEM;
+
+ ndwatch = kmalloc(sizeof(*ndwatch), GFP_KERNEL);
+ if (unlikely(!ndwatch)) {
+ kfree(ndparent);
+ return -ENOMEM;
+ }
+
+ err = path_lookup(path, LOOKUP_PARENT, ndparent);
+ if (err) {
+ kfree(ndparent);
+ kfree(ndwatch);
+ return err;
+ }
+
+ err = path_lookup(path, 0, ndwatch);
+ if (err) {
+ kfree(ndwatch);
+ ndwatch = NULL;
+ }
+
+ *ndp = ndparent;
+ *ndw = ndwatch;
+
+ return 0;
+}
+
+/* Release resources used for watch path information. */
+static inline void audit_put_nd(struct nameidata *ndp, struct nameidata *ndw)
+{
+ if (ndp) {
+ path_release(ndp);
+ kfree(ndp);
+ }
+ if (ndw) {
+ path_release(ndw);
+ kfree(ndw);
+ }
+}
+
+/* Find an existing parent entry for this watch, or create a new one.
+ * Caller must hold exit filterlist lock. */
+static inline struct audit_parent *audit_find_parent(struct nameidata *nd,
+ struct list_head *in_list)
+{
+ struct audit_parent *p, *parent, *next;
+ struct inode *inode = nd->dentry->d_inode;
+
+ list_for_each_entry_safe(p, next, &master_parents, mlist) {
+ if (p->ino != inode->i_ino ||
+ p->dev != inode->i_sb->s_dev)
+ continue;
+
+ parent = p;
+ goto out;
+ }
+
+ parent = audit_init_parent();
+ if (IS_ERR(parent))
+ goto out;
+ /* add new parent to inotify registration list */
+ list_add(&parent->ilist, in_list);
+
+out:
+ return parent;
+}
+
+/* Find a matching watch entry, or add this one.
+ * Caller must hold exit filterlist lock. */
+static inline int audit_add_watch(struct audit_krule *krule,
+ struct nameidata *ndp, struct nameidata *ndw,
+ struct list_head *list)
+{
+ struct audit_parent *parent;
+ struct audit_watch *w, *watch = krule->watch;
+
+ parent = audit_find_parent(ndp, list);
+ if (IS_ERR(parent))
+ return PTR_ERR(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:
+ list_add(&krule->rlist, &watch->rules);
+
+ if (ndw) {
+ watch->dev = ndw->dentry->d_inode->i_sb->s_dev;
+ watch->ino = ndw->dentry->d_inode->i_ino;
+ }
+
+ return 0;
+}
+
/* Add rule to given filterlist if not a duplicate. Protected by
* audit_netlink_mutex. */
static inline int audit_add_rule(struct audit_entry *entry,
- struct list_head *list)
+ struct audit_flist *flist)
{
struct audit_entry *e;
+ struct audit_watch *watch = entry->rule.watch;
+ struct nameidata *ndp, *ndw;
+ LIST_HEAD(inotify_list);
+ int err;
#ifdef CONFIG_AUDITSYSCALL
int dont_count = 0;
#endif

- /* 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 automatic filterlist
+ * updates or removals. */
+ rcu_read_lock();
+ list_for_each_entry_rcu(e, &flist->head, list) {
+ if (e->flags & AUDIT_ENTRY_DEL)
+ continue;
+ if (!audit_compare_rule(&entry->rule, &e->rule)) {
+ err = -EEXIST;
+ rcu_read_unlock();
+ goto error;
+ }
}
+ rcu_read_unlock();

/* If either of these, don't count towards total */
#ifdef CONFIG_AUDITSYSCALL
@@ -403,43 +934,111 @@ static inline int audit_add_rule(struct
entry->rule.listnr == AUDIT_FILTER_TYPE)
dont_count = 1;
#endif
+ /* Get watch nameidata before taking spinlock */
+ if (watch) {
+ err = audit_get_nd(watch->path, &ndp, &ndw);
+ if (err)
+ goto error;
+ }
+
+ spin_lock(&flist->lock);
+ if (watch) {
+ err = audit_add_watch(&entry->rule, ndp, ndw, &inotify_list);
+ if (err) {
+ audit_put_nd(ndp, ndw);
+ goto error;
+ }
+ }
if (entry->rule.flags & AUDIT_FILTER_PREPEND) {
- list_add_rcu(&entry->list, list);
+ list_add_rcu(&entry->list, &flist->head);
} else {
- list_add_tail_rcu(&entry->list, list);
+ list_add_tail_rcu(&entry->list, &flist->head);
}
+ spin_unlock(&flist->lock);
+
#ifdef CONFIG_AUDITSYSCALL
if (!dont_count)
audit_n_rules++;
#endif

+ if (watch) {
+ err = audit_inotify_register(ndp, &inotify_list);
+ if (err)
+ goto error;
+ audit_put_nd(ndp, ndw);
+ }
+
return 0;
+
+error:
+ if (watch)
+ audit_put_watch(watch);
+ return err;
+}
+
+/* Remove given krule from its associated watch's rules list and clean up any
+ * last instances of associated watch and parent.
+ * Caller must hold exit filterlist lock */
+static inline void audit_remove_watch(struct audit_krule *krule,
+ struct list_head *in_list)
+{
+ struct audit_watch *watch = krule->watch;
+ struct audit_parent *parent = watch->parent;
+
+ list_del(&krule->rlist);
+ if (list_empty(&watch->rules)) {
+ list_del(&watch->wlist);
+ audit_put_watch(watch);
+
+ if (list_empty(&parent->watches)) {
+ /* put parent on the inotify un-registration list */
+ list_add(&parent->ilist, in_list);
+ audit_get_parent(parent);
+ }
+ }
}

/* Remove an existing rule from filterlist. Protected by
* audit_netlink_mutex. */
static inline int audit_del_rule(struct audit_entry *entry,
- struct list_head *list)
+ struct audit_flist *flist)
{
struct audit_entry *e;
+ LIST_HEAD(inotify_list);

- /* 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);
+ spin_lock(&flist->lock);
+ list_for_each_entry(e, &flist->head, list) {
+ if (e->flags & AUDIT_ENTRY_DEL ||
+ audit_compare_rule(&entry->rule, &e->rule))
+ continue;
+
+ if (e->rule.watch) {
+ audit_remove_watch(&e->rule, &inotify_list);
+ audit_put_watch(entry->rule.watch);
+ }
+
+ list_del_rcu(&e->list);
+ e->flags |= AUDIT_ENTRY_DEL;
#ifdef CONFIG_AUDITSYSCALL
- if (entry->rule.listnr == AUDIT_FILTER_USER ||
- entry->rule.listnr == AUDIT_FILTER_TYPE)
- audit_n_rules++;
+ if (entry->rule.listnr == AUDIT_FILTER_USER ||
+ entry->rule.listnr == AUDIT_FILTER_TYPE)
+ audit_n_rules++;
#endif
- call_rcu(&e->rcu, audit_free_rule_rcu);
+ call_rcu(&e->rcu, audit_free_rule_rcu);
#ifdef CONFIG_AUDITSYSCALL
- audit_n_rules--;
+ audit_n_rules--;
#endif
- return 0;
- }
+ spin_unlock(&flist->lock);
+ audit_n_rules--;
+
+ if (e->rule.watch)
+ audit_inotify_unregister(&inotify_list);
+
+ return 0;
}
+ spin_unlock(&flist->lock);
+ if (entry->rule.watch)
+ audit_put_watch(entry->rule.watch);
return -ENOENT; /* No matching rule */
}

@@ -458,10 +1057,12 @@ static int audit_list(void *_dest)

mutex_lock(&audit_netlink_mutex);

- /* The *_rcu iterators not needed here because we are
- always called with audit_netlink_mutex 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].head,
+ list) {
struct audit_rule *rule;

rule = audit_krule_to_rule(&entry->rule);
@@ -471,6 +1072,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);

@@ -492,19 +1094,21 @@ static int audit_list_rules(void *_dest)

mutex_lock(&audit_netlink_mutex);

- /* The *_rcu iterators not needed here because we are
- always called with audit_netlink_mutex 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].head, list) {
struct audit_rule_data *data;

data = audit_krule_to_data(&e->rule);
if (unlikely(!data))
break;
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
- data, sizeof(*data));
+ data, sizeof(*data) + data->buflen);
kfree(data);
}
+ rcu_read_unlock();
}
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);

@@ -597,6 +1201,32 @@ int audit_receive_filter(int type, int p
return err;
}

+/**
+ * audit_handle_ievent - handler for Inotify events
+ * @event: information about the event
+ * @dname: dentry name associated with event
+ * @inode: inode associated with event
+ * @ptr: kernel's version of a watch descriptor
+ */
+void audit_handle_ievent(struct inotify_event *event, const char *dname,
+ struct inode *inode, void *ptr)
+{
+ struct audit_parent *parent = (struct audit_parent *)ptr;
+
+ if (event->mask & (IN_CREATE|IN_MOVED_TO) && inode)
+ audit_update_watch(parent, dname, inode->i_sb->s_dev,
+ inode->i_ino);
+ else if (event->mask & (IN_DELETE|IN_MOVED_FROM))
+ audit_update_watch(parent, dname, (dev_t)-1, (unsigned long)-1);
+ /* Note: Inotify doesn't remove the watch for the IN_MOVE_SELF event.
+ * Work around this by leaving the parent around with an empty
+ * watchlist. It will be re-used if new watches are added. */
+ else if (event->mask & (AUDIT_IN_SELF))
+ audit_remove_parent_watches(parent);
+ else if (event->mask & IN_IGNORED)
+ audit_remove_parent(parent);
+}
+
int audit_comparator(const u32 left, const u32 op, const u32 right)
{
switch (op) {
@@ -617,7 +1247,39 @@ int audit_comparator(const u32 left, con
return 0;
}

+/* Compare given dentry name with last component in given path,
+ * return of 0 indicates a match. */
+int audit_compare_dname_path(const char *dname, const char *path)
+{
+ int dlen, plen;
+ const char *p;
+
+ if (!dname || !path)
+ return 1;
+
+ dlen = strlen(dname);
+ plen = strlen(path);
+ if (plen < dlen)
+ return 1;

+ /* disregard trailing slashes */
+ p = path + plen - 1;
+ while ((*p == '/') && (p > path))
+ p--;
+
+ /* find last path component */
+ p = p - dlen + 1;
+ if (p < path)
+ return 1;
+ else if (p > path) {
+ if (*--p != '/')
+ return 1;
+ else
+ p++;
+ }
+
+ return strncmp(p, dname, dlen);
+}

static int audit_filter_user_rules(struct netlink_skb_parms *cb,
struct audit_krule *rule,
@@ -662,7 +1324,8 @@ int audit_filter_user(struct netlink_skb
int ret = 1;

rcu_read_lock();
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER].head,
+ list) {
if (audit_filter_user_rules(cb, &e->rule, &state)) {
if (state == AUDIT_DISABLED)
ret = 0;
@@ -680,10 +1343,10 @@ int audit_filter_type(int type)
int result = 0;

rcu_read_lock();
- if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE]))
+ if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE].head))
goto unlock_and_return;

- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE],
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE].head,
list) {
int i;
for (i = 0; i < e->rule.field_count; i++) {
@@ -723,62 +1386,6 @@ static inline int audit_rule_has_selinux
return 0;
}

-/* Make a copy of src in dest. This will be a deep copy with the exception
- of the watch - that pointer is carried over. The selinux specific fields
- will be updated in the copy. The point is to be able to replace the src
- rule with the dest rule in the list, then free the dest rule. */
-static inline int selinux_audit_rule_update_helper(struct audit_krule *dest,
- struct audit_krule *src)
-{
- int i, err = 0;
-
- dest->vers_ops = src->vers_ops;
- dest->flags = src->flags;
- dest->listnr = src->listnr;
- dest->action = src->action;
- for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
- dest->mask[i] = src->mask[i];
- dest->buflen = src->buflen;
- dest->field_count = src->field_count;
-
- /* deep copy this information, updating the se_rule fields, because
- the originals will all be freed when the old rule is freed. */
- dest->fields = kzalloc(sizeof(struct audit_field) * dest->field_count,
- GFP_ATOMIC);
- if (!dest->fields)
- return -ENOMEM;
- memcpy(dest->fields, src->fields,
- sizeof(struct audit_field) * dest->field_count);
- for (i = 0; i < dest->field_count; i++) {
- struct audit_field *df = &dest->fields[i];
- struct audit_field *sf = &src->fields[i];
- switch (df->type) {
- case AUDIT_SE_USER:
- case AUDIT_SE_ROLE:
- case AUDIT_SE_TYPE:
- case AUDIT_SE_SEN:
- case AUDIT_SE_CLR:
- /* our own copy of se_str */
- df->se_str = kstrdup(sf->se_str, GFP_ATOMIC);
- if (!df->se_str)
- return -ENOMEM;
- /* our own (refreshed) copy of se_rule */
- err = selinux_audit_rule_init(df->type, df->op,
- df->se_str, &df->se_rule);
- /* Keep currently invalid fields around in case they
- become valid after a policy reload. */
- if (err == -EINVAL) {
- printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", df->se_str);
- err = 0;
- }
- if (err)
- return err;
- }
- }
-
- return 0;
-}
-
/* This function will re-initialize the se_rule field of all applicable rules.
It will traverse the filter lists serarching for rules that contain selinux
specific filter fields. When such a rule is found, it is copied, the
@@ -787,44 +1394,41 @@ static inline int selinux_audit_rule_upd
static int selinux_audit_rule_update(void)
{
struct audit_entry *entry, *nentry;
- int i, err = 0, tmperr;
-
- /* audit_netlink_mutex synchronizes the writers */
- mutex_lock(&audit_netlink_mutex);
+ struct audit_watch *watch;
+ int i, err = 0;

for (i = 0; i < AUDIT_NR_FILTERS; i++) {
- list_for_each_entry(entry, &audit_filter_list[i], list) {
+ /* filterlist lock synchronizes the writers */
+ spin_lock(&audit_filter_list[i].lock);
+ list_for_each_entry(entry, &audit_filter_list[i].head, list) {
if (!audit_rule_has_selinux(&entry->rule))
continue;

- nentry = kmalloc(sizeof(*entry), GFP_ATOMIC);
- if (!nentry) {
+ watch = entry->rule.watch;
+ nentry = audit_dupe_rule(&entry->rule, watch);
+ if (unlikely(IS_ERR(nentry))) {
/* save the first error encountered for the
- return value */
+ * return value */
if (!err)
- err = -ENOMEM;
+ err = PTR_ERR(nentry);
audit_panic("error updating selinux filters");
- continue;
+ if (watch)
+ list_del(&entry->rule.rlist);
+ list_del_rcu(&entry->list);
+ } else {
+ if (watch) {
+ list_add(&nentry->rule.rlist,
+ &watch->rules);
+ list_del(&entry->rule.rlist);
+ }
+ list_replace_rcu(&entry->list, &nentry->list);
}
-
- tmperr = selinux_audit_rule_update_helper(&nentry->rule,
- &entry->rule);
- if (tmperr) {
- /* save the first error encountered for the
- return value */
- if (!err)
- err = tmperr;
- audit_free_rule(nentry);
- audit_panic("error updating selinux filters");
- continue;
- }
- list_replace_rcu(&entry->list, &nentry->list);
+ entry->flags |= AUDIT_ENTRY_DEL;
call_rcu(&entry->rcu, audit_free_rule_rcu);
}
+ spin_unlock(&audit_filter_list[i].lock);
}

- mutex_unlock(&audit_netlink_mutex);
-
return err;
}

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 05a2dc1..921e79b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -62,7 +62,7 @@

#include "audit.h"

-extern struct list_head audit_filter_list[];
+extern struct audit_flist audit_filter_list[];

/* No syscall auditing will take place unless audit_enabled != 0. */
extern int audit_enabled;
@@ -166,6 +166,27 @@ struct audit_context {
#endif
};

+/* Determine if any context name data matches a rule's watch data */
+static inline int audit_match_watch(struct audit_context *ctx,
+ struct audit_watch *watch)
+{
+ int i;
+
+ if (!ctx)
+ return 0;
+
+ if (watch->ino == (unsigned long)-1)
+ return 0;
+
+ for (i = 0; i < ctx->name_count; i++) {
+ if (ctx->names[i].dev == watch->dev &&
+ (ctx->names[i].ino == watch->ino ||
+ ctx->names[i].pino == watch->ino))
+ return 1;
+ }
+
+ return 0;
+}

/* Compare a task_struct with an audit_rule. Return 1 on match, 0
* otherwise. */
@@ -252,7 +273,7 @@ static int audit_filter_rules(struct tas
}
break;
case AUDIT_INODE:
- if (ctx) {
+ if (ctx && f->val != (unsigned int)-1) {
for (j = 0; j < ctx->name_count; j++) {
if (audit_comparator(ctx->names[j].ino, f->op, f->val) ||
audit_comparator(ctx->names[j].pino, f->op, f->val)) {
@@ -262,6 +283,9 @@ static int audit_filter_rules(struct tas
}
}
break;
+ case AUDIT_WATCH:
+ result = audit_match_watch(ctx, rule->watch);
+ break;
case AUDIT_LOGINUID:
result = 0;
if (ctx)
@@ -313,7 +337,8 @@ static enum audit_state audit_filter_tas
enum audit_state state;

rcu_read_lock();
- list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list) {
+ list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK].head,
+ list) {
if (audit_filter_rules(tsk, &e->rule, NULL, &state)) {
rcu_read_unlock();
return state;
@@ -369,7 +394,7 @@ static inline struct audit_context *audi

if (context->in_syscall && !context->auditable) {
enum audit_state state;
- state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_EXIT]);
+ state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_EXIT].head);
if (state == AUDIT_RECORD_CONTEXT)
context->auditable = 1;
}
@@ -847,7 +872,7 @@ void audit_syscall_entry(struct task_str

state = context->state;
if (state == AUDIT_SETUP_CONTEXT || state == AUDIT_BUILD_CONTEXT)
- state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_ENTRY]);
+ state = audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_ENTRY].head);
if (likely(state == AUDIT_DISABLED))
return;

@@ -1091,37 +1116,20 @@ void __audit_inode_child(const char *dna
return;

/* determine matching parent */
- if (dname)
- for (idx = 0; idx < context->name_count; idx++)
- if (context->names[idx].pino == pino) {
- const char *n;
- const char *name = context->names[idx].name;
- int dlen = strlen(dname);
- int nlen = name ? strlen(name) : 0;
-
- if (nlen < dlen)
- continue;
-
- /* disregard trailing slashes */
- n = name + nlen - 1;
- while ((*n == '/') && (n > name))
- n--;
-
- /* find last path component */
- n = n - dlen + 1;
- if (n < name)
- continue;
- else if (n > name) {
- if (*--n != '/')
- continue;
- else
- n++;
- }
+ if (!dname)
+ goto no_match;
+ for (idx = 0; idx < context->name_count; idx++)
+ if (context->names[idx].pino == pino) {
+ const char *name = context->names[idx].name;

- if (strncmp(n, dname, dlen) == 0)
- goto update_context;
- }
+ if (!name)
+ continue;
+
+ if (audit_compare_dname_path(dname, name) == 0)
+ goto update_context;
+ }

+no_match:
/* catch-all in case match not found */
idx = context->name_count++;
context->names[idx].name = NULL;
Alexander Viro
2006-03-10 11:37:02 UTC
Permalink
Post by Amy Griffis
+/* Remove given krule from its associated watch's rules list and clean up any
+ * last instances of associated watch and parent.
+ * Caller must hold exit filterlist lock */
+static inline void audit_remove_watch(struct audit_krule *krule,
+ struct list_head *in_list)
+{
+ struct audit_watch *watch = krule->watch;
+ struct audit_parent *parent = watch->parent;
+
+ list_del(&krule->rlist);
+ if (list_empty(&watch->rules)) {
+ list_del(&watch->wlist);
+ audit_put_watch(watch);
+
+ if (list_empty(&parent->watches)) {
+ /* put parent on the inotify un-registration list */
+ list_add(&parent->ilist, in_list);
+ audit_get_parent(parent);
+ }
+ }
}
Umm... What guarantees that parent survives audit_put_watch()?

Another thing that looks rather fishy: audit_inotify_register()
+ if (wd < 0) {
+ audit_remove_parent_watches(p);
+ audit_remove_parent(p);
+ audit_put_parent(p);
+ ret = wd;
+ } else {
followed by _another_ audit_put_parent(), balancing audit_get_parent()
before. Since audit_remove_parent() also drops a reference... Something
doesn't add up.

AFAICS, audit_get_watch() puts new parent on list, with refcount 2
and one watch attached. We bump refcount to 3. Then we have
audit_remove_parent_watches() do audit_put_watch() on the only watch,
which will drop its reference to parent, leaving us with refcount 2.
Then audit_remove_parent() will call audit_put_parent(), getting the
refcount to 1. Then we do audit_put_parent(), releaseing the last
reference and freeing p; *then* we do audit_put_parent(p) again. Oops...

Could you put at least short comments on refcounting in there?

One more: audit_put_nd() blocks, so
+ spin_lock(&flist->lock);
+ if (watch) {
+ err = audit_add_watch(&entry->rule, ndp, ndw, &inotify_list);
+ if (err) {
+ audit_put_nd(ndp, ndw);
deadlocks - audit_add_watch() won't drop the spinlock for us. Forgotten
spin_unlock()?

In audit_list():

+ rcu_read_lock();
+ list_for_each_entry_rcu(e, &audit_filter_list[i].head, list) {
struct audit_rule_data *data;

data = audit_krule_to_data(&e->rule);
if (unlikely(!data))
break;
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
- data, sizeof(*data));
+ data, sizeof(*data) + data->buflen);

audit_send_reply() quite obviously blocks. To quote include/linux/rcupdate.h:
/**
* rcu_read_lock - mark the beginning of an RCU read-side critical section.
*
...
*
* It is illegal to block while in an RCU read-side critical section.
*/
#define rcu_read_lock() preempt_disable()


In audit_list_rules(): ditto.
Amy Griffis
2006-03-10 20:06:19 UTC
Permalink
Post by Alexander Viro
Post by Amy Griffis
+/* Remove given krule from its associated watch's rules list and clean up any
+ * last instances of associated watch and parent.
+ * Caller must hold exit filterlist lock */
+static inline void audit_remove_watch(struct audit_krule *krule,
+ struct list_head *in_list)
+{
+ struct audit_watch *watch = krule->watch;
+ struct audit_parent *parent = watch->parent;
+
+ list_del(&krule->rlist);
+ if (list_empty(&watch->rules)) {
+ list_del(&watch->wlist);
+ audit_put_watch(watch);
+
+ if (list_empty(&parent->watches)) {
+ /* put parent on the inotify un-registration list */
+ list_add(&parent->ilist, in_list);
+ audit_get_parent(parent);
+ }
+ }
}
Umm... What guarantees that parent survives audit_put_watch()?
The put that balances the initial get is in audit_remove_parent().
Other than the case where we couldn't add the inotify watch, this is
only reached after receiving the IN_IGNORED event from inotify. We
receive IN_IGNORED after explicitly telling inotify to remove a watch
or if the parent is being removed from the filesystem. In the latter
case, we first receive an event that tells us the parent is going
away, to which we respond by taking the filterlist lock and removing
all watches & rules associated with the parent. We leave the parent
around until we receive the IN_IGNORED confirmation from inotify.

In the code above, the audit_netlink_mutex prevents another explicit
removal, and the filterlist lock requires any of the inotify events
preceding an IN_IGNORED to wait.

The inotify device semaphore prevents us from explicitly removing a
watch between the IN_UNMOUNT and IN_IGNORED events. Since we take an
extra parent reference before releasing the lock, we're okay in
audit_inotify_unregister() and the inotify_ignore() call will silently
fail.

The inotify device semaphore is dropped between processing the
IN_DELETE_SELF and IN_IGNORED events, so it is possible to do an
explicit removal between them. In the situation where we receive
IN_DELETE_SELF (a no-op when the parent.watches list is empty), then
explicitly remove the watch via inotify_ignore(), we won't receive a
second IN_IGNORED event because our watch will no longer be in the
inode's inotify_watches list. I believe this addresses all of the
possible removal scenarios.
Post by Alexander Viro
Another thing that looks rather fishy: audit_inotify_register()
+ if (wd < 0) {
+ audit_remove_parent_watches(p);
+ audit_remove_parent(p);
+ audit_put_parent(p);
+ ret = wd;
+ } else {
followed by _another_ audit_put_parent(), balancing audit_get_parent()
before. Since audit_remove_parent() also drops a reference... Something
doesn't add up.
AFAICS, audit_get_watch() puts new parent on list, with refcount 2
and one watch attached. We bump refcount to 3. Then we have
audit_remove_parent_watches() do audit_put_watch() on the only watch,
which will drop its reference to parent, leaving us with refcount 2.
Then audit_remove_parent() will call audit_put_parent(), getting the
refcount to 1. Then we do audit_put_parent(), releaseing the last
reference and freeing p; *then* we do audit_put_parent(p) again. Oops...
The put after audit_remove_parent() is a mistake. Fixed.
Post by Alexander Viro
Could you put at least short comments on refcounting in there?
Yes, will do.
Post by Alexander Viro
One more: audit_put_nd() blocks, so
+ spin_lock(&flist->lock);
+ if (watch) {
+ err = audit_add_watch(&entry->rule, ndp, ndw, &inotify_list);
+ if (err) {
+ audit_put_nd(ndp, ndw);
deadlocks - audit_add_watch() won't drop the spinlock for us. Forgotten
spin_unlock()?
Yes, thanks.
Post by Alexander Viro
+ rcu_read_lock();
+ list_for_each_entry_rcu(e, &audit_filter_list[i].head, list) {
struct audit_rule_data *data;
data = audit_krule_to_data(&e->rule);
if (unlikely(!data))
break;
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
- data, sizeof(*data));
+ data, sizeof(*data) + data->buflen);
/**
* rcu_read_lock - mark the beginning of an RCU read-side critical section.
*
...
*
* It is illegal to block while in an RCU read-side critical section.
*/
#define rcu_read_lock() preempt_disable()
In audit_list_rules(): ditto.
As discussed on IRC, will drop the per-filterlist spinlocks in favor
of the audit_netlink_mutex. The mutex will synchronize list
manipulations and blocking reads (list rules). The rcu iterator will
protect non-blocking reads (filtering).

Thanks for the review.

Amy
Amy Griffis
2006-03-14 00:18:05 UTC
Permalink
Here is another iteration based off of audit-current.git plus the following
pre-requisites:

selinux support for context based audit filtering:
https://www.redhat.com/archives/linux-audit/2006-February/msg00160.html

context based audit filtering:
https://www.redhat.com/archives/linux-audit/2006-March/msg00107.html

inotify kernel api:
https://www.redhat.com/archives/linux-audit/2006-January/msg00084.html

This version fixes the following:

- remove extra parent put in audit_inotify_register()
- add missing unlock in audit_add_rule() error path
- replace per-filterlist spinlocks with use of audit_netlink_mutex (see below)
- remove now un-needed GFP_ATOMIC allocations
- remove now unused AUDIT_ENTRY_DEL flag - all code paths either avoid stale
data by taking the mutex, or don't care
- take mutex to update parent data in audit_inotify_register()
- kernel enforces 1 watch per rule to avoid potential memleak
- add comments describing locking and refcounts
- miscellaneous code cleanup

The audit_netlink_mutex was previously taken/released in audit_receive() with
the following comment:

/* The netlink socket is only to be read by 1 CPU, which lets us assume
* that list additions and deletions never happen simultaneously in
* auditsc.c */

audit_receive() is three calls up the stack from where we need to release the
mutex for some operations in audit_add_rule() and audit_del_rule(). However,
from what I could see, it didn't seem to be protecting anything specific to the
netlink socket itself, but rather the operations on filterlists. For that
reason I renamed it to audit_filter_mutex and modified the code to use it
explicitly around filterlist manipulations.

Please verify my analysis on this matter. If incorrect we will need two
mutexes: audit_netlink_mutex and audit_filter_mutex.

Thanks,
Amy


---

include/linux/audit.h | 1
init/Kconfig | 2
kernel/audit.c | 19 -
kernel/audit.h | 32 ++
kernel/auditfilter.c | 679 +++++++++++++++++++++++++++++++++++++++++++++++---
kernel/auditsc.c | 65 ++--
6 files changed, 718 insertions(+), 80 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 76feae3..4fb7fbd 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -159,6 +159,7 @@
#define AUDIT_INODE 102
#define AUDIT_EXIT 103
#define AUDIT_SUCCESS 104 /* exit >= 0; value ignored */
+#define AUDIT_WATCH 105

#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
diff --git a/init/Kconfig b/init/Kconfig
index 38416a1..7fc7b20 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -177,7 +177,7 @@ config AUDIT

config AUDITSYSCALL
bool "Enable system-call auditing support"
- depends on AUDIT && (X86 || PPC || PPC64 || S390 || IA64 || UML || SPARC64)
+ depends on AUDIT && INOTIFY && (X86 || PPC || PPC64 || S390 || IA64 || UML || SPARC64)
default y if SECURITY_SELINUX
help
Enable low-overhead system-call auditing infrastructure that
diff --git a/kernel/audit.c b/kernel/audit.c
index 65e1d03..6eff223 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -56,6 +56,7 @@
#include <linux/skbuff.h>
#include <linux/netlink.h>
#include <linux/selinux.h>
+#include <linux/inotify.h>

#include "audit.h"

@@ -102,6 +103,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). */
@@ -114,11 +118,6 @@ static struct task_struct *kauditd_task;
static DECLARE_WAIT_QUEUE_HEAD(kauditd_wait);
static DECLARE_WAIT_QUEUE_HEAD(audit_backlog_wait);

-/* The netlink socket is only to be read by 1 CPU, which lets us assume
- * that list additions and deletions never happen simultaneously in
- * auditsc.c */
-DEFINE_MUTEX(audit_netlink_mutex);
-
/* AUDIT_BUFSIZ is the size of the temporary buffer used for formatting
* audit records. Since printk uses a 1024 byte buffer, this buffer
* should be at least that large. */
@@ -541,14 +540,11 @@ static void audit_receive(struct sock *s
struct sk_buff *skb;
unsigned int qlen;

- mutex_lock(&audit_netlink_mutex);
-
for (qlen = skb_queue_len(&sk->sk_receive_queue); qlen; qlen--) {
skb = skb_dequeue(&sk->sk_receive_queue);
audit_receive_skb(skb);
kfree_skb(skb);
}
- mutex_unlock(&audit_netlink_mutex);
}


@@ -573,6 +569,13 @@ static int __init audit_init(void)
selinux_audit_set_callback(&selinux_audit_rule_update);

audit_log(NULL, GFP_KERNEL, AUDIT_KERNEL, "initialized");
+
+#ifdef CONFIG_AUDITSYSCALL
+ audit_idev = inotify_init(audit_handle_ievent);
+ if (IS_ERR(audit_idev))
+ audit_panic("cannot initialize inotify device");
+#endif
+
return 0;
}
__initcall(audit_init);
diff --git a/kernel/audit.h b/kernel/audit.h
index 6f73392..423e826 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -19,10 +19,11 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/

-#include <linux/mutex.h>
#include <linux/fs.h>
#include <linux/audit.h>

+struct inotify_event;
+
/* 0 = no checking
1 = put_count checking
2 = verbose put_count checking
@@ -53,6 +54,27 @@ enum audit_state {
};

/* Rule lists */
+struct audit_parent {
+ atomic_t count; /* reference count */
+ unsigned int flags; /* flag in-process removals */
+ u32 wd; /* inotify watch descriptor */
+ dev_t dev; /* associated superblock device */
+ unsigned long ino; /* associated inode number */
+ struct list_head mlist; /* entry in master_parents */
+ struct list_head ilist; /* entry in inotify registration list*/
+ struct list_head watches; /* associated watches */
+};
+
+struct audit_watch {
+ atomic_t count; /* reference count */
+ char *path; /* watch insertion path */
+ dev_t dev; /* associated superblock device */
+ unsigned long ino; /* associated inode number */
+ struct audit_parent *parent; /* associated parent */
+ struct list_head wlist; /* entry in audit_parent.watches list*/
+ struct list_head rules; /* associated rules */
+};
+
struct audit_field {
u32 type;
u32 val;
@@ -70,6 +92,8 @@ struct audit_krule {
u32 buflen; /* for data alloc on list rules */
u32 field_count;
struct audit_field *fields;
+ struct audit_watch *watch; /* associated watch */
+ struct list_head rlist; /* entry in audit_watch.rules list */
};

struct audit_entry {
@@ -81,12 +105,14 @@ struct audit_entry {

extern int audit_pid;
extern int audit_comparator(const u32 left, const u32 op, const u32 right);
-
+extern int audit_compare_dname_path(const char *dname, const char *path);
extern void audit_send_reply(int pid, int seq, int type,
int done, int multi,
void *payload, int size);
extern void audit_log_lost(const char *message);
extern void audit_panic(const char *message);
-extern struct mutex audit_netlink_mutex;

extern int selinux_audit_rule_update(void);
+extern void audit_handle_ievent(struct inotify_event *event,
+ const char *dname, struct inode * inode,
+ void *ptr);
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index c895de7..0a4c99d 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -22,13 +22,45 @@
#include <linux/kernel.h>
#include <linux/audit.h>
#include <linux/kthread.h>
+#include <linux/mutex.h>
+#include <linux/fs.h>
+#include <linux/namei.h>
#include <linux/netlink.h>
+#include <linux/inotify.h>
#include <linux/selinux.h>
#include "audit.h"

-/* There are three lists of rules -- one to search at task creation
- * time, one to search at syscall entry time, and another to search at
- * syscall exit time. */
+/*
+ * Locking model:
+ *
+ * audit_filter_mutex:
+ * Synchronizes writes and blocking reads of audit's filterlist
+ * data. Rcu is used to traverse the filterlist and access
+ * contents of structs audit_entry, audit_watch and opaque
+ * selinux rules during filtering. If modified, these structures
+ * must be copied and replace their counterparts in the filterlist.
+ * An audit_parent struct is not accessed during filtering, so may
+ * be written directly provided audit_filter_mutex is held.
+ *
+ * master_parents_lock: (spinlock)
+ * Protects master_parents list.
+ */
+
+/*
+ * Reference counting:
+ *
+ * audit_parent: lifetime is from audit_init_parent() to audit_remove_parent().
+ * Each audit_watch holds a reference to its associated parent.
+ *
+ * audit_watch: if added to lists, lifetime is from audit_init_watch() to one
+ * of: audit_remove_watch() [user removes], audit_update_watch() [kernel
+ * replaces], or audit_remove_parent_watches() [kernel removes].
+ * Additionally, an audit_watch may exist temporarily to assist in
+ * searching existing filter data. Each audit_krule holds a reference to
+ * its associated watch.
+ */
+
+/* Audit filter lists, defined in <linux/audit.h> */
struct list_head audit_filter_list[AUDIT_NR_FILTERS] = {
LIST_HEAD_INIT(audit_filter_list[0]),
LIST_HEAD_INIT(audit_filter_list[1]),
@@ -41,9 +73,55 @@ struct list_head audit_filter_list[AUDIT
#endif
};

+DEFINE_MUTEX(audit_filter_mutex);
+
+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_IN_WATCH IN_MOVE|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF
+#define AUDIT_IN_SELF IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT
+
+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)) {
+ WARN_ON(!list_empty(&parent->watches));
+ 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)) {
+ WARN_ON(!list_empty(&watch->rules));
+ /* watches that were never added don't have a parent */
+ if (watch->parent)
+ audit_put_parent(watch->parent);
+ kfree(watch->path);
+ kfree(watch);
+ }
+}
+
static inline void audit_free_rule(struct audit_entry *e)
{
int i;
+
+ /* some rules don't have associated watches */
+ if (e->rule.watch)
+ audit_put_watch(e->rule.watch);
if (e->rule.fields)
for (i = 0; i < e->rule.field_count; i++) {
struct audit_field *f = &e->rule.fields[i];
@@ -60,6 +138,43 @@ static inline void audit_free_rule_rcu(s
audit_free_rule(e);
}

+/* Initialize a parent watch entry. */
+static inline struct audit_parent *audit_init_parent(void)
+{
+ struct audit_parent *parent;
+
+ parent = kzalloc(sizeof(*parent), GFP_KERNEL);
+ if (unlikely(!parent))
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&parent->watches);
+ atomic_set(&parent->count, 1);
+
+ spin_lock(&master_parents_lock);
+ list_add(&parent->mlist, &master_parents);
+ spin_unlock(&master_parents_lock);
+
+ return parent;
+}
+
+/* Initialize a watch entry. */
+static inline struct audit_watch *audit_init_watch(char *path)
+{
+ struct audit_watch *watch;
+
+ watch = kzalloc(sizeof(*watch), GFP_KERNEL);
+ if (unlikely(!watch))
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&watch->rules);
+ atomic_set(&watch->count, 1);
+ watch->path = path;
+ watch->dev = (dev_t)-1;
+ watch->ino = (unsigned long)-1;
+
+ return watch;
+}
+
/* Initialize an audit filterlist entry. */
static inline struct audit_entry *audit_init_entry(u32 field_count)
{
@@ -107,6 +222,28 @@ static char *audit_unpack_string(void **
return str;
}

+/* Translate a watch string to kernel respresentation. */
+static int audit_to_watch(struct audit_krule *krule, char *path, int len,
+ u32 op)
+{
+ struct audit_watch *watch;
+
+ if (path[0] != '/' || path[len-1] == '/' ||
+ krule->listnr != AUDIT_FILTER_EXIT ||
+ op & ~AUDIT_EQUAL ||
+ krule->watch) /* allow only 1 watch per rule */
+ return -EINVAL;
+
+ watch = audit_init_watch(path);
+ if (unlikely(IS_ERR(watch)))
+ return PTR_ERR(watch);
+
+ audit_get_watch(watch);
+ krule->watch = watch;
+
+ return 0;
+}
+
/* Common user-space to kernel rule translation. */
static inline struct audit_entry *audit_to_entry_common(struct audit_rule *rule)
{
@@ -177,7 +314,8 @@ static struct audit_entry *audit_rule_to
f->type == AUDIT_SE_ROLE ||
f->type == AUDIT_SE_TYPE ||
f->type == AUDIT_SE_SEN ||
- f->type == AUDIT_SE_CLR) {
+ f->type == AUDIT_SE_CLR ||
+ f->type == AUDIT_WATCH) {
err = -EINVAL;
goto exit_free;
}
@@ -260,6 +398,18 @@ static struct audit_entry *audit_data_to
} else
f->se_str = str;
break;
+ case AUDIT_WATCH:
+ str = audit_unpack_string(&bufp, &remain, f->val);
+ if (IS_ERR(str))
+ goto exit_free;
+ entry->rule.buflen += f->val;
+
+ err = audit_to_watch(&entry->rule, str, f->val, f->op);
+ if (err) {
+ kfree(str);
+ goto exit_free;
+ }
+ break;
}
}

@@ -343,6 +493,10 @@ static struct audit_rule_data *audit_kru
data->buflen += data->values[i] =
audit_pack_string(&bufp, f->se_str);
break;
+ case AUDIT_WATCH:
+ data->buflen += data->values[i] =
+ audit_pack_string(&bufp, krule->watch->path);
+ break;
default:
data->values[i] = f->val;
}
@@ -378,6 +532,10 @@ static int audit_compare_rule(struct aud
if (strcmp(a->fields[i].se_str, b->fields[i].se_str))
return 1;
break;
+ case AUDIT_WATCH:
+ if (strcmp(a->watch->path, b->watch->path))
+ return 1;
+ break;
default:
if (a->fields[i].val != b->fields[i].val)
return 1;
@@ -391,6 +549,31 @@ static int audit_compare_rule(struct aud
return 0;
}

+/* Duplicate the given audit watch. The new watch's rules list is initialized
+ * to an empty list and wlist is undefined. */
+static inline struct audit_watch *audit_dupe_watch(struct audit_watch *old)
+{
+ char *path;
+ struct audit_watch *new;
+
+ path = kstrdup(old->path, GFP_KERNEL);
+ if (unlikely(!path))
+ return ERR_PTR(-ENOMEM);
+
+ new = audit_init_watch(path);
+ if (unlikely(!new)) {
+ kfree(path);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ new->dev = old->dev;
+ new->ino = old->ino;
+ audit_get_parent(old->parent);
+ new->parent = old->parent;
+
+ return new;
+}
+
/* Duplicate selinux field information. The se_rule is opaque, so must be
* re-initialized. */
static inline int audit_dupe_selinux_field(struct audit_field *df,
@@ -422,8 +605,11 @@ static inline int audit_dupe_selinux_fie
/* Duplicate an audit rule. This will be a deep copy with the exception
* of the watch - that pointer is carried over. The selinux specific fields
* will be updated in the copy. The point is to be able to replace the old
- * rule with the new rule in the filterlist, then free the old rule. */
-static struct audit_entry *audit_dupe_rule(struct audit_krule *old)
+ * rule with the new rule in the filterlist, then free the old rule.
+ * The rlist element is undefined; list manipulations are handled apart from
+ * the initial copy. */
+static struct audit_entry *audit_dupe_rule(struct audit_krule *old,
+ struct audit_watch *watch)
{
u32 fcount = old->field_count;
struct audit_entry *entry;
@@ -442,6 +628,7 @@ static struct audit_entry *audit_dupe_ru
for (i = 0; i < AUDIT_BITMASK_SIZE; i++)
new->mask[i] = old->mask[i];
new->buflen = old->buflen;
+ new->watch = NULL;
new->field_count = old->field_count;
memcpy(new->fields, old->fields, sizeof(struct audit_field) * fcount);

@@ -463,25 +650,302 @@ static struct audit_entry *audit_dupe_ru
}
}

+ if (watch) {
+ audit_get_watch(watch);
+ new->watch = watch;
+ }
+
return entry;
}

-/* Add rule to given filterlist if not a duplicate. Protected by
- * audit_netlink_mutex. */
-static inline int audit_add_rule(struct audit_entry *entry,
+/* Update inode numbers in audit rules based on filesystem event. */
+static inline void audit_update_watch(struct audit_parent *parent,
+ const char *dname, dev_t dev,
+ unsigned long ino)
+{
+ struct audit_watch *owatch, *nwatch, *nextw;
+ struct audit_krule *r, *nextr;
+ struct audit_entry *oentry, *nentry;
+ struct audit_buffer *ab;
+
+ mutex_lock(&audit_filter_mutex);
+ list_for_each_entry_safe(owatch, nextw, &parent->watches, wlist) {
+ if (audit_compare_dname_path(dname, owatch->path))
+ continue;
+
+ nwatch = audit_dupe_watch(owatch);
+ if (unlikely(IS_ERR(nwatch))) {
+ mutex_unlock(&audit_filter_mutex);
+ audit_panic("error updating watch, skipping");
+ return;
+ }
+ nwatch->dev = dev;
+ nwatch->ino = ino;
+
+ list_for_each_entry_safe(r, nextr, &owatch->rules, rlist) {
+ oentry = container_of(r, struct audit_entry, rule);
+
+ nentry = audit_dupe_rule(&oentry->rule, nwatch);
+ if (unlikely(IS_ERR(nentry))) {
+ audit_panic("error updating watch, removing");
+ list_del(&oentry->rule.rlist);
+ list_del_rcu(&oentry->list);
+ } else {
+ list_add(&nentry->rule.rlist, &nwatch->rules);
+ list_del(&oentry->rule.rlist);
+ list_replace_rcu(&oentry->list, &nentry->list);
+ }
+ call_rcu(&oentry->rcu, audit_free_rule_rcu);
+ }
+
+ ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+ audit_log_format(ab, "audit updated rules specifying watch=");
+ audit_log_untrustedstring(ab, owatch->path);
+ audit_log_format(ab, " with dev=%u ino=%lu\n", dev, ino);
+ audit_log_end(ab);
+
+ list_del(&owatch->wlist);
+ audit_put_watch(owatch); /* matches initial get */
+ goto add_watch_to_parent; /* event applies to a single watch */
+ }
+ mutex_unlock(&audit_filter_mutex);
+ return;
+
+add_watch_to_parent:
+ list_add(&nwatch->wlist, &parent->watches);
+ mutex_unlock(&audit_filter_mutex);
+ return;
+}
+
+/* Remove all watches & rules associated with a parent that is going away. */
+static inline void audit_remove_parent_watches(struct audit_parent *parent)
+{
+ struct audit_watch *w, *nextw;
+ struct audit_krule *r, *nextr;
+ struct audit_entry *e;
+
+ mutex_lock(&audit_filter_mutex);
+ list_for_each_entry_safe(w, nextw, &parent->watches, wlist) {
+ list_for_each_entry_safe(r, nextr, &w->rules, rlist) {
+ e = container_of(r, struct audit_entry, rule);
+ list_del(&r->rlist);
+ list_del_rcu(&e->list);
+ call_rcu(&e->rcu, audit_free_rule_rcu);
+
+ audit_log(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE,
+ "audit implicitly removed rule from list=%d\n",
+ AUDIT_FILTER_EXIT);
+ }
+ list_del(&w->wlist);
+ audit_put_watch(w); /* matches initial get */
+ }
+ mutex_unlock(&audit_filter_mutex);
+}
+
+/* Actually remove the parent; inotify has acknowleged the removal. */
+static inline void audit_remove_parent(struct audit_parent *parent)
+{
+ WARN_ON(!list_empty(&parent->watches));
+ spin_lock(&master_parents_lock);
+ list_del(&parent->mlist);
+ audit_put_parent(parent);
+ spin_unlock(&master_parents_lock);
+}
+
+/* Register inotify watches for parents on in_list. */
+static int audit_inotify_register(struct nameidata *nd,
+ struct list_head *in_list)
+{
+ struct audit_parent *p;
+ s32 wd;
+ int ret = 0;
+
+ list_for_each_entry(p, in_list, ilist) {
+ /* Grab a ref while calling inotify_add_watch(), so parent
+ * can't be removed until we've updated its data. */
+ audit_get_parent(p);
+
+ if (!audit_idev)
+ wd = -EOPNOTSUPP;
+ else
+ wd = inotify_add_watch(audit_idev, nd->dentry->d_inode,
+ AUDIT_IN_WATCH, p);
+ if (wd < 0) {
+ audit_remove_parent_watches(p);
+ audit_remove_parent(p);
+ /* save the first error for return value */
+ if (!ret)
+ ret = wd;
+ } else {
+ struct inode *inode = nd->dentry->d_inode;
+
+ mutex_lock(&audit_filter_mutex);
+ p->wd = wd;
+ p->dev = inode->i_sb->s_dev;
+ p->ino = inode->i_ino;
+ mutex_unlock(&audit_filter_mutex);
+ }
+
+ audit_put_parent(p);
+ }
+
+ return ret;
+}
+
+/* Unregister inotify watches for parents on in_list.
+ * Generates an IN_IGNORED event. */
+static void audit_inotify_unregister(struct list_head *in_list)
+{
+ struct audit_parent *p;
+
+ list_for_each_entry(p, in_list, ilist) {
+ if (audit_idev)
+ inotify_ignore(audit_idev, p->wd);
+ /* matches get in audit_remove_watch() */
+ audit_put_parent(p);
+ }
+}
+
+/* Get path information necessary for adding watches. */
+static int audit_get_nd(char *path, struct nameidata **ndp,
+ struct nameidata **ndw)
+{
+ struct nameidata *ndparent, *ndwatch;
+ int err;
+
+ ndparent = kmalloc(sizeof(*ndparent), GFP_KERNEL);
+ if (unlikely(!ndparent))
+ return -ENOMEM;
+
+ ndwatch = kmalloc(sizeof(*ndwatch), GFP_KERNEL);
+ if (unlikely(!ndwatch)) {
+ kfree(ndparent);
+ return -ENOMEM;
+ }
+
+ err = path_lookup(path, LOOKUP_PARENT, ndparent);
+ if (err) {
+ kfree(ndparent);
+ kfree(ndwatch);
+ return err;
+ }
+
+ err = path_lookup(path, 0, ndwatch);
+ if (err) {
+ kfree(ndwatch);
+ ndwatch = NULL;
+ }
+
+ *ndp = ndparent;
+ *ndw = ndwatch;
+
+ return 0;
+}
+
+/* Release resources used for watch path information. */
+static inline void audit_put_nd(struct nameidata *ndp, struct nameidata *ndw)
+{
+ if (ndp) {
+ path_release(ndp);
+ kfree(ndp);
+ }
+ if (ndw) {
+ path_release(ndw);
+ kfree(ndw);
+ }
+}
+
+/* Find an existing parent entry for this watch, or create a new one.
+ * Caller must hold audit_filter_mutex. */
+static inline struct audit_parent *audit_find_parent(struct nameidata *nd,
+ struct list_head *in_list)
+{
+ struct audit_parent *p, *parent, *next;
+ struct inode *inode = nd->dentry->d_inode;
+
+ list_for_each_entry_safe(p, next, &master_parents, mlist) {
+ if (p->ino != inode->i_ino ||
+ p->dev != inode->i_sb->s_dev)
+ continue;
+
+ parent = p;
+ goto out;
+ }
+
+ parent = audit_init_parent();
+ if (IS_ERR(parent))
+ goto out;
+ /* add new parent to inotify registration list */
+ list_add(&parent->ilist, in_list);
+
+out:
+ return parent;
+}
+
+/* Find a matching watch entry, or add this one.
+ * Caller must hold audit_filter_mutex. */
+static inline int audit_add_watch(struct audit_krule *krule,
+ struct nameidata *ndp, struct nameidata *ndw,
struct list_head *list)
{
+ struct audit_parent *parent;
+ struct audit_watch *w, *watch = krule->watch;
+
+ parent = audit_find_parent(ndp, list);
+ if (IS_ERR(parent))
+ return PTR_ERR(parent);
+
+ list_for_each_entry(w, &parent->watches, wlist) {
+ if (strcmp(watch->path, w->path))
+ continue;
+
+ audit_put_watch(watch); /* tmp watch, krule's ref */
+ audit_put_watch(watch); /* tmp watch, matches initial get */
+
+ 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:
+ list_add(&krule->rlist, &watch->rules);
+
+ if (ndw) {
+ watch->dev = ndw->dentry->d_inode->i_sb->s_dev;
+ watch->ino = ndw->dentry->d_inode->i_ino;
+ }
+
+ return 0;
+}
+
+/* Add rule to given filterlist if not a duplicate. */
+static inline int audit_add_rule(struct audit_entry *entry,
+ struct list_head *list)
+{
struct audit_entry *e;
+ struct audit_watch *watch = entry->rule.watch;
+ struct nameidata *ndp, *ndw;
+ LIST_HEAD(inotify_list);
+ int err;
#ifdef CONFIG_AUDITSYSCALL
int dont_count = 0;
#endif

- /* Do not use the _rcu iterator here, since this is the only
- * addition routine. */
+ /* Taking audit_filter_mutex protects from stale rule data and
+ * writes to an audit_parent. */
+ mutex_lock(&audit_filter_mutex);
list_for_each_entry(e, list, list) {
- if (!audit_compare_rule(&entry->rule, &e->rule))
- return -EEXIST;
+ if (!audit_compare_rule(&entry->rule, &e->rule)) {
+ err = -EEXIST;
+ mutex_unlock(&audit_filter_mutex);
+ goto error;
+ }
}
+ mutex_unlock(&audit_filter_mutex);

/* If either of these, don't count towards total */
#ifdef CONFIG_AUDITSYSCALL
@@ -489,43 +953,113 @@ static inline int audit_add_rule(struct
entry->rule.listnr == AUDIT_FILTER_TYPE)
dont_count = 1;
#endif
+ /* Avoid calling path_lookup under audit_filter_mutex. */
+ if (watch) {
+ err = audit_get_nd(watch->path, &ndp, &ndw);
+ if (err)
+ goto error;
+ }
+
+ mutex_lock(&audit_filter_mutex);
+ if (watch) {
+ err = audit_add_watch(&entry->rule, ndp, ndw, &inotify_list);
+ if (err) {
+ mutex_unlock(&audit_filter_mutex);
+ audit_put_nd(ndp, ndw);
+ goto error;
+ }
+ }
if (entry->rule.flags & AUDIT_FILTER_PREPEND) {
list_add_rcu(&entry->list, list);
} else {
list_add_tail_rcu(&entry->list, list);
}
+ mutex_unlock(&audit_filter_mutex);
+
#ifdef CONFIG_AUDITSYSCALL
if (!dont_count)
audit_n_rules++;
#endif

+ if (watch) {
+ err = audit_inotify_register(ndp, &inotify_list);
+ if (err)
+ goto error;
+ audit_put_nd(ndp, ndw);
+ }
+
return 0;
+
+error:
+ if (watch)
+ audit_put_watch(watch); /* tmp watch, matches initial get */
+ return err;
}

-/* Remove an existing rule from filterlist. Protected by
- * audit_netlink_mutex. */
+/* Remove given krule from its associated watch's rules list and clean up any
+ * last instances of associated watch and parent.
+ * Caller must hold audit_filter_mutex. */
+static inline void audit_remove_watch(struct audit_krule *krule,
+ struct list_head *in_list)
+{
+ struct audit_watch *watch = krule->watch;
+ struct audit_parent *parent = watch->parent;
+
+ list_del(&krule->rlist);
+ if (list_empty(&watch->rules)) {
+ list_del(&watch->wlist);
+ audit_put_watch(watch); /* matches initial get */
+
+ if (list_empty(&parent->watches)) {
+ /* Put parent on the inotify un-registration list.
+ * Grab a reference before releasing audit_filter_mutex,
+ * to be released in audit_inotify_unregister(). */
+ list_add(&parent->ilist, in_list);
+ audit_get_parent(parent);
+ }
+ }
+}
+
+/* Remove an existing rule from filterlist. */
static inline int audit_del_rule(struct audit_entry *entry,
struct list_head *list)
{
struct audit_entry *e;
+ LIST_HEAD(inotify_list);

- /* Do not use the _rcu iterator here, since this is the only
- * deletion routine. */
+ mutex_lock(&audit_filter_mutex);
list_for_each_entry(e, list, list) {
- if (!audit_compare_rule(&entry->rule, &e->rule)) {
- list_del_rcu(&e->list);
+ if (audit_compare_rule(&entry->rule, &e->rule))
+ continue;
+
+ if (e->rule.watch) {
+ audit_remove_watch(&e->rule, &inotify_list);
+ /* match initial get for tmp watch */
+ audit_put_watch(entry->rule.watch);
+ }
+
+ list_del_rcu(&e->list);
#ifdef CONFIG_AUDITSYSCALL
- if (entry->rule.listnr == AUDIT_FILTER_USER ||
- entry->rule.listnr == AUDIT_FILTER_TYPE)
- audit_n_rules++;
+ if (entry->rule.listnr == AUDIT_FILTER_USER ||
+ entry->rule.listnr == AUDIT_FILTER_TYPE)
+ audit_n_rules++;
#endif
- call_rcu(&e->rcu, audit_free_rule_rcu);
+ call_rcu(&e->rcu, audit_free_rule_rcu);
#ifdef CONFIG_AUDITSYSCALL
- audit_n_rules--;
+ audit_n_rules--;
#endif
- return 0;
- }
+ mutex_unlock(&audit_filter_mutex);
+ audit_n_rules--;
+
+ if (e->rule.watch)
+ audit_inotify_unregister(&inotify_list);
+
+ return 0;
}
+ mutex_unlock(&audit_filter_mutex);
+ /* match initial get for tmp watch */
+ if (entry->rule.watch)
+ audit_put_watch(entry->rule.watch);
return -ENOENT; /* No matching rule */
}

@@ -542,10 +1076,10 @@ static int audit_list(void *_dest)
seq = dest[1];
kfree(dest);

- mutex_lock(&audit_netlink_mutex);
+ mutex_lock(&audit_filter_mutex);

- /* The *_rcu iterators not needed here because we are
- always called with audit_netlink_mutex held. */
+ /* This is a blocking read, so use audit_filter_mutex instead of rcu
+ * iterator to sync with list writers. */
for (i=0; i<AUDIT_NR_FILTERS; i++) {
list_for_each_entry(entry, &audit_filter_list[i], list) {
struct audit_rule *rule;
@@ -560,7 +1094,7 @@ static int audit_list(void *_dest)
}
audit_send_reply(pid, seq, AUDIT_LIST, 1, 1, NULL, 0);

- mutex_unlock(&audit_netlink_mutex);
+ mutex_unlock(&audit_filter_mutex);
return 0;
}

@@ -576,10 +1110,10 @@ static int audit_list_rules(void *_dest)
seq = dest[1];
kfree(dest);

- mutex_lock(&audit_netlink_mutex);
+ mutex_lock(&audit_filter_mutex);

- /* The *_rcu iterators not needed here because we are
- always called with audit_netlink_mutex held. */
+ /* This is a blocking read, so use audit_filter_mutex instead of rcu
+ * iterator to sync with list writers. */
for (i=0; i<AUDIT_NR_FILTERS; i++) {
list_for_each_entry(e, &audit_filter_list[i], list) {
struct audit_rule_data *data;
@@ -588,13 +1122,13 @@ static int audit_list_rules(void *_dest)
if (unlikely(!data))
break;
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 0, 1,
- data, sizeof(*data));
+ data, sizeof(*data) + data->buflen);
kfree(data);
}
}
audit_send_reply(pid, seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);

- mutex_unlock(&audit_netlink_mutex);
+ mutex_unlock(&audit_filter_mutex);
return 0;
}

@@ -683,6 +1217,32 @@ int audit_receive_filter(int type, int p
return err;
}

+/**
+ * audit_handle_ievent - handler for Inotify events
+ * @event: information about the event
+ * @dname: dentry name associated with event
+ * @inode: inode associated with event
+ * @ptr: kernel's version of a watch descriptor
+ */
+void audit_handle_ievent(struct inotify_event *event, const char *dname,
+ struct inode *inode, void *ptr)
+{
+ struct audit_parent *parent = (struct audit_parent *)ptr;
+
+ if (event->mask & (IN_CREATE|IN_MOVED_TO) && inode)
+ audit_update_watch(parent, dname, inode->i_sb->s_dev,
+ inode->i_ino);
+ else if (event->mask & (IN_DELETE|IN_MOVED_FROM))
+ audit_update_watch(parent, dname, (dev_t)-1, (unsigned long)-1);
+ /* Note: Inotify doesn't remove the watch for the IN_MOVE_SELF event.
+ * Work around this by leaving the parent around with an empty
+ * watchlist. It will be re-used if new watches are added. */
+ else if (event->mask & (AUDIT_IN_SELF))
+ audit_remove_parent_watches(parent);
+ else if (event->mask & IN_IGNORED)
+ audit_remove_parent(parent);
+}
+
int audit_comparator(const u32 left, const u32 op, const u32 right)
{
switch (op) {
@@ -703,7 +1263,39 @@ int audit_comparator(const u32 left, con
return 0;
}

+/* Compare given dentry name with last component in given path,
+ * return of 0 indicates a match. */
+int audit_compare_dname_path(const char *dname, const char *path)
+{
+ int dlen, plen;
+ const char *p;
+
+ if (!dname || !path)
+ return 1;
+
+ dlen = strlen(dname);
+ plen = strlen(path);
+ if (plen < dlen)
+ return 1;
+
+ /* disregard trailing slashes */
+ p = path + plen - 1;
+ while ((*p == '/') && (p > path))
+ p--;
+
+ /* find last path component */
+ p = p - dlen + 1;
+ if (p < path)
+ return 1;
+ else if (p > path) {
+ if (*--p != '/')
+ return 1;
+ else
+ p++;
+ }

+ return strncmp(p, dname, dlen);
+}

static int audit_filter_user_rules(struct netlink_skb_parms *cb,
struct audit_krule *rule,
@@ -817,32 +1409,41 @@ static inline int audit_rule_has_selinux
int selinux_audit_rule_update(void)
{
struct audit_entry *entry, *nentry;
+ struct audit_watch *watch;
int i, err = 0;

- /* audit_netlink_mutex synchronizes the writers */
- mutex_lock(&audit_netlink_mutex);
+ /* audit_filter_mutex synchronizes the writers */
+ mutex_lock(&audit_filter_mutex);

for (i = 0; i < AUDIT_NR_FILTERS; i++) {
list_for_each_entry(entry, &audit_filter_list[i], list) {
if (!audit_rule_has_selinux(&entry->rule))
continue;

- nentry = audit_dupe_rule(&entry->rule);
+ watch = entry->rule.watch;
+ nentry = audit_dupe_rule(&entry->rule, watch);
if (unlikely(IS_ERR(nentry))) {
/* save the first error encountered for the
* return value */
if (!err)
err = PTR_ERR(nentry);
audit_panic("error updating selinux filters");
+ if (watch)
+ list_del(&entry->rule.rlist);
list_del_rcu(&entry->list);
} else {
+ if (watch) {
+ list_add(&nentry->rule.rlist,
+ &watch->rules);
+ list_del(&entry->rule.rlist);
+ }
list_replace_rcu(&entry->list, &nentry->list);
}
call_rcu(&entry->rcu, audit_free_rule_rcu);
}
}

- mutex_unlock(&audit_netlink_mutex);
+ mutex_unlock(&audit_filter_mutex);

return err;
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 3aea29b..64f8489 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -166,6 +166,27 @@ struct audit_context {
#endif
};

+/* Determine if any context name data matches a rule's watch data */
+static inline int audit_match_watch(struct audit_context *ctx,
+ struct audit_watch *watch)
+{
+ int i;
+
+ if (!ctx)
+ return 0;
+
+ if (watch->ino == (unsigned long)-1)
+ return 0;
+
+ for (i = 0; i < ctx->name_count; i++) {
+ if (ctx->names[i].dev == watch->dev &&
+ (ctx->names[i].ino == watch->ino ||
+ ctx->names[i].pino == watch->ino))
+ return 1;
+ }
+
+ return 0;
+}

/* Compare a task_struct with an audit_rule. Return 1 on match, 0
* otherwise. */
@@ -262,6 +283,9 @@ static int audit_filter_rules(struct tas
}
}
break;
+ case AUDIT_WATCH:
+ result = audit_match_watch(ctx, rule->watch);
+ break;
case AUDIT_LOGINUID:
result = 0;
if (ctx)
@@ -1092,37 +1116,20 @@ void __audit_inode_child(const char *dna
return;

/* determine matching parent */
- if (dname)
- for (idx = 0; idx < context->name_count; idx++)
- if (context->names[idx].pino == pino) {
- const char *n;
- const char *name = context->names[idx].name;
- int dlen = strlen(dname);
- int nlen = name ? strlen(name) : 0;
-
- if (nlen < dlen)
- continue;
-
- /* disregard trailing slashes */
- n = name + nlen - 1;
- while ((*n == '/') && (n > name))
- n--;
-
- /* find last path component */
- n = n - dlen + 1;
- if (n < name)
- continue;
- else if (n > name) {
- if (*--n != '/')
- continue;
- else
- n++;
- }
+ if (!dname)
+ goto no_match;
+ for (idx = 0; idx < context->name_count; idx++)
+ if (context->names[idx].pino == pino) {
+ const char *name = context->names[idx].name;

- if (strncmp(n, dname, dlen) == 0)
- goto update_context;
- }
+ if (!name)
+ continue;
+
+ if (audit_compare_dname_path(dname, name) == 0)
+ goto update_context;
+ }

+no_match:
/* catch-all in case match not found */
idx = context->name_count++;
context->names[idx].name = NULL;

Continue reading on narkive:
Loading...