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

["Isaac J. Manjarres" <isaacmanjarres@google.com>]

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 <isaacmanjarres@google.com>
---
 kernel/audit.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

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