Message-ID: <20170307033954.GS18258@madcap2.tricolour.ca>
Date:   Mon, 6 Mar 2017 22:39:54 -0500
From:   Richard Guy Briggs <rgb@...hat.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Linux-Audit Mailing List <linux-audit@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ingo Molnar <mingo@...hat.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: Hundreds of null PATH records for *init_module syscall audit logs

On 2017-03-03 19:19, Paul Moore wrote:
> On Tue, Feb 28, 2017 at 10:37 PM, Richard Guy Briggs <rgb@...hat.com> 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'm at the same level of understanding.

> >> 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?

>From the output I've seen, it doesn't look particularly useful, but it
was useful to finally see the source of those huge numbers of PATH
records.  Here's an fpaste:
	https://paste.fedoraproject.org/paste/UpZoYuokojR0es1ayNdx5l5M1UNdIGYhyRLivL9gydE=/

> >> 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.

Glad we agree there.

> >> 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.

I have another patch that tried to reuse existing entries even if no
struct filename was supplied to __audit_inode, but it didn't seem to
make a difference.  Everything that was working continued to do so and
everything that was broken remained so.

> >> 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.

I did think of supplementing that information with a general
characterization that all the items came from a filesystem that was of
no concern so that there was evidence of what was happenning but that it
wouldn't overwhelm the logs.

> >> 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.

Agreed, but looks like too much useless information.

> >> 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
> 
> paul moore

- RGB

--
Richard Guy Briggs <rgb@...hat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

Powered by blists - more mailing lists