Re: [PATCH 4/4] match audit name data

Reposting with fixes for audit_inc_name_count() return value and better conditional context->name_count >= AUDIT_NAMES. Thanks Eric P.

Make more effort to detect previously collected names, so we don't log multiple PATH records for a single filesystem object. Add audit_inc_name_count() to reduce duplicate code. Signed-off-by: Amy Griffis <amy.griffis(a)hp.com&gt; --- kernel/auditsc.c | 140 +++++++++++++++++++++++++++++++----------------------- 1 files changed, 81 insertions(+), 59 deletions(-) diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 6f9c14e..1b427d9 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -78,11 +78,6 @@ extern int audit_enabled; * for saving names from getname(). */ #define AUDIT_NAMES 20 -/* AUDIT_NAMES_RESERVED is the number of slots we reserve in the - * audit_context from being used for nameless inodes from - * path_lookup. */ -#define AUDIT_NAMES_RESERVED 7 - /* Indicates that audit should log the full pathname. */ #define AUDIT_NAME_FULL -1 @@ -1282,6 +1277,28 @@ void audit_putname(const char *name) #endif } +static int audit_inc_name_count(struct audit_context *context, + const struct inode *inode) +{ + if (context->name_count >= AUDIT_NAMES) { + if (inode) + printk(KERN_DEBUG "name_count maxed, losing inode data: " + "dev=%02x:%02x, inode=%lu", + MAJOR(inode->i_sb->s_dev), + MINOR(inode->i_sb->s_dev), + inode->i_ino); + + else + printk(KERN_DEBUG "name_count maxed, losing inode data"); + return 1; + } + context->name_count++; +#if AUDIT_DEBUG + context->ino_count++; +#endif + return 0; +} + /* Copy inode data into an audit_names. */ static void audit_copy_inode(struct audit_names *name, const struct inode *inode) { @@ -1319,13 +1336,10 @@ void __audit_inode(const char *name, const struct inode *inode) else { /* FIXME: how much do we care about inodes that have no * associated name? */ - if (context->name_count >= AUDIT_NAMES - AUDIT_NAMES_RESERVED) + if (audit_inc_name_count(context, inode)) return; - idx = context->name_count++; + idx = context->name_count - 1; context->names[idx].name = NULL; -#if AUDIT_DEBUG - ++context->ino_count; -#endif } audit_copy_inode(&context->names[idx], inode); } @@ -1349,69 +1363,77 @@ void __audit_inode_child(const char *dname, const struct inode *inode, { int idx; struct audit_context *context = current->audit_context; - const char *found_name = NULL; + const char *found_parent = NULL, *found_child = NULL; int dirlen = 0; if (!context->in_syscall) return; - /* determine matching parent */ if (!dname) - goto update_context; - for (idx = 0; idx < context->name_count; idx++) - if (context->names[idx].ino == parent->i_ino) { - const char *name = context->names[idx].name; + goto add_names; - if (!name) - continue; + /* parent is more likely, look for it first */ + for (idx = 0; idx < context->name_count; idx++) { + struct audit_names *n = &context->names[idx]; - if (audit_compare_dname_path(dname, name, &dirlen) == 0) { - context->names[idx].name_len = dirlen; - found_name = name; - break; - } + if (!n->name) + continue; + + if ((n->ino == parent->i_ino) && + !audit_compare_dname_path(dname, n->name, &dirlen)) { + n->name_len = dirlen; /* update parent data in place */ + found_parent = n->name; + goto add_names; } + } -update_context: - idx = context->name_count; - if (context->name_count == AUDIT_NAMES) { - printk(KERN_DEBUG "name_count maxed and losing %s\n", - found_name ?: "(null)"); - return; + /* no matching parent, look for matching child */ + for (idx = 0; idx < context->name_count; idx++) { + struct audit_names *n = &context->names[idx]; + + if (!n->name) + continue; + + /* strcmp() is the more likely scenario */ + if (!strcmp(dname, n->name) || + !audit_compare_dname_path(dname, n->name, &dirlen)) { + if (inode) + audit_copy_inode(n, inode); + else + n->ino = (unsigned long)-1; + found_child = n->name; + goto add_names; + } } - context->name_count++; -#if AUDIT_DEBUG - context->ino_count++; -#endif - /* Re-use the name belonging to the slot for a matching parent directory. - * All names for this context are relinquished in audit_free_names() */ - context->names[idx].name = found_name; - context->names[idx].name_len = AUDIT_NAME_FULL; - context->names[idx].name_put = 0; /* don't call __putname() */ - - if (!inode) - context->names[idx].ino = (unsigned long)-1; - else - audit_copy_inode(&context->names[idx], inode); - - /* A parent was not found in audit_names, so copy the inode data for the - * provided parent. */ - if (!found_name) { - idx = context->name_count; - if (context->name_count == AUDIT_NAMES) { - printk(KERN_DEBUG - "name_count maxed and losing parent inode data: dev=%02x:%02x, inode=%lu", - MAJOR(parent->i_sb->s_dev), - MINOR(parent->i_sb->s_dev), - parent->i_ino); + +add_names: + if (found_child || (!found_parent && !found_child)) { + if (audit_inc_name_count(context, parent)) return; - } - context->name_count++; -#if AUDIT_DEBUG - context->ino_count++; -#endif + idx = context->name_count - 1; audit_copy_inode(&context->names[idx], parent); } + + if (found_parent || (!found_parent && !found_child)) { + if (audit_inc_name_count(context, inode)) + return; + idx = context->name_count - 1; + + /* Re-use the name belonging to the slot for a matching parent + * directory. All names for this context are relinquished in + * audit_free_names() */ + if (found_parent) { + context->names[idx].name = found_parent; + context->names[idx].name_len = AUDIT_NAME_FULL; + /* don't call __putname() */ + context->names[idx].name_put = 0; + } + + if (inode) + audit_copy_inode(&context->names[idx], inode); + else + context->names[idx].ino = (unsigned long)-1; + } } /**