[PATCH] audit: missing variable declaration/initialization when AUDIT_DEBUG == 2.

[Peter Moody]

On Thu, Jul 26, 2012 at 5:34 AM, Jeff Layton <jlayton at redhat.com> wrote:
> On Wed, 18 Jul 2012 14:30:41 -0700
> Peter Moody <pmoody at google.com> wrote:
>
>> Additionally it looks like audit_free_names might return too early when
>> AUDIT_DEBUG was set to 2.
>>
>> Signed-off-by: Peter Moody <pmoody at google.com>
>> ---
>>  kernel/auditsc.c |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> index 4b96415..0c1db46 100644
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@ -997,6 +997,7 @@ static inline void audit_free_names(struct audit_context *context)
>>
>>  #if AUDIT_DEBUG == 2
>>       if (context->put_count + context->ino_count != context->name_count) {
>> +             int i = 0;
>>               printk(KERN_ERR "%s:%d(:%d): major=%d in_syscall=%d"
>>                      " name_count=%d put_count=%d"
>>                      " ino_count=%d [NOT freeing]\n",
>> @@ -1005,11 +1006,10 @@ static inline void audit_free_names(struct audit_context *context)
>>                      context->name_count, context->put_count,
>>                      context->ino_count);
>>               list_for_each_entry(n, &context->names_list, list) {
>> -                     printk(KERN_ERR "names[%d] = %p = %s\n", i,
>> +                     printk(KERN_ERR "names[%d] = %p = %s\n", i++,
>>                              n->name, n->name ?: "(null)");
>>               }
>>               dump_stack();
>> -             return;
>>       }
>
> I'm not certain what the intent of this code was, but if you remove the
> "return" above, then the printk above it that says "[NOT FREEING]". Will
> no longer be valid.

Oh, good point. I was going from what I presumed the intent to be from
the comment from above __audit_syscall_exit

/**
 * Tear down after system call.  If the audit context has been marked as
 * auditable (either because of the AUDIT_RECORD_CONTEXT state from
 * filtering, or because some other part of the kernel wrote an audit
 * message), then write out the syscall information.  In call cases,
 * free the names stored from getname().
 */

(and I am assuming that 'in call cases' is a typo for 'in all cases')

The other thing is that my testing indicated that my box hung if
audit_free_names returned right there.

I need to wait for Eric anyway; hopefully he'll be able to shed some light.

Cheers,
peter

>>  #endif
>>  #if AUDIT_DEBUG
>> @@ -2084,10 +2084,10 @@ void audit_putname(const char *name)
>>                      __FILE__, __LINE__, context->serial, name);
>>               if (context->name_count) {
>>                       struct audit_names *n;
>> -                     int i;
>> +                     int i = 0;
>>
>>                       list_for_each_entry(n, &context->names_list, list)
>> -                             printk(KERN_ERR "name[%d] = %p = %s\n", i,
>> +                             printk(KERN_ERR "name[%d] = %p = %s\n", i++,
>>                                      n->name, n->name ?: "(null)");
>>                       }
>>  #endif



-- 
Peter Moody      Google    1.650.253.7306
Security Engineer  pgp:0xC3410038