Date: Fri, 3 Mar 2017 16:14:54 -0500
From: Richard Guy Briggs <rgb@redhat.com>
To: Steve Grubb <sgrubb@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Linux-Audit Mailing List <linux-audit@redhat.com>,
        Al Viro <viro@zeniv.linux.org.uk>, Ingo Molnar <mingo@redhat.com>,
        Jessica Yu <jeyu@redhat.com>
Subject: Re: Hundreds of null PATH records for *init_module syscall audit logs
Message-ID: <20170303211454.GK3818@madcap2.tricolour.ca>
References: <20170301031549.GT18258@madcap2.tricolour.ca>
 <20170301033704.GU18258@madcap2.tricolour.ca>
 <2137861.7RBAWtfTXJ@x2>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2137861.7RBAWtfTXJ@x2>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4458
Lines: 98

On 2017-02-28 23:15, Steve Grubb wrote:
> On Tuesday, February 28, 2017 10:37:04 PM EST Richard Guy Briggs 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 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.
> 
> 0.5 - Notice that we are in *init_module & delete_module and inhibit 
> generation of any record type except SYSCALL and KERN_MODULE ? There are some 
> classification routines for -F perms=wrxa that might be used to create a new 
> class for loading/deleting modules that sets a flag that we use to suppress 
> some record types.

Ok, I was partially able to do this.

If I try and catch it in audit_log_start() which is the common point for
all the record types to be able to limit to just SYSCALL and
KERN_MODULE, there will already be a linked list of hundreds to
thousands of audit_names and will still print a non-zero items count in
the SYSCALL record.  This also sounds like a potentially lazy way to
deal with other record spam (like setuid BRPM_FCAPS).

If I catch it in __audit_inode_child in the same place as I caught the
filesystem type, it is effective for only the PATH record, which is all
that is a problem at the moment.

It touches nine arch-related files, which is a lot more disruptive than
I was hoping.

> > > 1 - In __audit_inode_child, return immedialy upon detecting TRACEFS and
> > > 
> > >     DEBUGFS (and potentially other filesystems identified, via s_magic).
> 
> XFS creates them too. Who knows what else.

Why would this happen?  I would assume it is a mounted filesystem.  Do
you have a sample of the extra records?

This brings me back to the original reaction I had to your suggestion
which is: Are you certain there is never a circumstance where *_module
syscalls never involve a file?  Say, the module itself on loading pulls
in other files from the mounted filesystem?

> -Steve
> 
> > > 2 - In __audit_inode_child, return after not finding the parent in that
> > > 
> > >     task context's audit names_list.
> > > 
> > > 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.
> > > 
> > > 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)
> > >
> > > 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@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635