On Thu, Aug 2, 2018 at 7:45 AM Ondrej Mosnacek <omosnace(a)redhat.com> wrote:
When a relative path has just a single component and we want to emit a
nametype=PARENT record, the current implementation just reports the full
CWD path (which is alrady available in the audit context).
This is wrong for three reasons:
1. Wasting log space for redundant data (CWD path is already in the CWD
record).
2. Inconsistency with other PATH records (if a relative PARENT directory
path contains at least one component, only the verbatim relative path
is logged).
3. In some syscalls (e.g. openat(2)) the relative path may not even be
relative to the CWD, but to another directory specified as a file
descriptor. In that case the logged path is simply plain wrong.
This patch modifies this behavior to simply report "." in the
aforementioned case, which is equivalent to an "empty" directory path
and can be concatenated with the actual base directory path (CWD or
dirfd from openat(2)-like syscall) once support for its logging is added
later. In the meantime, defaulting to CWD as base directory on relative
paths (as already done by the userspace tools) will be enough to achieve
results equivalent to the current behavior.
See:
https://github.com/linux-audit/audit-kernel/issues/95
Fixes: 9c937dcc7102 ("[PATCH] log more info for directory entry change
events")
Signed-off-by: Ondrej Mosnacek <omosnace(a)redhat.com>
---
kernel/audit.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 2a8058764aa6..4f18bd48eb4b 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2127,28 +2127,27 @@ void audit_log_name(struct audit_context *context, struct
audit_names *n,
audit_log_format(ab, "item=%d", record_num);
+ audit_log_format(ab, " name=");
if (path)
- audit_log_d_path(ab, " name=", path);
+ audit_log_d_path(ab, NULL, path);
else if (n->name) {
switch (n->name_len) {
case AUDIT_NAME_FULL:
/* log the full path */
- audit_log_format(ab, " name=");
audit_log_untrustedstring(ab, n->name->name);
break;
case 0:
/* name was specified as a relative path and the
* directory component is the cwd */
- audit_log_d_path(ab, " name=", &context->pwd);
+ audit_log_untrustedstring(ab, ".");
This isn't a comprehensive review, I only gave this a quick look, but
considering you are only logging "." above we can safely use
audit_log_string() and safe a few cycles.
Honestly, looking at the rest of this function, why are we using
audit_log_format() in the case where we are simply writing a string
literal? While I haven't tested it, simple code inspection would seem
to indicate that audit_log_string() should be much quicker than
audit_log_format()? I realize this isn't strictly a problem from this
patch, but we probably shouldn't be propagating this mistake any
further.
--
paul moore