[PATCH v2 1/3] audit: compact name entries and context fields

[PATCH v2 1/3] audit: compact name entries and context fields

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

Group the inode metadata and the small scalar fields of audit_names to
remove padding, while keeping the pathname next to its length and the
file capability revision next to the file capabilities. Store the
per-name file capability revision in s16: it must represent both the
eight-bit on-disk revision and the -1 AUDIT_INODE_NOEVAL sentinel. Keep
the existing formatter and its unknown-capability output unchanged.

Move name_count ahead of the embedded name slots to remove two alignment
holes, and place personality before the task credential scalars.
Preserve the first int dummy member required by audit_dummy_context(),
all five embedded name slots, and the existing allocation and reference
lifetimes.

On x86_64 with SELinux this reduces audit_context from 928 to 880 bytes,
leaving room for descriptor paths while staying below 1 KiB. On the tested
arm64 configuration without property-bearing LSMs it falls to 872 bytes.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
- keep name next to name_len and fcap_ver next to fcap
- move name_count below the names_list comment
---
 kernel/audit.h | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/kernel/audit.h b/kernel/audit.h
index 7640d2c0fba4..614afa1cfa14 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -77,17 +77,14 @@ struct audit_names {
 
 	struct filename		*name;
 	int			name_len;	/* number of chars to log */
-	bool			hidden;		/* don't log this record */
-
-	u64			ino;
 	dev_t			dev;
-	umode_t			mode;
+	u64			ino;
 	kuid_t			uid;
 	kgid_t			gid;
 	dev_t			rdev;
 	struct lsm_prop		oprop;
-	struct audit_file_caps	fcap;
-	unsigned int		fcap_ver;
+	umode_t			mode;
+	bool			hidden;		/* don't log this record */
 	unsigned char		type;		/* record type */
 	/*
 	 * This was an allocated audit_names and not from the array of
@@ -95,6 +92,8 @@ struct audit_names {
 	 * should be freed on syscall exit.
 	 */
 	bool			should_free;
+	s16			fcap_ver;	/* 8-bit revision, or -1 for NOEVAL */
+	struct audit_file_caps	fcap;
 };
 
 struct audit_proctitle {
@@ -132,8 +131,8 @@ struct audit_context {
 	 * through the preallocated_names array and should only be found/used
 	 * by running the names_list.
 	 */
-	struct audit_names  preallocated_names[AUDIT_NAMES];
 	int		    name_count; /* total records in names_list */
+	struct audit_names  preallocated_names[AUDIT_NAMES];
 	struct list_head    names_list;	/* struct audit_names->list anchor */
 	char		    *filterkey;	/* key for rule that triggered record */
 	struct path	    pwd;
@@ -142,10 +141,10 @@ struct audit_context {
 	struct sockaddr_storage *sockaddr;
 	size_t sockaddr_len;
 	/* Save things to print about task_struct */
+	unsigned long	    personality;
 	pid_t		    ppid;
 	kuid_t		    uid, euid, suid, fsuid;
 	kgid_t		    gid, egid, sgid, fsgid;
-	unsigned long	    personality;
 	int		    arch;
 
 	pid_t		    target_pid;
-- 
2.55.0

[PATCH v2 2/3] audit: return the collected entry from __audit_inode()

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

Return the selected audit_names entry, or NULL when collection is
skipped or fails, so audit_file() can attach descriptor-specific
information reliably. The existing audit_inode() and
audit_inode_parent_hidden() wrappers ignore the result, and collection
behavior is unchanged.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2: return the entry from __audit_inode() instead of adding a private
    helper
---
 include/linux/audit.h |  6 ++++--
 kernel/auditsc.c      | 14 +++++++++-----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 767e349d4834..1f456f954c2e 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -35,6 +35,7 @@ struct audit_sig_info {
 
 struct audit_buffer;
 struct audit_context;
+struct audit_names;
 struct inode;
 struct netlink_skb_parms;
 struct path;
@@ -327,8 +328,9 @@ extern void __audit_syscall_entry(int major, unsigned long a0, unsigned long a1,
 				  unsigned long a2, unsigned long a3);
 extern void __audit_syscall_exit(int ret_success, long ret_value);
 extern void __audit_getname(struct filename *name);
-extern void __audit_inode(struct filename *name, const struct dentry *dentry,
-				unsigned int flags);
+extern struct audit_names *__audit_inode(struct filename *name,
+					 const struct dentry *dentry,
+					 unsigned int flags);
 extern void __audit_file(const struct file *);
 extern void __audit_inode_child(struct inode *parent,
 				const struct dentry *dentry,
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 552ba9f4fb0a..b740e056cd80 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2242,9 +2242,12 @@ static void audit_copy_inode(struct audit_names *name,
  * @name: name being audited
  * @dentry: dentry being audited
  * @flags: attributes for this particular entry
+ *
+ * Return: the collected entry, or NULL if collection was skipped or failed.
  */
-void __audit_inode(struct filename *name, const struct dentry *dentry,
-		   unsigned int flags)
+struct audit_names *__audit_inode(struct filename *name,
+				  const struct dentry *dentry,
+				  unsigned int flags)
 {
 	struct audit_context *context = audit_context();
 	struct inode *inode = d_backing_inode(dentry);
@@ -2255,7 +2258,7 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
 	int i;
 
 	if (context->context == AUDIT_CTX_UNUSED)
-		return;
+		return NULL;
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(e, list, list) {
@@ -2267,7 +2270,7 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
 						f->op, f->val)
 			    && e->rule.action == AUDIT_NEVER) {
 				rcu_read_unlock();
-				return;
+				return NULL;
 			}
 		}
 	}
@@ -2321,7 +2324,7 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
 	/* unable to find an entry with both a matching name and type */
 	n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
 	if (!n)
-		return;
+		return NULL;
 	if (name) {
 		n->name = name;
 		name->refcnt++;
@@ -2339,6 +2342,7 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
 	}
 	handle_path(dentry);
 	audit_copy_inode(n, dentry, inode, flags & AUDIT_INODE_NOEVAL);
+	return n;
 }
 
 void __audit_file(const struct file *file)
-- 
2.55.0

[PATCH v2 3/3] audit: retain file paths for descriptor PATH records

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

fchown() and the other audit_file() callers collect inode metadata but
emit name=(null), even though file->f_path is available.

Retain the path with balanced dentry and mount references, and use the
existing audit_log_d_path() formatter when no filename was collected.
Release the references on context cleanup. Descriptor entries carry no
filename, so they can only be reused by a later lookup that matches the
same inode; keep the retained path then instead of falling back to
name=(null). Filesystem exclusions continue to skip entry creation.

The name reflects d_path() at event emission: concurrent renames and
unlinks can affect it. Original lookup names retain precedence.

This adds sizeof(struct path) to each audit_names. With the preceding
capability and layout changes, audit_context grows from 880 to 960 bytes
on the tested x86_64 SELinux configuration and remains in the 1 KiB
kmalloc class. Other LSM configurations can have different object sizes.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
- reset fd_path after path_put() in audit_free_names()
- keep fd_path when a later lookup reuses the entry for the same inode,
  instead of dropping it in audit_copy_inode()
- place fd_path ahead of name
- use the entry returned by __audit_inode()
---
 kernel/audit.h   |  1 +
 kernel/auditsc.c | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/kernel/audit.h b/kernel/audit.h
index 614afa1cfa14..476821dbed64 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -75,6 +75,7 @@ struct audit_cap_data {
 struct audit_names {
 	struct list_head	list;		/* audit_context->names_list */
 
+	struct path		fd_path;	/* owned audit_file() fallback */
 	struct filename		*name;
 	int			name_len;	/* number of chars to log */
 	dev_t			dev;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index b740e056cd80..31c45f681b13 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -936,6 +936,8 @@ static inline void audit_free_names(struct audit_context *context)
 		list_del(&n->list);
 		if (n->name)
 			putname(n->name);
+		path_put(&n->fd_path);
+		n->fd_path = (struct path) { };
 		if (n->should_free)
 			kfree(n);
 	}
@@ -1531,7 +1533,9 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
 			audit_log_n_untrustedstring(ab, n->name->name,
 						    n->name_len);
 		}
-	} else
+	} else if (n->fd_path.dentry)
+		audit_log_d_path(ab, " name=", &n->fd_path);
+	else
 		audit_log_format(ab, " name=(null)");
 
 	if (n->ino != AUDIT_INO_UNSET)
@@ -2347,7 +2351,15 @@ struct audit_names *__audit_inode(struct filename *name,
 
 void __audit_file(const struct file *file)
 {
-	__audit_inode(NULL, file->f_path.dentry, 0);
+	struct audit_names *n;
+
+	n = __audit_inode(NULL, file->f_path.dentry, 0);
+	if (!n)
+		return;
+
+	/* Resolve at event emission, so renames and unlinks can affect the name. */
+	n->fd_path = file->f_path;
+	path_get(&n->fd_path);
 }
 
 /**
-- 
2.55.0

Re: [PATCH v2 2/3] audit: return the collected entry from __audit_inode()

[sashiko-bot]

> From: Christian Göttsche <cgzones@googlemail.com>
> 
> Return the selected audit_names entry, or NULL when collection is
> skipped or fails, so audit_file() can attach descriptor-specific
> information reliably. The existing audit_inode() and
> audit_inode_parent_hidden() wrappers ignore the result, and collection
> behavior is unchanged.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106150342.670199-1-cgoettsche@seltendoof.de?part=2

Re: [PATCH v2 1/3] audit: compact name entries and context fields

[sashiko-bot]

> From: Christian Göttsche <cgzones@googlemail.com>
> 
> Group the inode metadata and the small scalar fields of audit_names to
> remove padding, while keeping the pathname next to its length and the
> file capability revision next to the file capabilities. Store the
> per-name file capability revision in s16: it must represent both the
> eight-bit on-disk revision and the -1 AUDIT_INODE_NOEVAL sentinel. Keep
> the existing formatter and its unknown-capability output unchanged.
> 
> Move name_count ahead of the embedded name slots to remove two alignment
> holes, and place personality before the task credential scalars.
> Preserve the first int dummy member required by audit_dummy_context(),
> all five embedded name slots, and the existing allocation and reference
> lifetimes.
> 
> On x86_64 with SELinux this reduces audit_context from 928 to 880 bytes,
> leaving room for descriptor paths while staying below 1 KiB. On the tested
> arm64 configuration without property-bearing LSMs it falls to 872 bytes.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106150342.670199-1-cgoettsche@seltendoof.de?part=1

Re: [PATCH v2 3/3] audit: retain file paths for descriptor PATH records

[sashiko-bot]

> From: Christian Göttsche <cgzones@googlemail.com>
> 
> fchown() and the other audit_file() callers collect inode metadata but
> emit name=(null), even though file->f_path is available.
> 
> Retain the path with balanced dentry and mount references, and use the
> existing audit_log_d_path() formatter when no filename was collected.
> Release the references on context cleanup. Descriptor entries carry no
> filename, so they can only be reused by a later lookup that matches the
> same inode; keep the retained path then instead of falling back to
> name=(null). Filesystem exclusions continue to skip entry creation.
> 
> The name reflects d_path() at event emission: concurrent renames and
> unlinks can affect it. Original lookup names retain precedence.
> 
> This adds sizeof(struct path) to each audit_names. With the preceding
> capability and layout changes, audit_context grows from 880 to 960 bytes
> on the tested x86_64 SELinux configuration and remains in the 1 KiB
> kmalloc class. Other LSM configurations can have different object sizes.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106150342.670199-1-cgoettsche@seltendoof.de?part=3