Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1589967 > unrolled thread

Hundreds of null PATH records for *init_module syscall audit logs

Back to article view | Back to linux.kernel

Contents

  Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-01 04:30 +0100
    [PATCH ALT2] audit: don't create PATH records for anonymous parents and their children Richard Guy Briggs <rgb@redhat.com> - 2017-03-01 04:30 +0100
    Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-01 04:50 +0100
      Re: Hundreds of null PATH records for *init_module syscall audit logs Steve Grubb <sgrubb@redhat.com> - 2017-03-01 05:20 +0100
        Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-03 22:30 +0100
          [PATCH ALT5] audit: ignore module syscalls on inode child Richard Guy Briggs <rgb@redhat.com> - 2017-03-03 23:30 +0100
          Re: Hundreds of null PATH records for *init_module syscall audit logs Paul Moore <paul@paul-moore.com> - 2017-03-04 01:30 +0100
            Re: Hundreds of null PATH records for *init_module syscall audit logs Jessica Yu <jeyu@redhat.com> - 2017-03-06 23:40 +0100
              Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-07 04:50 +0100
            Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-07 03:10 +0100
              Re: Hundreds of null PATH records for *init_module syscall audit logs Steve Grubb <sgrubb@redhat.com> - 2017-03-09 14:30 +0100
          Re: Hundreds of null PATH records for *init_module syscall audit logs Steve Grubb <sgrubb@redhat.com> - 2017-03-09 14:30 +0100
      Re: Hundreds of null PATH records for *init_module syscall audit logs Paul Moore <paul@paul-moore.com> - 2017-03-04 01:30 +0100
        Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-07 05:00 +0100
          Re: Hundreds of null PATH records for *init_module syscall audit  logs Steven Rostedt <rostedt@goodmis.org> - 2017-03-07 17:00 +0100
            Re: Hundreds of null PATH records for *init_module syscall audit  logs Steven Rostedt <rostedt@goodmis.org> - 2017-03-07 17:30 +0100
              Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-07 18:50 +0100
                Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-07 19:40 +0100
                  Re: Hundreds of null PATH records for *init_module syscall audit  logs Steven Rostedt <rostedt@goodmis.org> - 2017-03-07 20:20 +0100
                    Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-07 23:30 +0100
                Re: Hundreds of null PATH records for *init_module syscall audit  logs Steven Rostedt <rostedt@goodmis.org> - 2017-03-07 21:50 +0100
            Re: Hundreds of null PATH records for *init_module syscall audit logs Richard Guy Briggs <rgb@redhat.com> - 2017-03-07 17:30 +0100
              Re: Hundreds of null PATH records for *init_module syscall audit logs Steve Grubb <sgrubb@redhat.com> - 2017-03-09 14:40 +0100
        Re: Hundreds of null PATH records for *init_module syscall audit  logs Steven Rostedt <rostedt@goodmis.org> - 2017-03-07 16:40 +0100
    [PATCH ALT4] audit: show fstype:pathname for entries with anonymous parents Richard Guy Briggs <rgb@redhat.com> - 2017-03-01 08:50 +0100
      Re: [PATCH ALT4] audit: show fstype:pathname for entries with  anonymous parents kbuild test robot <lkp@intel.com> - 2017-03-02 14:10 +0100

Page 2 of 2 — ← Prev page 1 [2]

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

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

[toc] | [prev] | [next] | [standalone]

#1594406

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

[toc] | [prev] | [next] | [standalone]

#1596070

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web