[PATCH ghak95] audit: Do not log full CWD path on empty relative paths
Paul Moore
paul at paul-moore.com
Thu Aug 2 22:24:13 UTC 2018
On Thu, Aug 2, 2018 at 7:45 AM Ondrej Mosnacek <omosnace at 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 at 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
www.paul-moore.com
More information about the Linux-audit
mailing list