[PATCH v5 4/6] x86/tdx: Add a restriction on access to MMIO address

Alexey Gladkov posted 6 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v5 4/6] x86/tdx: Add a restriction on access to MMIO address
Posted by Alexey Gladkov 1 year, 3 months ago
From: "Alexey Gladkov (Intel)" <legion@kernel.org>

For security reasons, access from kernel space to MMIO addresses in
userspace should be restricted. All MMIO operations from kernel space
are considered trusted and are not validated.

For instance, if in response to a syscall, kernel does put_user() and
the target address is MMIO mapping in userspace, current #VE handler
threat this access as kernel MMIO which is wrong and have security
implications.

Cc: stable@vger.kernel.org
Signed-off-by: Alexey Gladkov (Intel) <legion@kernel.org>
---
 arch/x86/coco/tdx/tdx.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 5d2d07aa08ce..65f65015238a 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -411,6 +411,11 @@ static inline bool is_private_gpa(u64 gpa)
 	return gpa == cc_mkenc(gpa);
 }
 
+static inline bool is_kernel_addr(unsigned long addr)
+{
+	return (long)addr < 0;
+}
+
 static int get_phys_addr(unsigned long addr, phys_addr_t *phys_addr, bool *writable)
 {
 	unsigned int level;
@@ -606,6 +611,11 @@ static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
 	if (WARN_ON_ONCE(mmio == INSN_MMIO_DECODE_FAILED))
 		return -EINVAL;
 
+	if (!user_mode(regs) && !is_kernel_addr(ve->gla)) {
+		WARN_ONCE(1, "Access to userspace address is not supported");
+		return -EINVAL;
+	}
+
 	vaddr = (unsigned long)insn_get_addr_ref(&insn, regs);
 
 	if (user_mode(regs)) {
-- 
2.46.0
Re: [PATCH v5 4/6] x86/tdx: Add a restriction on access to MMIO address
Posted by Kirill A. Shutemov 1 year, 3 months ago
On Wed, Aug 28, 2024 at 12:44:34PM +0200, Alexey Gladkov wrote:
> From: "Alexey Gladkov (Intel)" <legion@kernel.org>
> 
> For security reasons, access from kernel space to MMIO addresses in
> userspace should be restricted. All MMIO operations from kernel space
> are considered trusted and are not validated.
> 
> For instance, if in response to a syscall, kernel does put_user() and
> the target address is MMIO mapping in userspace, current #VE handler
> threat this access as kernel MMIO which is wrong and have security
> implications.

What about this:

------------------------------------8<-----------------------------------

Subject: x86/tdx: Fix "in-kernel MMIO" check

TDX only supports kernel-initiated MMIO operations. The handle_mmio()
function checks if the #VE exception occurred in the kernel and rejects
the operation if it did not.

However, userspace can deceive the kernel into performing MMIO on its
behalf. For example, if userspace can point a syscall to an MMIO address,
syscall does get_user() or put_user() on it, triggering MMIO #VE. The
kernel will treat the #VE as in-kernel MMIO.

Ensure that the target MMIO address is within the kernel before decoding
instruction.

------------------------------------8<-----------------------------------

And please make this patch the first in the patchset. It has to be
backported to stable trees and should have zero dependencies on the rest
of the patchset.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov