Discussion:
[PATCH ghak8 ALT4 V4 1/3] audit: show partial pathname for entries with anonymous parents
(too old to reply)
Richard Guy Briggs
2018-02-12 05:02:21 UTC
Permalink
Raw Message
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 for a device that may
no longer be visible upon log inspeciton, and an inode number.

Fill in the partial known pathname from the filesystem mount point to
the leaf node 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 able to address them.

Sample output:
type=PROCTITLE msg=audit(1488317694.446:143): proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D006E66737634
type=PATH msg=audit(1488317694.446:143): item=797 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1488317694.446:143): item=796 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
...
type=PATH msg=audit(1488317694.446:143): item=1 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1488317694.446:143): item=0 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=KERN_MODULE 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 <***@redhat.com>

---
v4:
fix fullpath memleak
switch from log_format() to audit_log_untrustedstring()
remove leading / from pathname relative to unknown mount point

v3:
fix audit_buffer leak and dname error allocation leak audit_log_name
only put audit_name->dentry if it is being replaced

v2:
deconstify struct dentry*
add hex prefix to fstype
---
include/linux/audit.h | 8 ++++----
kernel/audit.c | 28 +++++++++++++++++++++++++++-
kernel/audit.h | 1 +
kernel/auditsc.c | 8 +++++++-
4 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..2020f1d 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -232,7 +232,7 @@ extern void __audit_inode(struct filename *name, const struct dentry *dentry,
unsigned int flags);
extern void __audit_file(const struct file *);
extern void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type);
extern void __audit_seccomp(unsigned long syscall, long signr, int code);
extern void __audit_ptrace(struct task_struct *t);
@@ -297,7 +297,7 @@ static inline void audit_inode_parent_hidden(struct filename *name,
AUDIT_INODE_PARENT | AUDIT_INODE_HIDDEN);
}
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type) {
if (unlikely(!audit_dummy_context()))
__audit_inode_child(parent, dentry, type);
@@ -481,7 +481,7 @@ static inline void __audit_inode(struct filename *name,
unsigned int flags)
{ }
static inline void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_inode(struct filename *name,
@@ -495,7 +495,7 @@ static inline void audit_inode_parent_hidden(struct filename *name,
const struct dentry *dentry)
{ }
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_core_dumps(long signr)
diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99..0c8d5a8 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -72,6 +72,7 @@
#include <linux/freezer.h>
#include <linux/pid_namespace.h>
#include <net/netns/generic.h>
+#include <linux/dcache.h>

#include "audit.h"

@@ -2056,6 +2057,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);
}

@@ -2097,8 +2102,29 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
audit_log_n_untrustedstring(ab, n->name->name,
n->name_len);
}
- } else
+ } else if (n->dentry) {
+ char *fullpath;
+ const char *fullpathp = NULL;
+
+ fullpath = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (fullpath) {
+ fullpathp = dentry_path_raw(n->dentry, fullpath, PATH_MAX);
+ if (IS_ERR(fullpathp)) {
+ fullpathp = NULL;
+ } else {
+ while (*fullpathp == '/')
+ fullpathp++;
+ if (*fullpathp == 0)
+ fullpathp = NULL;
+ }
+ }
+ audit_log_format(ab, " name=");
+ audit_log_untrustedstring(ab, fullpathp ?: "?");
+ if (fullpath)
+ kfree(fullpath);
+ } else {
audit_log_format(ab, " name=(null)");
+ }

if (n->ino != AUDIT_INO_UNSET)
audit_log_format(ab, " inode=%lu"
diff --git a/kernel/audit.h b/kernel/audit.h
index af5bc59..81f6865 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -85,6 +85,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 e80459f..b73ede0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -75,6 +75,7 @@
#include <linux/uaccess.h>
#include <linux/fsnotify_backend.h>
#include <uapi/linux/limits.h>
+#include <linux/dcache.h>

#include "audit.h"

@@ -882,6 +883,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);
}
@@ -1862,7 +1865,7 @@ void __audit_file(const struct file *file)
* unsuccessful attempts.
*/
void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{
struct audit_context *context = current->audit_context;
@@ -1941,6 +1944,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) {
@@ -1962,6 +1966,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.8.3.1
Richard Guy Briggs
2018-02-12 05:02:23 UTC
Permalink
Raw Message
Use new filetypes PARENT_ANON and CREATE_ANON to indicate the pathname
supplied is incomplete and relative to the anonymous parent mountpoint
of type filesystem noted in the fstype field.

Sample output:
type=PATH msg=audit(1514350593.987:136): item=808 name="events/nfs4/nfs4_setclientid" inode=16778 dev=00:0b mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=PARENT_ANON cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=0x74726163
type=PATH msg=audit(1514350593.987:136): item=809 name="events/nfs4/nfs4_setclientid/format" inode=16783 dev=00:0b mode=0100444 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=CREATE_ANON cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=0x74726163

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 <***@redhat.com>
---
include/linux/audit.h | 2 ++
kernel/audit.c | 6 ++++++
kernel/auditsc.c | 6 ++++--
3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 2020f1d..828e451 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -105,6 +105,8 @@ struct audit_field {
#define AUDIT_TYPE_PARENT 2 /* a parent audit record */
#define AUDIT_TYPE_CHILD_DELETE 3 /* a child being deleted */
#define AUDIT_TYPE_CHILD_CREATE 4 /* a child being created */
+#define AUDIT_TYPE_PARENT_ANON 5 /* an anonymous parent audit record */
+#define AUDIT_TYPE_CHILD_ANON 6 /* an anonymous child being created */

/* maximized args number that audit_socketcall can process */
#define AUDITSC_ARGS 6
diff --git a/kernel/audit.c b/kernel/audit.c
index 1c9d0a4..64f0025 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2170,6 +2170,12 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
case AUDIT_TYPE_CHILD_CREATE:
audit_log_format(ab, "CREATE");
break;
+ case AUDIT_TYPE_CHILD_ANON:
+ audit_log_format(ab, "CREATE_ANON");
+ break;
+ case AUDIT_TYPE_PARENT_ANON:
+ audit_log_format(ab, "PARENT_ANON");
+ break;
default:
audit_log_format(ab, "UNKNOWN");
break;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index b73ede0..903595ec 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1940,7 +1940,7 @@ void __audit_inode_child(struct inode *parent,

if (!found_parent) {
/* create a new, "anonymous" parent record */
- n = audit_alloc_name(context, AUDIT_TYPE_PARENT);
+ n = audit_alloc_name(context, AUDIT_TYPE_PARENT_ANON);
if (!n)
return;
audit_copy_inode(n, NULL, parent);
@@ -1966,8 +1966,10 @@ void __audit_inode_child(struct inode *parent,
audit_copy_inode(found_child, dentry, inode);
else
found_child->ino = AUDIT_INO_UNSET;
- if (!found_parent)
+ if (!found_parent) {
found_child->dentry = dget(dentry);
+ found_child->type = AUDIT_TYPE_CHILD_ANON;
+ }
}
EXPORT_SYMBOL_GPL(__audit_inode_child);
--
1.8.3.1
Richard Guy Briggs
2018-02-12 05:02:22 UTC
Permalink
Raw Message
Append a new fstype field that gives the filesystem type magic value in
hexadecimal to help identify previously null PATH records produced by
audit_inode_child logging requests on inodes with anonymous parents.

Sample output:
type=PROCTITLE msg=audit(1488317694.446:143): proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D006E66737634
type=PATH msg=audit(1488317694.446:143): item=797 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=74726163
type=PATH msg=audit(1488317694.446:143): item=796 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=74726163
...
type=PATH msg=audit(1488317694.446:143): item=1 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=74726163
type=PATH msg=audit(1488317694.446:143): item=0 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=74726163
type=KERN_MODULE 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 <***@redhat.com>
---
kernel/audit.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/kernel/audit.c b/kernel/audit.c
index 0c8d5a8..1c9d0a4 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2076,6 +2076,8 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
const struct path *path, int record_num, int *call_panic)
{
struct audit_buffer *ab;
+ unsigned long fstype;
+
ab = audit_log_start(context, GFP_KERNEL, AUDIT_PATH);
if (!ab)
return;
@@ -2120,6 +2122,7 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
}
audit_log_format(ab, " name=");
audit_log_untrustedstring(ab, fullpathp ?: "?");
+ fstype = n->dentry->d_sb->s_magic;
if (fullpath)
kfree(fullpath);
} else {
@@ -2173,6 +2176,10 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
}

audit_log_fcaps(ab, n);
+ if (fstype)
+ audit_log_format(ab, " fstype=0x%lx", fstype);
+ else
+ audit_log_format(ab, " fstype=?");
audit_log_end(ab);
}
--
1.8.3.1
Paul Moore
2018-02-15 22:15:05 UTC
Permalink
Raw Message
More than one filesystem was causing hundreds to thousands of null PATH
records to be associated with the *init_module SYSCALL records on a few
modules with corresponding audit syscall rules.
This patchset adds extra information to those PATH records to provide
insight into what is generating them, including a partial pathname,
fstype field, and two new filetypes that indicate the pathname isn't
anchored at the root of the task's root filesystem.
audit: show partial pathname for entries with anonymous parents
audit: append new fstype field for anonymous PATH records
audit: add new filetypes CREATE_ANON and PARENT_ANON
The more I look at this, the more I prefer your original approach that
prefixed the relative pathname with the fstype. Yes, I do realize
that you sort of work around that by including the fstype as a new
field in the PATH records, but we're still stuck with those odd
relative/un-rooted name fields.

Further, I don't recall ever hearing a good reason why the original
approach wasn't acceptable to Steve's userspace. I know he did make
some very last minute hand-wavy comments, but none of those made any
sense to me; I don't understand why Steve's audit record parser is
even looking in the pathname string.

I'm going to park these patches in limbo for the time being.
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2018-02-16 08:23:30 UTC
Permalink
Raw Message
Post by Paul Moore
More than one filesystem was causing hundreds to thousands of null PATH
records to be associated with the *init_module SYSCALL records on a few
modules with corresponding audit syscall rules.
This patchset adds extra information to those PATH records to provide
insight into what is generating them, including a partial pathname,
fstype field, and two new filetypes that indicate the pathname isn't
anchored at the root of the task's root filesystem.
audit: show partial pathname for entries with anonymous parents
audit: append new fstype field for anonymous PATH records
audit: add new filetypes CREATE_ANON and PARENT_ANON
The more I look at this, the more I prefer your original approach that
prefixed the relative pathname with the fstype. Yes, I do realize
that you sort of work around that by including the fstype as a new
field in the PATH records, but we're still stuck with those odd
relative/un-rooted name fields.
They are signalled as being unrooted by the ANON filetypes. And now
that you mention it, should fail the ausearch-test since it isn't a "full
path", as claimed is necessary in ghak70 (so I don't see why the
KERN_MODULE name= record/field fails that test).
Post by Paul Moore
Further, I don't recall ever hearing a good reason why the original
approach wasn't acceptable to Steve's userspace. I know he did make
some very last minute hand-wavy comments, but none of those made any
sense to me; I don't understand why Steve's audit record parser is
even looking in the pathname string.
I'm going to park these patches in limbo for the time being.
Can you give me an idea how long that might be?
Post by Paul Moore
paul moore
- RGB

--
Richard Guy Briggs <***@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
2018-02-16 18:29:22 UTC
Permalink
Raw Message
Post by Richard Guy Briggs
Post by Paul Moore
More than one filesystem was causing hundreds to thousands of null PATH
records to be associated with the *init_module SYSCALL records on a few
modules with corresponding audit syscall rules.
This patchset adds extra information to those PATH records to provide
insight into what is generating them, including a partial pathname,
fstype field, and two new filetypes that indicate the pathname isn't
anchored at the root of the task's root filesystem.
audit: show partial pathname for entries with anonymous parents
audit: append new fstype field for anonymous PATH records
audit: add new filetypes CREATE_ANON and PARENT_ANON
The more I look at this, the more I prefer your original approach that
prefixed the relative pathname with the fstype. Yes, I do realize
that you sort of work around that by including the fstype as a new
field in the PATH records, but we're still stuck with those odd
relative/un-rooted name fields.
They are signalled as being unrooted by the ANON filetypes. And now
that you mention it, should fail the ausearch-test since it isn't a "full
path", as claimed is necessary in ghak70 (so I don't see why the
KERN_MODULE name= record/field fails that test).
Yes. I still prefer your original approach.
Post by Richard Guy Briggs
Post by Paul Moore
Further, I don't recall ever hearing a good reason why the original
approach wasn't acceptable to Steve's userspace. I know he did make
some very last minute hand-wavy comments, but none of those made any
sense to me; I don't understand why Steve's audit record parser is
even looking in the pathname string.
I'm going to park these patches in limbo for the time being.
Can you give me an idea how long that might be?
If you need an answer right now, consider it to be "indefinitely".
--
paul moore
www.paul-moore.com
Steve Grubb
2018-02-15 23:07:41 UTC
Permalink
Raw Message
Post by 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
-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 for a device that may
no longer be visible upon log inspeciton, and an inode number.
Fill in the partial known pathname from the filesystem mount point to
the leaf node 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 able to address them.
proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D006E66737634 type=PATH
msg=audit(1488317694.446:143): item=797
name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
cap_fver=0 type=PATH msg=audit(1488317694.446:143): item=796
name=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
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 ...
type=PATH msg=audit(1488317694.446:143): item=1 name=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 cap_fp=0000000000000000
cap_fi=0000000000000000 cap_fe=0 cap_fver=0 type=PATH
msg=audit(1488317694.446:143): item=0 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
cap_fver=0 type=KERN_MODULE 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"
Thanks for the samples, but the event above fails the ausearch-test test
suite. The "name" field in the PATH record is not properly escaped.

-Steve
Post by Richard Guy Briggs
See: https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42
---
fix fullpath memleak
switch from log_format() to audit_log_untrustedstring()
remove leading / from pathname relative to unknown mount point
fix audit_buffer leak and dname error allocation leak audit_log_name
only put audit_name->dentry if it is being replaced
deconstify struct dentry*
add hex prefix to fstype
---
include/linux/audit.h | 8 ++++----
kernel/audit.c | 28 +++++++++++++++++++++++++++-
kernel/audit.h | 1 +
kernel/auditsc.c | 8 +++++++-
4 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..2020f1d 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -232,7 +232,7 @@ extern void __audit_inode(struct filename *name, const
struct dentry *dentry, unsigned int flags);
extern void __audit_file(const struct file *);
extern void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type);
extern void __audit_seccomp(unsigned long syscall, long signr, int code);
extern void __audit_ptrace(struct task_struct *t);
@@ -297,7 +297,7 @@ static inline void audit_inode_parent_hidden(struct
filename *name, AUDIT_INODE_PARENT | AUDIT_INODE_HIDDEN);
}
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type) {
if (unlikely(!audit_dummy_context()))
__audit_inode_child(parent, dentry, type);
@@ -481,7 +481,7 @@ static inline void __audit_inode(struct filename *name,
unsigned int flags)
{ }
static inline void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_inode(struct filename *name,
@@ -495,7 +495,7 @@ static inline void audit_inode_parent_hidden(struct
filename *name, const struct dentry *dentry)
{ }
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_core_dumps(long signr)
diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99..0c8d5a8 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -72,6 +72,7 @@
#include <linux/freezer.h>
#include <linux/pid_namespace.h>
#include <net/netns/generic.h>
+#include <linux/dcache.h>
#include "audit.h"
@@ -2056,6 +2057,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);
}
@@ -2097,8 +2102,29 @@ void audit_log_name(struct audit_context *context,
struct audit_names *n, audit_log_n_untrustedstring(ab, n->name->name,
n->name_len);
}
- } else
+ } else if (n->dentry) {
+ char *fullpath;
+ const char *fullpathp = NULL;
+
+ fullpath = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (fullpath) {
+ fullpathp = dentry_path_raw(n->dentry, fullpath, PATH_MAX);
+ if (IS_ERR(fullpathp)) {
+ fullpathp = NULL;
+ } else {
+ while (*fullpathp == '/')
+ fullpathp++;
+ if (*fullpathp == 0)
+ fullpathp = NULL;
+ }
+ }
+ audit_log_format(ab, " name=");
+ audit_log_untrustedstring(ab, fullpathp ?: "?");
+ if (fullpath)
+ kfree(fullpath);
+ } else {
audit_log_format(ab, " name=(null)");
+ }
if (n->ino != AUDIT_INO_UNSET)
audit_log_format(ab, " inode=%lu"
diff --git a/kernel/audit.h b/kernel/audit.h
index af5bc59..81f6865 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -85,6 +85,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 e80459f..b73ede0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -75,6 +75,7 @@
#include <linux/uaccess.h>
#include <linux/fsnotify_backend.h>
#include <uapi/linux/limits.h>
+#include <linux/dcache.h>
#include "audit.h"
@@ -882,6 +883,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);
}
@@ -1862,7 +1865,7 @@ void __audit_file(const struct file *file)
* unsuccessful attempts.
*/
void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{
struct audit_context *context = current->audit_context;
@@ -1941,6 +1944,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) {
@@ -1962,6 +1966,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);
Richard Guy Briggs
2018-02-15 23:19:07 UTC
Permalink
Raw Message
Post by Steve Grubb
Post by 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
-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 for a device that may
no longer be visible upon log inspeciton, and an inode number.
Fill in the partial known pathname from the filesystem mount point to
the leaf node 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 able to address them.
proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D006E66737634 type=PATH
msg=audit(1488317694.446:143): item=797
name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
cap_fver=0 type=PATH msg=audit(1488317694.446:143): item=796
name=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
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 ...
type=PATH msg=audit(1488317694.446:143): item=1 name=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 cap_fp=0000000000000000
cap_fi=0000000000000000 cap_fe=0 cap_fver=0 type=PATH
msg=audit(1488317694.446:143): item=0 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
cap_fver=0 type=KERN_MODULE 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"
Thanks for the samples, but the event above fails the ausearch-test test
suite. The "name" field in the PATH record is not properly escaped.
Let me re-run the ausearch-test on the log file from the machine in
question... I don't remember if I copied the above from a recent run,
or just hand-edited the output to update it. It should be fine since it
was updated to now run through audit_log_untrustedstring().
Post by Steve Grubb
-Steve
Post by Richard Guy Briggs
See: https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42
---
fix fullpath memleak
switch from log_format() to audit_log_untrustedstring()
remove leading / from pathname relative to unknown mount point
fix audit_buffer leak and dname error allocation leak audit_log_name
only put audit_name->dentry if it is being replaced
deconstify struct dentry*
add hex prefix to fstype
---
include/linux/audit.h | 8 ++++----
kernel/audit.c | 28 +++++++++++++++++++++++++++-
kernel/audit.h | 1 +
kernel/auditsc.c | 8 +++++++-
4 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..2020f1d 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -232,7 +232,7 @@ extern void __audit_inode(struct filename *name, const
struct dentry *dentry, unsigned int flags);
extern void __audit_file(const struct file *);
extern void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type);
extern void __audit_seccomp(unsigned long syscall, long signr, int code);
extern void __audit_ptrace(struct task_struct *t);
@@ -297,7 +297,7 @@ static inline void audit_inode_parent_hidden(struct
filename *name, AUDIT_INODE_PARENT | AUDIT_INODE_HIDDEN);
}
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type) {
if (unlikely(!audit_dummy_context()))
__audit_inode_child(parent, dentry, type);
@@ -481,7 +481,7 @@ static inline void __audit_inode(struct filename *name,
unsigned int flags)
{ }
static inline void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_inode(struct filename *name,
@@ -495,7 +495,7 @@ static inline void audit_inode_parent_hidden(struct
filename *name, const struct dentry *dentry)
{ }
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_core_dumps(long signr)
diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99..0c8d5a8 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -72,6 +72,7 @@
#include <linux/freezer.h>
#include <linux/pid_namespace.h>
#include <net/netns/generic.h>
+#include <linux/dcache.h>
#include "audit.h"
@@ -2056,6 +2057,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);
}
@@ -2097,8 +2102,29 @@ void audit_log_name(struct audit_context *context,
struct audit_names *n, audit_log_n_untrustedstring(ab, n->name->name,
n->name_len);
}
- } else
+ } else if (n->dentry) {
+ char *fullpath;
+ const char *fullpathp = NULL;
+
+ fullpath = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (fullpath) {
+ fullpathp = dentry_path_raw(n->dentry, fullpath, PATH_MAX);
+ if (IS_ERR(fullpathp)) {
+ fullpathp = NULL;
+ } else {
+ while (*fullpathp == '/')
+ fullpathp++;
+ if (*fullpathp == 0)
+ fullpathp = NULL;
+ }
+ }
+ audit_log_format(ab, " name=");
+ audit_log_untrustedstring(ab, fullpathp ?: "?");
+ if (fullpath)
+ kfree(fullpath);
+ } else {
audit_log_format(ab, " name=(null)");
+ }
if (n->ino != AUDIT_INO_UNSET)
audit_log_format(ab, " inode=%lu"
diff --git a/kernel/audit.h b/kernel/audit.h
index af5bc59..81f6865 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -85,6 +85,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 e80459f..b73ede0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -75,6 +75,7 @@
#include <linux/uaccess.h>
#include <linux/fsnotify_backend.h>
#include <uapi/linux/limits.h>
+#include <linux/dcache.h>
#include "audit.h"
@@ -882,6 +883,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);
}
@@ -1862,7 +1865,7 @@ void __audit_file(const struct file *file)
* unsuccessful attempts.
*/
void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{
struct audit_context *context = current->audit_context;
@@ -1941,6 +1944,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) {
@@ -1962,6 +1966,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);
- RGB

--
Richard Guy Briggs <***@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Richard Guy Briggs
2018-02-16 06:30:23 UTC
Permalink
Raw Message
Post by Richard Guy Briggs
Post by Steve Grubb
Post by 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
-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 for a device that may
no longer be visible upon log inspeciton, and an inode number.
Fill in the partial known pathname from the filesystem mount point to
the leaf node 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 able to address them.
proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D006E66737634 type=PATH
msg=audit(1488317694.446:143): item=797
name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
cap_fver=0 type=PATH msg=audit(1488317694.446:143): item=796
name=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
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 ...
type=PATH msg=audit(1488317694.446:143): item=1 name=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 cap_fp=0000000000000000
cap_fi=0000000000000000 cap_fe=0 cap_fver=0 type=PATH
msg=audit(1488317694.446:143): item=0 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
cap_fver=0 type=KERN_MODULE 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"
So, updated sample output is:
type=SYSCALL msg=audit(1518738520.800:264): arch=c000003e syscall=313 success=yes exit=0 a0=8 a1=55c51f395fc6 a2=0 a3=8 items=834 ppid=579 pid=608 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=1 comm="modprobe" exe="/usr/bin/kmod" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="mod-load"
type=KERN_MODULE msg=audit(1518738520.800:264): name="nfsv4"
type=PATH msg=audit(1518738520.800:264): item=0 name="events" inode=127 dev=00:0b mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=PARENT_ANON cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=0x74726163
type=PATH msg=audit(1518738520.800:264): item=1 name="events/nfs4" inode=17795 dev=00:0b mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=CREATE_ANON cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=0x74726163
...
type=PATH msg=audit(1518738520.800:264): item=832 name="events/nfs4/nfs4_setclientid" inode=18206 dev=00:0b mode=040755 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=PARENT_ANON cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=0x74726163
type=PATH msg=audit(1518738520.800:264): item=833 name="events/nfs4/nfs4_setclientid/format" inode=18211 dev=00:0b mode=0100444 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tracefs_t:s0 nametype=CREATE_ANON cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 fstype=0x74726163
type=PROCTITLE msg=audit(1518738520.800:264): proctitle=6D6F6470726F6265006E66737634
Post by Richard Guy Briggs
Post by Steve Grubb
Thanks for the samples, but the event above fails the ausearch-test test
suite. The "name" field in the PATH record is not properly escaped.
Let me re-run the ausearch-test on the log file from the machine in
question... I don't remember if I copied the above from a recent run,
or just hand-edited the output to update it. It should be fine since it
was updated to now run through audit_log_untrustedstring().
It is fine, as expected. The only errors I get are expected ones
already documented in ghak70 for the KERN_MODULE record, complaints
about the name= field.
Post by Richard Guy Briggs
Post by Steve Grubb
-Steve
Post by Richard Guy Briggs
See: https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42
---
fix fullpath memleak
switch from log_format() to audit_log_untrustedstring()
remove leading / from pathname relative to unknown mount point
fix audit_buffer leak and dname error allocation leak audit_log_name
only put audit_name->dentry if it is being replaced
deconstify struct dentry*
add hex prefix to fstype
---
include/linux/audit.h | 8 ++++----
kernel/audit.c | 28 +++++++++++++++++++++++++++-
kernel/audit.h | 1 +
kernel/auditsc.c | 8 +++++++-
4 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..2020f1d 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -232,7 +232,7 @@ extern void __audit_inode(struct filename *name, const
struct dentry *dentry, unsigned int flags);
extern void __audit_file(const struct file *);
extern void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type);
extern void __audit_seccomp(unsigned long syscall, long signr, int code);
extern void __audit_ptrace(struct task_struct *t);
@@ -297,7 +297,7 @@ static inline void audit_inode_parent_hidden(struct
filename *name, AUDIT_INODE_PARENT | AUDIT_INODE_HIDDEN);
}
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type) {
if (unlikely(!audit_dummy_context()))
__audit_inode_child(parent, dentry, type);
@@ -481,7 +481,7 @@ static inline void __audit_inode(struct filename *name,
unsigned int flags)
{ }
static inline void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_inode(struct filename *name,
@@ -495,7 +495,7 @@ static inline void audit_inode_parent_hidden(struct
filename *name, const struct dentry *dentry)
{ }
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_core_dumps(long signr)
diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99..0c8d5a8 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -72,6 +72,7 @@
#include <linux/freezer.h>
#include <linux/pid_namespace.h>
#include <net/netns/generic.h>
+#include <linux/dcache.h>
#include "audit.h"
@@ -2056,6 +2057,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);
}
@@ -2097,8 +2102,29 @@ void audit_log_name(struct audit_context *context,
struct audit_names *n, audit_log_n_untrustedstring(ab, n->name->name,
n->name_len);
}
- } else
+ } else if (n->dentry) {
+ char *fullpath;
+ const char *fullpathp = NULL;
+
+ fullpath = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (fullpath) {
+ fullpathp = dentry_path_raw(n->dentry, fullpath, PATH_MAX);
+ if (IS_ERR(fullpathp)) {
+ fullpathp = NULL;
+ } else {
+ while (*fullpathp == '/')
+ fullpathp++;
+ if (*fullpathp == 0)
+ fullpathp = NULL;
+ }
+ }
+ audit_log_format(ab, " name=");
+ audit_log_untrustedstring(ab, fullpathp ?: "?");
+ if (fullpath)
+ kfree(fullpath);
+ } else {
audit_log_format(ab, " name=(null)");
+ }
if (n->ino != AUDIT_INO_UNSET)
audit_log_format(ab, " inode=%lu"
diff --git a/kernel/audit.h b/kernel/audit.h
index af5bc59..81f6865 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -85,6 +85,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 e80459f..b73ede0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -75,6 +75,7 @@
#include <linux/uaccess.h>
#include <linux/fsnotify_backend.h>
#include <uapi/linux/limits.h>
+#include <linux/dcache.h>
#include "audit.h"
@@ -882,6 +883,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);
}
@@ -1862,7 +1865,7 @@ void __audit_file(const struct file *file)
* unsuccessful attempts.
*/
void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{
struct audit_context *context = current->audit_context;
@@ -1941,6 +1944,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) {
@@ -1962,6 +1966,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);
- RGB
- RGB

--
Richard Guy Briggs <***@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Richard Guy Briggs
2018-02-16 06:00:33 UTC
Permalink
Raw Message
Post by Steve Grubb
Post by 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
-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 for a device that may
no longer be visible upon log inspeciton, and an inode number.
Fill in the partial known pathname from the filesystem mount point to
the leaf node 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 able to address them.
proctitle=2F7362696E2F6D6F6470726F6265002D71002D2D006E66737634 type=PATH
msg=audit(1488317694.446:143): item=797
name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
cap_fver=0 type=PATH msg=audit(1488317694.446:143): item=796
name=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
cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0 ...
type=PATH msg=audit(1488317694.446:143): item=1 name=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 cap_fp=0000000000000000
cap_fi=0000000000000000 cap_fe=0 cap_fver=0 type=PATH
msg=audit(1488317694.446:143): item=0 name=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 cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
cap_fver=0 type=KERN_MODULE 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"
Thanks for the samples, but the event above fails the ausearch-test test
suite. The "name" field in the PATH record is not properly escaped.
Is the ausearch-test on github yet? Last I see is v0.6 on your rh
people page.
Post by Steve Grubb
-Steve
Post by Richard Guy Briggs
See: https://github.com/linux-audit/audit-kernel/issues/8
Test case: https://github.com/linux-audit/audit-testsuite/issues/42
---
fix fullpath memleak
switch from log_format() to audit_log_untrustedstring()
remove leading / from pathname relative to unknown mount point
fix audit_buffer leak and dname error allocation leak audit_log_name
only put audit_name->dentry if it is being replaced
deconstify struct dentry*
add hex prefix to fstype
---
include/linux/audit.h | 8 ++++----
kernel/audit.c | 28 +++++++++++++++++++++++++++-
kernel/audit.h | 1 +
kernel/auditsc.c | 8 +++++++-
4 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..2020f1d 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -232,7 +232,7 @@ extern void __audit_inode(struct filename *name, const
struct dentry *dentry, unsigned int flags);
extern void __audit_file(const struct file *);
extern void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type);
extern void __audit_seccomp(unsigned long syscall, long signr, int code);
extern void __audit_ptrace(struct task_struct *t);
@@ -297,7 +297,7 @@ static inline void audit_inode_parent_hidden(struct
filename *name, AUDIT_INODE_PARENT | AUDIT_INODE_HIDDEN);
}
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type) {
if (unlikely(!audit_dummy_context()))
__audit_inode_child(parent, dentry, type);
@@ -481,7 +481,7 @@ static inline void __audit_inode(struct filename *name,
unsigned int flags)
{ }
static inline void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_inode(struct filename *name,
@@ -495,7 +495,7 @@ static inline void audit_inode_parent_hidden(struct
filename *name, const struct dentry *dentry)
{ }
static inline void audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{ }
static inline void audit_core_dumps(long signr)
diff --git a/kernel/audit.c b/kernel/audit.c
index 227db99..0c8d5a8 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -72,6 +72,7 @@
#include <linux/freezer.h>
#include <linux/pid_namespace.h>
#include <net/netns/generic.h>
+#include <linux/dcache.h>
#include "audit.h"
@@ -2056,6 +2057,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);
}
@@ -2097,8 +2102,29 @@ void audit_log_name(struct audit_context *context,
struct audit_names *n, audit_log_n_untrustedstring(ab, n->name->name,
n->name_len);
}
- } else
+ } else if (n->dentry) {
+ char *fullpath;
+ const char *fullpathp = NULL;
+
+ fullpath = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (fullpath) {
+ fullpathp = dentry_path_raw(n->dentry, fullpath, PATH_MAX);
+ if (IS_ERR(fullpathp)) {
+ fullpathp = NULL;
+ } else {
+ while (*fullpathp == '/')
+ fullpathp++;
+ if (*fullpathp == 0)
+ fullpathp = NULL;
+ }
+ }
+ audit_log_format(ab, " name=");
+ audit_log_untrustedstring(ab, fullpathp ?: "?");
+ if (fullpath)
+ kfree(fullpath);
+ } else {
audit_log_format(ab, " name=(null)");
+ }
if (n->ino != AUDIT_INO_UNSET)
audit_log_format(ab, " inode=%lu"
diff --git a/kernel/audit.h b/kernel/audit.h
index af5bc59..81f6865 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -85,6 +85,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 e80459f..b73ede0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -75,6 +75,7 @@
#include <linux/uaccess.h>
#include <linux/fsnotify_backend.h>
#include <uapi/linux/limits.h>
+#include <linux/dcache.h>
#include "audit.h"
@@ -882,6 +883,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);
}
@@ -1862,7 +1865,7 @@ void __audit_file(const struct file *file)
* unsuccessful attempts.
*/
void __audit_inode_child(struct inode *parent,
- const struct dentry *dentry,
+ struct dentry *dentry,
const unsigned char type)
{
struct audit_context *context = current->audit_context;
@@ -1941,6 +1944,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) {
@@ -1962,6 +1966,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);
- RGB

--
Richard Guy Briggs <***@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Loading...