On Tue, Apr 4, 2017 at 5:21 AM, Richard Guy Briggs <rgb(a)redhat.com> wrote:
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 happens because the parent inode is not found in the task's
audit_names list and hence treats it as anonymous. This gives us no
information other than a numerical device number that may no longer be
visible upon log inspeciton, and an inode number.
Fill in the filesystem type, filesystem magic number and full pathname
from the filesystem mount point on previously null PATH records from
entries that have an anonymous parent from the child dentry using
dentry_path_raw().
Make the dentry argument of __audit_inode_child() non-const so that we
can take a reference to it in the case of an anonymous parent with
dget() and dget_parent() to be able to later print a partial path from
the host filesystem rather than null.
Since all we are given is an inode of the parent and the dentry of the
child, finding the path from the mount point to the root of the
filesystem is more challenging that would involve searching all
vfsmounts from "/" until a matching dentry is found for that
filesystem's root dentry. Even if one is found, there may be more than
one mount point. At this point the gain seems marginal since
knowing the filesystem type and path are a significant help in tracking
down the source of the PATH records and being to address them.
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"
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(a)redhat.com>
---
include/linux/audit.h | 8 ++++----
kernel/audit.c | 16 ++++++++++++++++
kernel/audit.h | 1 +
kernel/auditsc.c | 8 +++++++-
4 files changed, 28 insertions(+), 5 deletions(-)
...
diff --git a/kernel/audit.c b/kernel/audit.c
index 25dd70a..7d83c5a 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;
+ }
Out of curiosity, what terrible things happen if we take a reference
to a non-NULL dentry passed to audit_copy_inode() and store it in
name->dentry? Does performance tank?
Also out of curiosity, why do we want to drop a dentry reference here
if one already exists?
audit_copy_fcaps(name, dentry);
}
@@ -1925,6 +1930,17 @@ 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;
I'm wondering if there is some value in still emitting the record if
the kmalloc() fails, just with the name field set as the unset "?"
value, e.g. "name=?". Thoughts?
--
paul moore
www.paul-moore.com