[PATCH] filesystem location based auditing
Timothy R. Chavez
tinytim at us.ibm.com
Fri Feb 24 21:07:30 UTC 2006
On Fri, 2006-02-24 at 15:19 -0500, Amy Griffis wrote:
> 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:
<snip>
> Regards,
> Amy
>
Hi Amy,
Just took a cursory glance and noted a few little things.
-tim
<snip>
> +/* 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?
[..]
> +
> +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 :/
[..]
> +
> +/* 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;
Hm... on error you return 0? That's what you return on success too.
[..]
> +}
> +
> +/* 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?
[..]
> + 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?
[..]
> + }
> +
> + 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?
[..]
> + 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);
> +
I guess I should look over this more closely... not getting the use of 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 */
Why not path_lookup with LOOKUP_PARENT, then d_lookup?
> + 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;
>
>
More information about the Linux-audit
mailing list