On 2018-08-02 13:44, Ondrej Mosnacek 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

Untested.  This looks like a reasonable rationale and solution to me.

> Fixes: 9c937dcc7102 ("[PATCH] log more info for directory entry change 
> events")
> Signed-off-by: Ondrej Mosnacek <[email protected]>

Reviewed-by: Richard Guy Briggs <[email protected]>

> ---
>  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, ".");
>                       break;
>               default:
>                       /* log the name's directory component */
> -                     audit_log_format(ab, " name=");
>                       audit_log_n_untrustedstring(ab, n->name->name,
>                                                   n->name_len);
>               }
>       } else
> -             audit_log_format(ab, " name=(null)");
> +             audit_log_format(ab, "(null)");
>  
>       if (n->ino != AUDIT_INO_UNSET)
>               audit_log_format(ab, " inode=%lu"
> -- 
> 2.17.1
> 

- RGB

--
Richard Guy Briggs <[email protected]>
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

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to