[RFC PATCH] /dev/mem: Disable /dev/mem under TDX guest

[Nikolay Borisov]

If a piece of memory is read from /dev/mem that falls outside of the
System Ram region i.e bios data region the kernel creates a shared
mapping via xlate_dev_mem_ptr() (this behavior was introduced by
9aa6ea69852c ("x86/tdx: Make pages shared in ioremap()"). This results
in a region having both a shared and a private mapping.

Subsequent accesses to this region via the private mapping induce a
SEPT violation and a crash of the VMM. In this particular case the
scenario was a userspace process reading something from the bios data
area at address 0x497 which creates a shared mapping, and a followup
reboot accessing __va(0x472) which access pfn 0 via the private mapping
causing mayhem.

Fix this by simply forbidding access to /dev/mem when running as an TDX
guest.

Signed-off-by: Nikolay Borisov <nik.borisov@xxxxxxxx>
---

Sending this now to hopefully spur up discussion as to how to handle the described
scenario. This was hit on the GCP cloud and was causing their hypervisor to crash.

I guess the most pressing question is what will be the most sensible approach to
eliminate such situations happening in the future:

1. Should we forbid getting a descriptor to /dev/mem (this patch)
2. Skip creating /dev/mem altogether3
3. Possibly tinker with internals of ioremap to ensure that no memory which is
backed by kvm memslots is remapped as shared.
4. Eliminate the access to 0x472 from the x86 reboot path, after all we don't
really have a proper bios at that address.
5. Something else ?

 arch/x86/coco/tdx/tdx.c  | 4 ++++
 drivers/char/mem.c       | 3 +++
 include/linux/security.h | 6 ++++++
 3 files changed, 13 insertions(+)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 32809a06dab4..615e8a300fc7 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -40,6 +40,8 @@

 static atomic_long_t nr_shared;

+bool devmem_disabled = false;
+
 /* Called from __tdx_hypercall() for unrecoverable failure */
 noinstr void __noreturn __tdx_hypercall_failed(void)
 {
@@ -1063,6 +1065,8 @@ void __init tdx_early_init(void)

 	setup_force_cpu_cap(X86_FEATURE_TDX_GUEST);

+	devmem_disabled = true;
+
 	/* TSC is the only reliable clock in TDX guest */
 	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 169eed162a7f..8778d46216f2 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -616,6 +616,9 @@ static int open_port(struct inode *inode, struct file *filp)
 	if (iminor(inode) != DEVMEM_MINOR)
 		return 0;

+	if (devmem_disabled)
+		return -EINVAL;
+
 	/*
 	 * Use a unified address space to have a single point to manage
 	 * revocations when drivers want to take over a /dev/mem mapped
diff --git a/include/linux/security.h b/include/linux/security.h
index 980b6c207cad..1757f683a09d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -265,6 +265,12 @@ struct request_sock;
 #define LSM_UNSAFE_PTRACE	2
 #define LSM_UNSAFE_NO_NEW_PRIVS	4

+#ifdef CONFIG_INTEL_TDX_GUEST
+extern bool devmem_disabled;
+#else
+#define devmem_disabled 0
+#endif
+
 #ifdef CONFIG_MMU
 extern int mmap_min_addr_handler(const struct ctl_table *table, int write,
 				 void *buffer, size_t *lenp, loff_t *ppos);
--
2.43.0