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 <[EMAIL PROTECTED]>
> ---
> 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;
> + }
> }
>
> /**
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
