Re: [PATCH v1] audit: Improve path logging for unlinked files

On Tue, Oct 28, 2025 at 8:54 PM Isaac J. Manjarres
<[email protected]> wrote:
>
> When logging the path associated with a denial, the path is scanned
> to ensure that it does not contain control characters, unprintable
> characters, double quote marks, or spaces. If it does, the hexadecimal
> representation of the path is emitted.
>
> This can make debugging difficult in scenarios where the file name that
> was given to the file does not contain any of those characters,
> but the hexadecimal representation of the path is still emitted when a
> denial occurs because the file is unlinked.
>
> For example, suppose a memfd is created with the name "MemoryHeapBase".
> Memfds are unlinked, so when a denial related to that memfd is
> encountered, and the the path name for it is obtained via d_path(), the
> name will be: "/memfd:MemoryHeapBase (deleted)". Since the name has a
> space, the audit logic will just print the hexadecimal representation
> instead of the name:
>
> type=1400 audit(0.0:319): avc:  denied  { read write } for
> path=2F6D656D66643A4D656D6F72794865617042617365202864656C6574656429
> dev="tmpfs" ino=75 scontext=u:r:audioserver:s0
> tcontext=u:object_r:system_server:s0 tclass=memfd_file permissive=0
>
> To improve debuggability of denials related to unlinked files, check
> if the " (deleted)" suffix is detected in a path name and remove it
> if so. This allows the actual filename to be validated and emitted
> if appropriate, making denials easier to read and more actionable:
>
> type=1400 audit(0.0:254): avc:  denied  { read write } for
> path="/memfd:MemoryHeapBase" dev="tmpfs" ino=67
> scontext=u:r:audioserver:s0 tcontext=u:object_r:system_server:s0
> tclass=memfd_file permissive=0
>
> Signed-off-by: Isaac J. Manjarres <[email protected]>
> ---
>  kernel/audit.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)

I'd prefer not to add any additional string processing to the audit hot path.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 26a332ffb1b8..dcfa60e0277d 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2184,7 +2184,7 @@ void audit_log_untrustedstring(struct audit_buffer *ab, const char *string)
>  void audit_log_d_path(struct audit_buffer *ab, const char *prefix,
>                       const struct path *path)
>  {
> -       char *p, *pathname;
> +       char *p, *pathname, *suffix;
>
>         if (prefix)
>                 audit_log_format(ab, "%s", prefix);
> @@ -2199,8 +2199,20 @@ void audit_log_d_path(struct audit_buffer *ab, const char *prefix,
>         if (IS_ERR(p)) { /* Should never happen since we send PATH_MAX */
>                 /* FIXME: can we save some information here? */
>                 audit_log_format(ab, "\"<too_long>\"");
> -       } else
> +       } else {
> +               /*
> +                * Terminate the buffer where the " (deleted)" suffix starts so
> +                * that audit_log_untrustedstring() emits the pathname,
> +                * assuming it doesn't have other control characters or spaces.
> +                */
> +               suffix = strstr(p, " (deleted)");
> +               /* Ensure the string ends with the " (deleted)" suffix. */
> +               if (suffix &&
> +                   ((p + strlen(p) - strlen(" (deleted)")) == suffix))
> +                       *suffix = '\0';
> +
>                 audit_log_untrustedstring(ab, p);
> +       }
>         kfree(pathname);
>  }
>
> --
> 2.51.1.851.g4ebd6896fd-goog

-- 
paul-moore.com