From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Grubb Subject: Re: [PATCH 4/4] match audit name data Date: Sat, 17 Mar 2007 19:02:31 -0400 Message-ID: <200703171902.32088.sgrubb@redhat.com> References: <20070213191308.GA7536@fc.hp.com> <20070213191522.GE7536@fc.hp.com> <20070214180803.GA17068@fc.hp.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20070214180803.GA17068@fc.hp.com> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: linux-audit@redhat.com List-Id: linux-audit@redhat.com On Wednesday 14 February 2007 13:08:03 Amy Griffis wrote: > Reposting with fixes for audit_inc_name_count() return value and > better conditional context->name_count >= AUDIT_NAMES. Thanks Eric P. Something is wrong with this patch as its causing slab corruption in the lspp 65 and later kernels. I'll try to figure out what's wrong... -Steve > 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 > --- > 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; > + } > } > > /**