Re: Hundreds of null PATH records for *init_module syscall audit logs

[Paul Moore]

On Tue, Feb 28, 2017 at 10:37 PM, Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> Sorry, I forgot to include Cc: in this cover letter for context to the 4
> alt patches.
>
> On 2017-02-28 22:15, Richard Guy Briggs wrote:
>> The background to this is:
>>       https://github.com/linux-audit/audit-kernel/issues/8
>>
>> In short, audit SYSCALL records for *init_module were occasionally
>> accompanied by hundreds to thousands of null PATH records.
>>
>> I chatted with Al Viro and Eric Paris about this Friday afternoon and
>> they seemed to vaguely recall this issue and didn't have any solid
>> recommendations as to what was the right thing to do (other than the
>> same suggestion from both that I won't print here).
>>
>> It was reproducible on a number of vintages of distributions with
>> default kernels, but triggering on very few of the many modules loaded
>> at boot time.  It was reproduced with fs-nfs4 and nfsv4 modules on
>> tracefs, but there are reports of it also happening with debugfs.  It
>> was triggering only in __audit_inode_child with a parent that was not
>> found in the task context's audit names_list.

I'm no expert on the tracing system, but my understanding is that it
used to use debugfs but now prefers tracefs so perhaps depending on
the vintage of the kernel/userspace you will see it on either debugfs
or tracefs.  I'm also guessing that module load order may have an
effect, maybe not.

>> I have four potential solutions listed in my order of preference and I'd
>> like to get some feedback about which one would be the most acceptable.

>From an audit perspective, I'm generally not a fan of throwing away
information, especially since solution #4 seems to provide some basic
PATH information.  Although I guess the issue is do we care about
tracefs/debugfs PATH records?

>> 1 - In __audit_inode_child, return immedialy upon detecting TRACEFS and
>>     DEBUGFS (and potentially other filesystems identified, via s_magic).

If we decide we want to ignore debugfs/tracefs this may be the best solution.

>> 2 - In __audit_inode_child, return after not finding the parent in that
>>     task context's audit names_list.

This doesn't seem like the right answer.

>> 3 - In __audit_inode_child, mark the parent and its child as "hidden"
>>     when the parent isn't found in that task context's audit names_list.
>>     This will still result in an "items=" count that does not match the
>>     number of accompanying PATH records for that SYSCALL record, which
>>     may upset userspace tools but would still indicate suppressed
>>     records.

Similar to door #2, this doesn't seem right to me.

>> 4 - In __audit_inode_child, when the parent isn't found, store the
>>     child's dentry in the child's (new or not) audit_names structure
>>     (properly refcounted with dget) and store the parent's dentry in its
>>     newly created audit_names structure (via dget_parent), then if the
>>     name isn't available at PATH record generation time, use that stored
>>     value (with dentry_path_raw and released with dput)

This seems most in keeping with the spirit of audit.

>> Is there another more elegant solution that I've missed that catches
>> things before they get anywhere near audit_inode_child (called from
>> tracefs' notifiers)?
>>
>> I'll thread onto this message tested patches for all four solutions.
>>
>> - RGB
>>
>> --
>> Richard Guy Briggs <rgb@xxxxxxxxxx>
>> Kernel Security Engineering, Base Operating Systems, Red Hat
>> Remote, Ottawa, Canada
>> Voice: +1.647.777.2635, Internal: (81) 32635

-- 
paul moore
www.paul-moore.com