Semantics of blktrace with lockdown (integrity) enabled kernel.

[Konrad Rzeszutek Wilk]

Hey Jens, Paul, James, Nathan,

We are trying to use blktrace with a kernel that has lockdown enabled and find that it cannot run.

Specifically the issue is that we are trying to do is pretty simple:

strace -f blktrace -d /dev/sda -w 60
 
[pid 148882] <... mprotect resumed>)    = 0
[pid 148881] openat(AT_FDCWD, "/sys/kernel/debug/block/sda/trace0", O_RDONLY|O_NONBLOCK <unfinished ...>
[pid 148882] sched_setaffinity(0, 8, [1]) = 0
[pid 148881] <... openat resumed>)      = -1 EPERM (Operation not permitted)

which fails. The analysis from Eric (CCed) is that 

All debugfs entries do not exist until blktrace is run.  It is opening
/sys/kernel/debug/block/sda/trace0 which isn’t there normally. While running the utility, 
to place something in it, it must have the write permission set.  When exiting out of 
blktrace, the entry is gone, both on a machine running with secure boot enabled 
and one with it disabled.  Which also indicates the write permission was set, 
otherwise the entry would still be there.

The fix is simple enough (see attachment) but we are not sure about the semantics of what
lockdown has in mind.

Looking at the include/linux/security.h the LOCKDOWN_TRACEFS exists which would
imply that it is expected that operations with tracefs *should* work with lockdown (integrity mode).

But at the same point, debugfs writable attributes are a nono with lockdown.

So what is the right way forward?

Thank you.
-------------- next part --------------
>From 20bd7b8c91463191924ec69833bbd6e6a6231f52 Mon Sep 17 00:00:00 2001
From: Junxiao Bi <junxiao.bi at oracle.com>
Date: Tue, 4 Apr 2023 19:13:21 -0700
Subject: [PATCH] debugfs: whitelisted relay file for lockdown

Relay files in debugfs are used for sending data from kernel to userspace,
the permission of these files are 0444, looks safe to skip lockdown.

Signed-off-by: Junxiao Bi <junxiao.bi at oracle.com>
---
 fs/debugfs/file.c     | 17 +++++++++++++++++
 fs/debugfs/internal.h |  5 +++++
 2 files changed, 22 insertions(+)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index d574bda24e21..93ab719d8c7b 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -20,6 +20,7 @@
 #include <linux/device.h>
 #include <linux/poll.h>
 #include <linux/security.h>
+#include <linux/relay.h>
 
 #include "internal.h"
 
@@ -137,6 +138,22 @@ void debugfs_file_put(struct dentry *dentry)
 }
 EXPORT_SYMBOL_GPL(debugfs_file_put);
 
+bool debugfs_file_is_relay(struct dentry *dentry)
+{
+	struct debugfs_fsdata *fsd;
+	void *d_fsd;
+	void *fops;
+
+	d_fsd = READ_ONCE(dentry->d_fsdata);
+	if (!((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) {
+		fsd = d_fsd;
+		fops = (void *)fsd->real_fops;
+	} else
+		fops = (void *)((unsigned long)d_fsd &
+					~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
+    return fops == (void *)&relay_file_operations;
+}
+
 /*
  * Only permit access to world-readable files when the kernel is locked down.
  * We also need to exclude any file that has ways to write or alter it as root
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 6bcedb3f90b3..392bb1972226 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -37,6 +37,7 @@ static const char * const arch_whitelist[] = {
 	"mds_user_clear"
 };
 
+extern bool debugfs_file_is_relay(struct dentry *dentry);
 struct dentry *__attribute__((weak))get_arch_debugfs_dir(void) {return NULL; }
 
 static bool debugfs_lockdown_whitelisted(struct dentry *dentry)
@@ -51,6 +52,10 @@ static bool debugfs_lockdown_whitelisted(struct dentry *dentry)
 		}
 	}
 
+	/* relay file is used for userspace/kernel communicate.*/
+	if (debugfs_file_is_relay(dentry))
+		return true;
+
 	return false;
 }
 
-- 
2.24.3 (Apple Git-128)