Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932631AbdCGDuf (ORCPT ); Mon, 6 Mar 2017 22:50:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57974 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932408AbdCGDu0 (ORCPT ); Mon, 6 Mar 2017 22:50:26 -0500 Date: Mon, 6 Mar 2017 22:39:54 -0500 From: Richard Guy Briggs To: Paul Moore Cc: Linux-Audit Mailing List , LKML , Greg Kroah-Hartman , Ingo Molnar , Steven Rostedt , Al Viro Subject: Re: Hundreds of null PATH records for *init_module syscall audit logs Message-ID: <20170307033954.GS18258@madcap2.tricolour.ca> References: <20170301031549.GT18258@madcap2.tricolour.ca> <20170301033704.GU18258@madcap2.tricolour.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 07 Mar 2017 03:40:03 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4589 Lines: 104 On 2017-03-03 19:19, Paul Moore wrote: > On Tue, Feb 28, 2017 at 10:37 PM, 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'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 Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635