[PATCH v2] lsm: don't call d_path() when current->fs is NULL

[PATCH v2] lsm: don't call d_path() when current->fs is NULL

[Drif Abdelmalek Mohamed Said]

When the last reference to a file is put from exit_task_work(), the task
has already run exit_fs(), so current->fs is NULL.

syzbot reported:

Call trace:
 get_fs_root_rcu fs/d_path.c:244 [inline]
 d_path+0x224/0x4dc fs/d_path.c:287
 audit_log_d_path+0xdc/0x19c kernel/audit.c:2221
 audit_log_lsm_data+0xe0/0x12d0 security/lsm_audit.c:199
 dump_common_audit_data security/lsm_audit.c:421 [inline]
...
 security_file_open+0xec/0x258 security/security.c:2739
 do_dentry_open+0x2a8/0xfbc fs/open.c:973
 vfs_open+0x44/0x2dc fs/open.c:1101
 dentry_open+0x74/0xc0 fs/open.c:1124
...
 security_file_release+0x84/0x148 security/security.c:2424
 __fput+0x2c4/0x74c fs/file_table.c:506
 ____fput+0x20/0x30 fs/file_table.c:541
 task_work_run+0x1cc/0x25c kernel/task_work.c:233
 exit_task_work include/linux/task_work.h:40 [inline]
 do_exit+0x58c/0x1a74 kernel/exit.c:986

d_path() renders a path relative to current->fs->root.  After exit_fs()
there is no such root, and no substitute root is meaningful: the
record's pid/comm describe a task that can no longer name the file, and
the file may have been opened by a different task entirely.  Falling
back to another root would produce a well-formed but misleading path,
so report the field as path=?

Handle this in audit_log_lsm_data(), where the meaning of the field is
known, for LSM_AUDIT_DATA_PATH (Smack file_open), LSM_AUDIT_DATA_FILE
(SELinux file_open), LSM_AUDIT_DATA_IOCTL_OP and the AF_UNIX path in
LSM_AUDIT_DATA_NET.

Fixes: 4a9d4b024a31 ("switch fput to task_work_add")
Reported-by: syzbot+975ace299d6ba2eb1cea@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=975ace299d6ba2eb1cea
Signed-off-by: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@gmail.com>
---
v2:
  - Replaced logging from path=(null) to path=?
  - Added audit_log_lsm_inode helper
v1: https://lore.kernel.org/all/20261019222858.1766-1-drifabdelmalekmohamedsaid@gmail.com/
 security/lsm_audit.c | 70 ++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 39 deletions(-)

diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 955b2e7b2c8e..d92b0823e871 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -169,6 +169,27 @@ static inline void print_ipv4_addr(struct audit_buffer *ab, __be32 addr,
 		audit_log_format(ab, " %s=%d", name2, ntohs(port));
 }
 
+static void audit_log_lsm_inode(struct audit_buffer *ab, struct inode *inode)
+{
+	if (!inode)
+		return;
+
+	audit_log_format(ab, " dev=");
+	audit_log_untrustedstring(ab, inode->i_sb->s_id);
+	audit_log_format(ab, " ino=%llu", inode->i_ino);
+}
+
+static void audit_log_lsm_path(struct audit_buffer *ab, struct inode *inode,
+			       const struct path *path)
+{
+	if (current->fs)
+		audit_log_d_path(ab, " path=", path);
+	else
+		audit_log_format(ab, " path=?");
+
+	audit_log_lsm_inode(ab, inode);
+}
+
 /**
  * audit_log_lsm_data - helper to log common LSM audit data
  * @ab : the audit buffer
@@ -194,60 +215,33 @@ void audit_log_lsm_data(struct audit_buffer *ab,
 		audit_log_format(ab, " capability=%d ", a->u.cap);
 		break;
 	case LSM_AUDIT_DATA_PATH: {
-		struct inode *inode;
-
-		audit_log_d_path(ab, " path=", &a->u.path);
+		struct inode *inode = d_backing_inode(a->u.path.dentry);
 
-		inode = d_backing_inode(a->u.path.dentry);
-		if (inode) {
-			audit_log_format(ab, " dev=");
-			audit_log_untrustedstring(ab, inode->i_sb->s_id);
-			audit_log_format(ab, " ino=%llu", inode->i_ino);
-		}
+		audit_log_lsm_path(ab, inode, &a->u.path);
 		break;
 	}
 	case LSM_AUDIT_DATA_FILE: {
-		struct inode *inode;
-
-		audit_log_d_path(ab, " path=", &a->u.file->f_path);
+		struct inode *inode = file_inode(a->u.file);
 
-		inode = file_inode(a->u.file);
-		if (inode) {
-			audit_log_format(ab, " dev=");
-			audit_log_untrustedstring(ab, inode->i_sb->s_id);
-			audit_log_format(ab, " ino=%llu", inode->i_ino);
-		}
+		audit_log_lsm_path(ab, inode, &a->u.file->f_path);
 		break;
 	}
 	case LSM_AUDIT_DATA_IOCTL_OP: {
-		struct inode *inode;
-
-		audit_log_d_path(ab, " path=", &a->u.op->path);
-
-		inode = a->u.op->path.dentry->d_inode;
-		if (inode) {
-			audit_log_format(ab, " dev=");
-			audit_log_untrustedstring(ab, inode->i_sb->s_id);
-			audit_log_format(ab, " ino=%llu", inode->i_ino);
-		}
+		struct inode *inode = a->u.op->path.dentry->d_inode;
 
+		audit_log_lsm_path(ab, inode, &a->u.op->path);
 		audit_log_format(ab, " ioctlcmd=0x%x", a->u.op->cmd);
 		break;
 	}
 	case LSM_AUDIT_DATA_DENTRY: {
-		struct inode *inode;
+		struct inode *inode = d_backing_inode(a->u.dentry);
 
 		audit_log_format(ab, " name=");
 		spin_lock(&a->u.dentry->d_lock);
 		audit_log_untrustedstring(ab, a->u.dentry->d_name.name);
 		spin_unlock(&a->u.dentry->d_lock);
 
-		inode = d_backing_inode(a->u.dentry);
-		if (inode) {
-			audit_log_format(ab, " dev=");
-			audit_log_untrustedstring(ab, inode->i_sb->s_id);
-			audit_log_format(ab, " ino=%llu", inode->i_ino);
-		}
+		audit_log_lsm_inode(ab, inode);
 		break;
 	}
 	case LSM_AUDIT_DATA_INODE: {
@@ -263,9 +257,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
 			audit_log_untrustedstring(ab, dentry->d_name.name);
 			spin_unlock(&dentry->d_lock);
 		}
-		audit_log_format(ab, " dev=");
-		audit_log_untrustedstring(ab, inode->i_sb->s_id);
-		audit_log_format(ab, " ino=%llu", inode->i_ino);
+		audit_log_lsm_inode(ab, inode);
 		rcu_read_unlock();
 		break;
 	}
@@ -321,7 +313,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
 				if (!addr)
 					break;
 				if (u->path.dentry) {
-					audit_log_d_path(ab, " path=", &u->path);
+					audit_log_lsm_path(ab, NULL, &u->path);
 					break;
 				}
 				len = addr->len-sizeof(short);
-- 
2.43.0

Re: [PATCH v2] lsm: don't call d_path() when current->fs is NULL

[sashiko-bot]

> When the last reference to a file is put from exit_task_work(), the task
> has already run exit_fs(), so current->fs is NULL.
> 
> syzbot reported:
> 
> Call trace:
>  get_fs_root_rcu fs/d_path.c:244 [inline]
>  d_path+0x224/0x4dc fs/d_path.c:287
>  audit_log_d_path+0xdc/0x19c kernel/audit.c:2221
>  audit_log_lsm_data+0xe0/0x12d0 security/lsm_audit.c:199
>  dump_common_audit_data security/lsm_audit.c:421 [inline]
> ...
>  security_file_open+0xec/0x258 security/security.c:2739
>  do_dentry_open+0x2a8/0xfbc fs/open.c:973
>  vfs_open+0x44/0x2dc fs/open.c:1101
> [ ... ]
> 
> Fixes: 4a9d4b024a31 ("switch fput to task_work_add")
> Reported-by: syzbot+975ace299d6ba2eb1cea@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=975ace299d6ba2eb1cea
> Signed-off-by: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106111016.5783-1-drifabdelmalekmohamedsaid@gmail.com?part=1