Hundreds of null PATH records for *init_module syscall audit logs

Hundreds of null PATH records for *init_module syscall audit logs

[Richard Guy Briggs]

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.

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

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

[PATCH ALT1] audit: ignore tracefs and debugfs on inode child

[Richard Guy Briggs]

Tracefs or debugfs were causing hundreds to thousands of null PATH records to
be associated with the init_module and finit_module SYSCALL records on a few
modules when the following rule was in place for startup:
	-a always,exit -F arch=x86_64 -S init_module -F key=mod-load

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

See https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditsc.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4db32e8..6050441 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1868,6 +1868,11 @@ void __audit_inode_child(struct inode *parent,
 
 	if (!context->in_syscall)
 		return;
+	switch (parent->i_sb->s_magic) {
+	case TRACEFS_MAGIC:
+	case DEBUGFS_MAGIC:
+		return;
+	}
 
 	if (inode)
 		handle_one(inode);
-- 
1.7.1

[PATCH ALT3] audit: hide PATH records of anonymous parents and their children

[Richard Guy Briggs]

Tracefs or debugfs were causing hundreds to thousands of null PATH records to
be associated with the init_module and finit_module SYSCALL records on a few
modules when the following rule was in place for startup:
        -a always,exit -F arch=x86_64 -S init_module -F key=mod-load

This patch hides those records, but the SYSCALL record "items=" count will
still reflect the number of hidden items.  (This will fail the test below.)

See https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditsc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4db32e8..58ea64e 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1914,6 +1914,7 @@ void __audit_inode_child(struct inode *parent,
 		if (!n)
 			return;
 		audit_copy_inode(n, NULL, parent);
+		n->hidden = true;
 	}
 
 	if (!found_child) {
@@ -1928,6 +1929,8 @@ void __audit_inode_child(struct inode *parent,
 			found_child->name = found_parent->name;
 			found_child->name_len = AUDIT_NAME_FULL;
 			found_child->name->refcnt++;
+		} else {
+			found_child->hidden = true;
 		}
 	}
 
-- 
1.7.1

[PATCH ALT2] audit: don't create PATH records for anonymous parents and their children

[Richard Guy Briggs]

Tracefs or debugfs were causing hundreds to thousands of null PATH records to
be associated with the init_module and finit_module SYSCALL records on a few
modules when the following rule was in place for startup:
        -a always,exit -F arch=x86_64 -S init_module -F key=mod-load

Don't create those records when the parent is not found in
that task context's audit names_list.

See https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditsc.c |   20 +++++++-------------
 1 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4db32e8..83eb3bc 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1889,6 +1889,10 @@ void __audit_inode_child(struct inode *parent,
 		}
 	}
 
+	if (!found_parent)
+		/* Don't track if parent is "anonymous" */
+		return;
+
 	/* is there a matching child entry? */
 	list_for_each_entry(n, &context->names_list, list) {
 		/* can only match entries that have a name */
@@ -1908,14 +1912,6 @@ void __audit_inode_child(struct inode *parent,
 		}
 	}
 
-	if (!found_parent) {
-		/* create a new, "anonymous" parent record */
-		n = audit_alloc_name(context, AUDIT_TYPE_PARENT);
-		if (!n)
-			return;
-		audit_copy_inode(n, NULL, parent);
-	}
-
 	if (!found_child) {
 		found_child = audit_alloc_name(context, type);
 		if (!found_child)
@@ -1924,11 +1920,9 @@ void __audit_inode_child(struct inode *parent,
 		/* Re-use the name belonging to the slot for a matching parent
 		 * directory. All names for this context are relinquished in
 		 * audit_free_names() */
-		if (found_parent) {
-			found_child->name = found_parent->name;
-			found_child->name_len = AUDIT_NAME_FULL;
-			found_child->name->refcnt++;
-		}
+		found_child->name = found_parent->name;
+		found_child->name_len = AUDIT_NAME_FULL;
+		found_child->name->refcnt++;
 	}
 
 	if (inode)
-- 
1.7.1

[PATCH ALT4] audit: show fstype:pathname for entries with anonymous parents

[Richard Guy Briggs]

Tracefs or debugfs were causing hundreds to thousands of null PATH records to
be associated with the init_module and finit_module SYSCALL records on a few
modules when the following rule was in place for startup:
        -a always,exit -F arch=x86_64 -S init_module -F key=mod-load

Fill in the filesystem type, magic and full pathname on previously null PATH
records from entries that have an anonymous parent from the child dentry using
dentry_path_raw.

Sample output:
type=PROCTITLE msg=audit(1488317694.446:143): proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D006E66737634
type=PATH msg=audit(1488317694.446:143): item=797 name=tracefs(74726163):/events/nfs4/nfs4_setclientid/format inode=15969 dev=00:09 mode=0100444 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=CREATE
type=PATH msg=audit(1488317694.446:143): item=796 name=tracefs(74726163):/events/nfs4/nfs4_setclientid inode=15964 dev=00:09 mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=PARENT
...
type=PATH msg=audit(1488317694.446:143): item=1 name=tracefs(74726163):/events/nfs4 inode=15571 dev=00:09 mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=CREATE
type=PATH msg=audit(1488317694.446:143): item=0 name=tracefs(74726163):/events inode=119 dev=00:09 mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=PARENT
type=UNKNOWN[1330] msg=audit(1488317694.446:143): name="nfsv4"
type=SYSCALL msg=audit(1488317694.446:143): arch=c000003e syscall=313 success=yes exit=0 a0=1 a1=55d5a35ce106 a2=0 a3=1 items=798 ppid=6 pid=528 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="modprobe" exe="/usr/bin/kmod" subj=system_u:system_r:insmod_t:s0 key="mod-load"

The test case listed below will need to be modified to check for no null PATH
records.

See: https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c   |   14 ++++++++++++++
 kernel/audit.h   |    1 +
 kernel/auditsc.c |    6 ++++++
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 25dd70a..c144af4 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -66,6 +66,7 @@
 #include <linux/freezer.h>
 #include <linux/pid_namespace.h>
 #include <net/netns/generic.h>
+#include <linux/dcache.h>
 
 #include "audit.h"
 
@@ -1884,6 +1885,10 @@ void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
 	name->gid   = inode->i_gid;
 	name->rdev  = inode->i_rdev;
 	security_inode_getsecid(inode, &name->osid);
+	if (name->dentry) {
+		dput(name->dentry);
+		name->dentry = NULL;
+	}
 	audit_copy_fcaps(name, dentry);
 }
 
@@ -1925,6 +1930,15 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
 			audit_log_n_untrustedstring(ab, n->name->name,
 						    n->name_len);
 		}
+	} else if (n->dentry) {
+		char *fullpath;
+		const char *fullpathp;
+
+		fullpath = kmalloc(PATH_MAX, GFP_KERNEL);
+		if (!fullpath)
+			return;
+		fullpathp = dentry_path_raw(n->dentry, fullpath, PATH_MAX);
+		audit_log_format(ab, " name=%s(%lx):%s", n->dentry->d_sb->s_type->name?:"?", n->dentry->d_sb->s_magic, fullpathp?:"?");
 	} else
 		audit_log_format(ab, " name=(null)");
 
diff --git a/kernel/audit.h b/kernel/audit.h
index 144b7eb..2a11583 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -84,6 +84,7 @@ struct audit_names {
 
 	unsigned long		ino;
 	dev_t			dev;
+	struct dentry		*dentry;
 	umode_t			mode;
 	kuid_t			uid;
 	kgid_t			gid;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4db32e8..a4ec1d8 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -74,6 +74,7 @@
 #include <linux/string.h>
 #include <linux/uaccess.h>
 #include <uapi/linux/limits.h>
+#include <linux/dcache.h>
 
 #include "audit.h"
 
@@ -881,6 +882,8 @@ static inline void audit_free_names(struct audit_context *context)
 		list_del(&n->list);
 		if (n->name)
 			putname(n->name);
+		if (n->dentry)
+			dput(n->dentry);
 		if (n->should_free)
 			kfree(n);
 	}
@@ -1914,6 +1917,7 @@ void __audit_inode_child(struct inode *parent,
 		if (!n)
 			return;
 		audit_copy_inode(n, NULL, parent);
+		n->dentry = dget_parent(dentry);
 	}
 
 	if (!found_child) {
@@ -1935,6 +1939,8 @@ void __audit_inode_child(struct inode *parent,
 		audit_copy_inode(found_child, dentry, inode);
 	else
 		found_child->ino = AUDIT_INO_UNSET;
+	if (!found_parent)
+		found_child->dentry = dget(dentry);
 }
 EXPORT_SYMBOL_GPL(__audit_inode_child);
 
-- 
1.7.1

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

[Richard Guy Briggs]

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.
> 
> 1 - In __audit_inode_child, return immedialy upon detecting TRACEFS and
>     DEBUGFS (and potentially other filesystems identified, via s_magic).
> 
> 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

- 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

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

[Steve Grubb]

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.

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

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

Re: [PATCH ALT4] audit: show fstype:pathname for entries with anonymous parents

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3018 bytes --]

Hi Richard,

[auto build test WARNING on pcmoore-audit/next]
[also build test WARNING on v4.10 next-20170302]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-show-fstype-pathname-for-entries-with-anonymous-parents/20170302-200143
base:   git://git.infradead.org/users/pcmoore/audit next
config: i386-defconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   kernel/auditsc.c: In function '__audit_inode_child':
>> kernel/auditsc.c:1920:27: warning: passing argument 1 of 'dget_parent' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      n->dentry = dget_parent(dentry);
                              ^~~~~~
   In file included from include/linux/fs.h:7:0,
                    from kernel/auditsc.c:50:
   include/linux/dcache.h:322:23: note: expected 'struct dentry *' but argument is of type 'const struct dentry *'
    extern struct dentry *dget_parent(struct dentry *dentry);
                          ^~~~~~~~~~~
>> kernel/auditsc.c:1943:30: warning: passing argument 1 of 'dget' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      found_child->dentry = dget(dentry);
                                 ^~~~~~
   In file included from include/linux/fs.h:7:0,
                    from kernel/auditsc.c:50:
   include/linux/dcache.h:315:30: note: expected 'struct dentry *' but argument is of type 'const struct dentry *'
    static inline struct dentry *dget(struct dentry *dentry)
                                 ^~~~

vim +1920 kernel/auditsc.c

  1914		if (!found_parent) {
  1915			/* create a new, "anonymous" parent record */
  1916			n = audit_alloc_name(context, AUDIT_TYPE_PARENT);
  1917			if (!n)
  1918				return;
  1919			audit_copy_inode(n, NULL, parent);
> 1920			n->dentry = dget_parent(dentry);
  1921		}
  1922	
  1923		if (!found_child) {
  1924			found_child = audit_alloc_name(context, type);
  1925			if (!found_child)
  1926				return;
  1927	
  1928			/* Re-use the name belonging to the slot for a matching parent
  1929			 * directory. All names for this context are relinquished in
  1930			 * audit_free_names() */
  1931			if (found_parent) {
  1932				found_child->name = found_parent->name;
  1933				found_child->name_len = AUDIT_NAME_FULL;
  1934				found_child->name->refcnt++;
  1935			}
  1936		}
  1937	
  1938		if (inode)
  1939			audit_copy_inode(found_child, dentry, inode);
  1940		else
  1941			found_child->ino = AUDIT_INO_UNSET;
  1942		if (!found_parent)
> 1943			found_child->dentry = dget(dentry);
  1944	}
  1945	EXPORT_SYMBOL_GPL(__audit_inode_child);
  1946	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24976 bytes --]

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

[Richard Guy Briggs]

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

[PATCH ALT5] audit: ignore module syscalls on inode child

[Richard Guy Briggs]

Tracefs or debugfs were causing hundreds to thousands of null PATH
records to be associated with the init_module and finit_module SYSCALL
records on a few modules when the following rule was in place for
startup:
	-a always,exit -F arch=x86_64 -S init_module -F key=mod-load

In __audit_inode_child, return immedialy upon detecting module-related
syscalls.

See https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditsc.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4db32e8..d7fe943 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1868,6 +1868,12 @@ void __audit_inode_child(struct inode *parent,
 
 	if (!context->in_syscall)
 		return;
+	switch (context->major) {
+	case __NR_init_module:
+	case __NR_delete_module:
+	case __NR_finit_module:
+		return;
+	}
 
 	if (inode)
 		handle_one(inode);
-- 
1.7.1

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@redhat.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 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@redhat.com>
>> 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

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

[Paul Moore]

On Fri, Mar 3, 2017 at 4:14 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> 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.

Blocking PATH record on creation based on syscall *really* seems like
a bad/dangerous idea.  If we want to block all these tracefs/debugfs
records, let's just block the fs.  Although as of right now I'm not a
fan of blocking anything.

-- 
paul moore
www.paul-moore.com

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

[Richard Guy Briggs]

On 2017-03-03 19:22, Paul Moore wrote:
> On Fri, Mar 3, 2017 at 4:14 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > 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.
> 
> Blocking PATH record on creation based on syscall *really* seems like
> a bad/dangerous idea.  If we want to block all these tracefs/debugfs
> records, let's just block the fs.  Although as of right now I'm not a
> fan of blocking anything.

I agree.  What makes me leery of this approach is if a kernel module in
turn accesses directly other files, or bypasses the load_module call to
load another module from a file and avoids logging.

> paul moore

- 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

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

[Jessica Yu]

+++ Richard Guy Briggs [06/03/17 16:49 -0500]:
>On 2017-03-03 19:22, Paul Moore wrote:
>> On Fri, Mar 3, 2017 at 4:14 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > 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.
>>
>> Blocking PATH record on creation based on syscall *really* seems like
>> a bad/dangerous idea.  If we want to block all these tracefs/debugfs
>> records, let's just block the fs.  Although as of right now I'm not a
>> fan of blocking anything.
>
>I agree.  What makes me leery of this approach is if a kernel module in
>turn accesses directly other files, or bypasses the load_module call to
>load another module from a file and avoids logging.

AFAIK load_module is *the* entry point for module loading, it is where
all the setup occurs in order for a module to be properly set up and
registered in our internal data structures (e.g the global modules
list). If a module wants another module loaded, it can request for it
to be loaded via request_module(), which punts the request to modprobe
in userspace to load the module in question, but I'm not sure if
that's at all related to this null PATH record issue.

Jessica

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

[Richard Guy Briggs]

On 2017-03-03 19:19, Paul Moore wrote:
> On Tue, Feb 28, 2017 at 10:37 PM, Richard Guy Briggs <rgb@redhat.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@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

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

[Richard Guy Briggs]

On 2017-03-06 17:30, Jessica Yu wrote:
> +++ Richard Guy Briggs [06/03/17 16:49 -0500]:
> >On 2017-03-03 19:22, Paul Moore wrote:
> >>On Fri, Mar 3, 2017 at 4:14 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >>> 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.
> >>
> >>Blocking PATH record on creation based on syscall *really* seems like
> >>a bad/dangerous idea.  If we want to block all these tracefs/debugfs
> >>records, let's just block the fs.  Although as of right now I'm not a
> >>fan of blocking anything.
> >
> >I agree.  What makes me leery of this approach is if a kernel module in
> >turn accesses directly other files, or bypasses the load_module call to
> >load another module from a file and avoids logging.
> 
> AFAIK load_module is *the* entry point for module loading, it is where
> all the setup occurs in order for a module to be properly set up and
> registered in our internal data structures (e.g the global modules
> list). If a module wants another module loaded, it can request for it
> to be loaded via request_module(), which punts the request to modprobe
> in userspace to load the module in question, but I'm not sure if
> that's at all related to this null PATH record issue.

Yes, there is a lot going on in that function and by far the easiest way
to be able to load another module, but I'm being a bit paranoid in
suggesting that a rogue module may try and skip some steps listed there
and roll its own, hence the desire to not disable all PATH auxilliary
records for *_module SYSCALL records, but only the filesystem types that
don't pose a threat.

> Jessica

- 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

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

[Steven Rostedt]

On Fri, 3 Mar 2017 19:19:47 -0500
Paul Moore <paul@paul-moore.com> wrote:

> On Tue, Feb 28, 2017 at 10:37 PM, Richard Guy Briggs <rgb@redhat.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.

Note, when you mount debugfs, it automounts tracefs in debugfs/tracing.

Userspace can also mount tracefs without debugfs. But tracing does not
use debugfs anymore, even though it appears in the debugfs directory.

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


I don't have enough context here to really understand what the issue
is. Is there a problem when modules have trace events and when they are
loaded, these trace events create files and directories in the tracefs
file system?

-- Steve

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

[Steven Rostedt]

On Mon, 6 Mar 2017 22:39:54 -0500
Richard Guy Briggs <rgb@redhat.com> wrote:

> >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=/

Those are the files for the module's trace events that are created.

I'm still confused about what the issue is.

-- Steve

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

[Richard Guy Briggs]

On 2017-03-07 10:41, Steven Rostedt wrote:
> On Mon, 6 Mar 2017 22:39:54 -0500
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > >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=/
> 
> Those are the files for the module's trace events that are created.
> 
> I'm still confused about what the issue is.

The issue is the audit subsystem being overwhelmed by potentially
useless information.

The initial report was "there's a bunch of null PATH records, please
make them go away", which was anywhere from 500 to 6000 records.

Once I found out what they generally were and a way to enumerate the
contents, we're reviewing that assessment to see if they really should
be tossed, or listed out in full.

> -- Steve

- 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

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

[Steven Rostedt]

On Tue, 7 Mar 2017 11:00:27 -0500
Richard Guy Briggs <rgb@redhat.com> wrote:

> On 2017-03-07 10:41, Steven Rostedt wrote:
> > On Mon, 6 Mar 2017 22:39:54 -0500
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> >   
> > > >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=/  
> > 
> > Those are the files for the module's trace events that are created.
> > 
> > I'm still confused about what the issue is.  
> 
> The issue is the audit subsystem being overwhelmed by potentially
> useless information.
> 
> The initial report was "there's a bunch of null PATH records, please
> make them go away", which was anywhere from 500 to 6000 records.

I don't know the audit system and exactly what it is looking for. How
does it deal with other virtual filesystems like procfs? Why is tracefs
different?

-- Steve

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

[Richard Guy Briggs]

On 2017-03-07 11:20, Steven Rostedt wrote:
> On Tue, 7 Mar 2017 11:00:27 -0500
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > On 2017-03-07 10:41, Steven Rostedt wrote:
> > > On Mon, 6 Mar 2017 22:39:54 -0500
> > > Richard Guy Briggs <rgb@redhat.com> wrote:
> > >   
> > > > >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=/  
> > > 
> > > Those are the files for the module's trace events that are created.
> > > 
> > > I'm still confused about what the issue is.  
> > 
> > The issue is the audit subsystem being overwhelmed by potentially
> > useless information.
> > 
> > The initial report was "there's a bunch of null PATH records, please
> > make them go away", which was anywhere from 500 to 6000 records.
> 
> I don't know the audit system and exactly what it is looking for. How
> does it deal with other virtual filesystems like procfs? Why is tracefs
> different?

The audit subsystem is looking, via sysadmin-crafted rules to notice
situations of interest, for details that could affect the integrity of
the system

This situation is specifically for syscall auditing.  Various syscalls
have expected numbers of accompanying PATH records, for example open(2)
would have directory and file PATH records, while rename(2) would have 2
directory and 2 file PATH records.  An open call on a file in /proc
could trigger a rule that was formulated to catch that type of activity
that lists its directory and its file PATH records, which would be fine.

We normally don't expect the init_module syscall to have any PATH
records associated with it, so when a few of them had hundreds or more
this was surprising.

If there is a way that debugfs or tracefs could be abused during an
init_module call (or any other syscall for that matter), we want to be
aware of it.  This is why simply ignoring those PATH records is making
two of us nervous.

> -- Steve

- 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

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

[Steven Rostedt]

On Tue, 7 Mar 2017 12:39:55 -0500
Richard Guy Briggs <rgb@redhat.com> wrote:

> We normally don't expect the init_module syscall to have any PATH
> records associated with it, so when a few of them had hundreds or more
> this was surprising.

Hmm, how does the syscall get a path associated to it? Just by its
creation? That is, by calling init_module() which would load a module,
would indeed create a path. Some modules do create their own debugfs
files, which would explain why debugfs is shown too.

> 
> If there is a way that debugfs or tracefs could be abused during an
> init_module call (or any other syscall for that matter), we want to be
> aware of it.  This is why simply ignoring those PATH records is making
> two of us nervous.

If there's a bug in the kernel code, then I'm sure there's probably a
way to abuse it.

I also don't believe it should be ignored, which is why I'm asking
these questions. I want to know what exactly is being looked at, and
what is considered "OK" and what isn't.

Now loading modules can indeed create files and directories. Is this
something that the audit system needs to understand?

-- Steve

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

[Richard Guy Briggs]

On 2017-03-07 13:04, Steven Rostedt wrote:
> On Tue, 7 Mar 2017 12:39:55 -0500
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > We normally don't expect the init_module syscall to have any PATH
> > records associated with it, so when a few of them had hundreds or more
> > this was surprising.
> 
> Hmm, how does the syscall get a path associated to it? Just by its
> creation? That is, by calling init_module() which would load a module,
> would indeed create a path. Some modules do create their own debugfs
> files, which would explain why debugfs is shown too.

My understanding is that a module binary blob is already acquired from
some source and then handed to the init_module (or finit_module) syscall
to add to the running kernel.  Some add functionality in /proc or /sys,
but these would not be exercised until they are called by name from
another syscall (such as open).

Syscall auditing is interested in the resources/details of *one* syscall
event at a time (from audit_syscall_entry to audit_syscall_exit),
logging the subject attributes of a process (who) doing what (which
syscall) to what (frequently a file).  Depending on the syscall, there
could be any number of auxilliary records to that event that help fill
in the whole picture that interests us.

So which file are you talking about that "would indeed create a path"?

> > If there is a way that debugfs or tracefs could be abused during an
> > init_module call (or any other syscall for that matter), we want to be
> > aware of it.  This is why simply ignoring those PATH records is making
> > two of us nervous.
> 
> If there's a bug in the kernel code, then I'm sure there's probably a
> way to abuse it.
> 
> I also don't believe it should be ignored, which is why I'm asking
> these questions. I want to know what exactly is being looked at, and
> what is considered "OK" and what isn't.

That one is harder to answer and depends on the syscall and its
potential to influence system behaviour, or to exfiltrate information.

> Now loading modules can indeed create files and directories. Is this
> something that the audit system needs to understand?

Does it create them immediately in that syscall?  Or does it make that
path available for other operations later?

> -- Steve

- 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

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

[Steven Rostedt]

On Tue, 7 Mar 2017 13:34:47 -0500
Richard Guy Briggs <rgb@redhat.com> wrote:

> On 2017-03-07 13:04, Steven Rostedt wrote:
> > On Tue, 7 Mar 2017 12:39:55 -0500
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> >   
> > > We normally don't expect the init_module syscall to have any PATH
> > > records associated with it, so when a few of them had hundreds or more
> > > this was surprising.  
> > 
> > Hmm, how does the syscall get a path associated to it? Just by its
> > creation? That is, by calling init_module() which would load a module,
> > would indeed create a path. Some modules do create their own debugfs
> > files, which would explain why debugfs is shown too.  
> 
> My understanding is that a module binary blob is already acquired from
> some source and then handed to the init_module (or finit_module) syscall
> to add to the running kernel.  Some add functionality in /proc or /sys,
> but these would not be exercised until they are called by name from
> another syscall (such as open).
> 
> Syscall auditing is interested in the resources/details of *one* syscall
> event at a time (from audit_syscall_entry to audit_syscall_exit),
> logging the subject attributes of a process (who) doing what (which
> syscall) to what (frequently a file).  Depending on the syscall, there
> could be any number of auxilliary records to that event that help fill
> in the whole picture that interests us.
> 
> So which file are you talking about that "would indeed create a path"?

The files in /sys/kernel{/debug}/tracing/events/*

A module may connect "trace events" to parts of its code. When a module
gets loaded, virtual directories and files are created with respect to
the events within the module. When the module is unloaded, those files
are removed.

> 
> > > If there is a way that debugfs or tracefs could be abused during an
> > > init_module call (or any other syscall for that matter), we want to be
> > > aware of it.  This is why simply ignoring those PATH records is making
> > > two of us nervous.  
> > 
> > If there's a bug in the kernel code, then I'm sure there's probably a
> > way to abuse it.
> > 
> > I also don't believe it should be ignored, which is why I'm asking
> > these questions. I want to know what exactly is being looked at, and
> > what is considered "OK" and what isn't.  
> 
> That one is harder to answer and depends on the syscall and its
> potential to influence system behaviour, or to exfiltrate information.
> 
> > Now loading modules can indeed create files and directories. Is this
> > something that the audit system needs to understand?  
> 
> Does it create them immediately in that syscall?  Or does it make that
> path available for other operations later?

Not sure what you mean here. The files are created, but to use them,
another process needs to do an open and write to them.

The inodes and dentrys are created. But the process should not have any
file descriptors created associated with them.

-- Steve

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

[Richard Guy Briggs]

On 2017-03-07 14:09, Steven Rostedt wrote:
> On Tue, 7 Mar 2017 13:34:47 -0500
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > On 2017-03-07 13:04, Steven Rostedt wrote:
> > > On Tue, 7 Mar 2017 12:39:55 -0500
> > > Richard Guy Briggs <rgb@redhat.com> wrote:
> > >   
> > > > We normally don't expect the init_module syscall to have any PATH
> > > > records associated with it, so when a few of them had hundreds or more
> > > > this was surprising.  
> > > 
> > > Hmm, how does the syscall get a path associated to it? Just by its
> > > creation? That is, by calling init_module() which would load a module,
> > > would indeed create a path. Some modules do create their own debugfs
> > > files, which would explain why debugfs is shown too.  
> > 
> > My understanding is that a module binary blob is already acquired from
> > some source and then handed to the init_module (or finit_module) syscall
> > to add to the running kernel.  Some add functionality in /proc or /sys,
> > but these would not be exercised until they are called by name from
> > another syscall (such as open).
> > 
> > Syscall auditing is interested in the resources/details of *one* syscall
> > event at a time (from audit_syscall_entry to audit_syscall_exit),
> > logging the subject attributes of a process (who) doing what (which
> > syscall) to what (frequently a file).  Depending on the syscall, there
> > could be any number of auxilliary records to that event that help fill
> > in the whole picture that interests us.
> > 
> > So which file are you talking about that "would indeed create a path"?
> 
> The files in /sys/kernel{/debug}/tracing/events/*

These appear to be the null PATHs I'm looking for.

So these appear to be the PATH records that are being seen, mounted on
/sys/kernel{/debug}/tracing/, but showing up as anonymous because the
path to the mount point is unknown or unavailable in the audit_names
list at the time of the syscall.

Could that tracefs instance have been populated but not yet mounted at
the time the module was loaded on boot? (fs-nfs4, nfsv4)

> A module may connect "trace events" to parts of its code. When a module
> gets loaded, virtual directories and files are created with respect to
> the events within the module. When the module is unloaded, those files
> are removed.

Can the availability of debug or trace points in a module be forced off
by a user to avoid or limit the problem?

Is the availability of these trace points a build time or run time switch?

Is there any security liability to having those trace points available
in the filesystem in terms of control or information leakage?  (Sounds
like yes.)

> > > > If there is a way that debugfs or tracefs could be abused during an
> > > > init_module call (or any other syscall for that matter), we want to be
> > > > aware of it.  This is why simply ignoring those PATH records is making
> > > > two of us nervous.  
> > > 
> > > If there's a bug in the kernel code, then I'm sure there's probably a
> > > way to abuse it.
> > > 
> > > I also don't believe it should be ignored, which is why I'm asking
> > > these questions. I want to know what exactly is being looked at, and
> > > what is considered "OK" and what isn't.  
> > 
> > That one is harder to answer and depends on the syscall and its
> > potential to influence system behaviour, or to exfiltrate information.
> > 
> > > Now loading modules can indeed create files and directories. Is this
> > > something that the audit system needs to understand?  
> > 
> > Does it create them immediately in that syscall?  Or does it make that
> > path available for other operations later?
> 
> Not sure what you mean here. The files are created, but to use them,
> another process needs to do an open and write to them.

Ok, so I think I've concluded that these are the same files.

So the system is working as intended.  The next question is how do we
address the issue, perhaps by answering the three questions above.

> The inodes and dentrys are created. But the process should not have any
> file descriptors created associated with them.

It doesn't need to.  I assume a module could open a file from within the
kernel to read it or write it and then close it, all within the module's
init routine and be done by the time the syscall finishes.

> -- Steve

- 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

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

[Steve Grubb]

On Friday, March 3, 2017 4:14:54 PM EST Richard Guy Briggs wrote:
> > > > 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?

I can't find them right away. But I've seen them.

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

We don't care about this. Audit events have to tell a story. They must have a 
subject, action, and object. In this case its "somebody loaded a kernel module 
X". Where X is the module name. Paths are irrelevant to the story and just 
make it hard to understand the event.

-Steve

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

[Steve Grubb]

On Monday, March 6, 2017 4:49:21 PM EST Richard Guy Briggs wrote:
> > Blocking PATH record on creation based on syscall *really* seems like
> > a bad/dangerous idea.  If we want to block all these tracefs/debugfs
> > records, let's just block the fs.  Although as of right now I'm not a
> > fan of blocking anything.
> 
> I agree.  What makes me leery of this approach is if a kernel module in
> turn accesses directly other files, or bypasses the load_module call to
> load another module from a file and avoids logging.

In this case, we want a second event with that module name. We do not want any 
PATH records.

-Steve

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

[Steve Grubb]

On Tuesday, March 7, 2017 11:00:27 AM EST Richard Guy Briggs wrote:
> On 2017-03-07 10:41, Steven Rostedt wrote:
> > On Mon, 6 Mar 2017 22:39:54 -0500
> > 
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> > > >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/
UpZoYuokojR0es1ayNdx5l5M1UNdIGYhy
> > > 	RLivL9gydE=/> 
> > Those are the files for the module's trace events that are created.
> > 
> > I'm still confused about what the issue is.
> 
> The issue is the audit subsystem being overwhelmed by potentially
> useless information.
> 
> The initial report was "there's a bunch of null PATH records, please
> make them go away", which was anywhere from 500 to 6000 records.
> 
> Once I found out what they generally were and a way to enumerate the
> contents, we're reviewing that assessment to see if they really should
> be tossed, or listed out in full.

They should be tossed. They do not help in any way. What we need for an 
auxiliary record is simply the module's name. That's all.

-Steve