Re: [PATCH] name_count array overrun

Amy Griffis
2006-09-27T21:05:31Z

Steve Grubb wrote: [Sun Sep 24 2006, 08:56:57AM EDT]
> On Thursday 07 September 2006 16:43, Amy Griffis wrote:
> > Did you consider just dropping any data encountered after we've filled
> > AUDIT_NAMES, instead of copying over the data for the last element?
>
> OK, corrected patch follows.

Sorry for the delayed response, I've been out of town.

Do we want a similar printk in __audit_inode()?

> The below patch closes an unbounded use of name_count. This can lead to oopses
> in some new file systems.
>
> Signed-off-by: Steve Grubb <sgrubb@redhat.com>
>
>
> diff -urp linux-2.6.18.x86_64.orig/kernel/auditsc.c
> linux-2.6.18.x86_64/kernel/auditsc.c
> --- linux-2.6.18.x86_64.orig/kernel/auditsc.c 2006-09-24
> 08:24:27.000000000 -0400
> +++ linux-2.6.18.x86_64/kernel/auditsc.c 2006-09-24 08:42:01.000000000 -0400
> @@ -1347,7 +1347,13 @@ void __audit_inode_child(const char *dna
> }
>
> update_context:
> - idx = context->name_count++;
> + idx = context->name_count;
> + if (context->name_count == AUDIT_NAMES) {
> + printk(KERN_DEBUG "name_count maxed and losing %s\n",
> + found_name ?: "(null)");
> + return;
> + }
> + context->name_count++;
> #if AUDIT_DEBUG
> context->ino_count++;
> #endif
> @@ -1365,7 +1371,18 @@ update_context:
> /* A parent was not found in audit_names, so copy the inode data for the
> * provided parent. */
> if (!found_name) {
> - idx = context->name_count++;
> + idx = context->name_count;
> + if (context->name_count == AUDIT_NAMES) {
> + printk(KERN_DEBUG
> + "name_count maxed and losing parent inode data: dev=%02x:%02x rdev=%02x:
> %02x, inode=%lu",
> + MAJOR(parent->i_sb->s_dev),
> + MINOR(parent->i_sb->s_dev),
> + MAJOR(parent->i_rdev),
> + MINOR(parent->i_rdev),
> + parent->i_ino);
> + return;
> + }
> + context->name_count++;
> #if AUDIT_DEBUG
> context->ino_count++;
> #endif
>