linux-coco.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Alexey Gladkov <legion@kernel.org>
To: linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev
Cc: "Alexey Gladkov (Intel)" <legion@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Yuan Yao <yuan.yao@intel.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Yuntao Wang <ytcoode@gmail.com>, Kai Huang <kai.huang@intel.com>,
	Baoquan He <bhe@redhat.com>, Oleg Nesterov <oleg@redhat.com>,
	cho@microsoft.com, decui@microsoft.com,
	John.Starks@microsoft.com
Subject: [PATCH v5 2/6] x86/tdx: Add validation of userspace MMIO instructions
Date: Wed, 28 Aug 2024 12:44:32 +0200	[thread overview]
Message-ID: <3204e85f96694e81817b22bf1d46ab3ff1ee5a91.1724837158.git.legion@kernel.org> (raw)
In-Reply-To: <cover.1724837158.git.legion@kernel.org>

From: "Alexey Gladkov (Intel)" <legion@kernel.org>

Instructions from kernel space are considered trusted. If the MMIO
instruction is from userspace it must be checked.

For userspace instructions, it is need to check that the INSN has not
changed at the time of #VE and before the execution of the instruction.

Once the userspace instruction parsed is enforced that the address
points to mapped memory of current process and that address does not
point to private memory.

After parsing the userspace instruction, it is necessary to ensure that:

1. the operation direction (read/write) corresponds to #VE info;
2. the address still points to mapped memory of current process;
3. the address does not point to private memory.

Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Alexey Gladkov (Intel) <legion@kernel.org>
---
 arch/x86/coco/tdx/tdx.c | 131 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 118 insertions(+), 13 deletions(-)

diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index af0b6c1cacf7..99634e12f9a7 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -8,6 +8,7 @@
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/kexec.h>
+#include <linux/mm.h>
 #include <asm/coco.h>
 #include <asm/tdx.h>
 #include <asm/vmx.h>
@@ -405,6 +406,87 @@ static bool mmio_write(int size, unsigned long addr, unsigned long val)
 			       EPT_WRITE, addr, val);
 }
 
+static inline bool is_private_gpa(u64 gpa)
+{
+	return gpa == cc_mkenc(gpa);
+}
+
+static int get_phys_addr(unsigned long addr, phys_addr_t *phys_addr, bool *writable)
+{
+	unsigned int level;
+	pgd_t *pgdp;
+	pte_t *ptep;
+
+	/*
+	 * Address validation only makes sense for a user process. The lock must
+	 * be obtained before validation can begin.
+	 */
+	mmap_assert_locked(current->mm);
+
+	pgdp = pgd_offset(current->mm, addr);
+
+	if (!pgd_none(*pgdp)) {
+		ptep = lookup_address_in_pgd(pgdp, addr, &level);
+		if (ptep) {
+			unsigned long offset;
+
+			if (!pte_decrypted(*ptep))
+				return -EFAULT;
+
+			offset = addr & ~page_level_mask(level);
+			*phys_addr = PFN_PHYS(pte_pfn(*ptep));
+			*phys_addr |= offset;
+
+			*writable = pte_write(*ptep);
+
+			return 0;
+		}
+	}
+
+	return -EFAULT;
+}
+
+static int valid_vaddr(struct ve_info *ve, enum insn_mmio_type mmio, int size,
+		       unsigned long vaddr)
+{
+	phys_addr_t phys_addr;
+	bool writable = false;
+
+	/* It's not fatal. This can happen due to swap out or page migration. */
+	if (get_phys_addr(vaddr, &phys_addr, &writable) || (ve->gpa != cc_mkdec(phys_addr)))
+		return -EAGAIN;
+
+	/*
+	 * Re-check whether #VE info matches the instruction that was decoded.
+	 *
+	 * The ve->gpa was valid at the time ve_info was received. But this code
+	 * executed with interrupts enabled, allowing tlb shootdown and therefore
+	 * munmap() to be executed in the parallel thread.
+	 *
+	 * By the time MMIO emulation is performed, ve->gpa may be already
+	 * unmapped from the process, the device it belongs to removed from
+	 * system and something else could be plugged in its place.
+	 */
+	switch (mmio) {
+	case INSN_MMIO_WRITE:
+	case INSN_MMIO_WRITE_IMM:
+		if (!writable || !(ve->exit_qual & EPT_VIOLATION_ACC_WRITE))
+			return -EFAULT;
+		break;
+	case INSN_MMIO_READ:
+	case INSN_MMIO_READ_ZERO_EXTEND:
+	case INSN_MMIO_READ_SIGN_EXTEND:
+		if (!(ve->exit_qual & EPT_VIOLATION_ACC_READ))
+			return -EFAULT;
+		break;
+	default:
+		WARN_ONCE(1, "Unsupported mmio instruction: %d", mmio);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int handle_mmio_write(struct insn *insn, enum insn_mmio_type mmio, int size,
 			     struct pt_regs *regs, struct ve_info *ve)
 {
@@ -489,7 +571,7 @@ static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
 	enum insn_mmio_type mmio;
 	struct insn insn = {};
 	unsigned long vaddr;
-	int size;
+	int size, ret;
 
 	/* Only in-kernel MMIO is supported */
 	if (WARN_ON_ONCE(user_mode(regs)))
@@ -505,6 +587,17 @@ static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
 	if (WARN_ON_ONCE(mmio == INSN_MMIO_DECODE_FAILED))
 		return -EINVAL;
 
+	vaddr = (unsigned long)insn_get_addr_ref(&insn, regs);
+
+	if (user_mode(regs)) {
+		if (mmap_read_lock_killable(current->mm))
+			return -EINTR;
+
+		ret = valid_vaddr(ve, mmio, size, vaddr);
+		if (ret)
+			goto unlock;
+	}
+
 	/*
 	 * Reject EPT violation #VEs that split pages.
 	 *
@@ -514,30 +607,39 @@ static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
 	 *
 	 * load_unaligned_zeropad() will recover using exception fixups.
 	 */
-	vaddr = (unsigned long)insn_get_addr_ref(&insn, regs);
-	if (vaddr / PAGE_SIZE != (vaddr + size - 1) / PAGE_SIZE)
-		return -EFAULT;
+	if (vaddr / PAGE_SIZE != (vaddr + size - 1) / PAGE_SIZE) {
+		ret = -EFAULT;
+		goto unlock;
+	}
 
 	switch (mmio) {
 	case INSN_MMIO_WRITE:
 	case INSN_MMIO_WRITE_IMM:
 	case INSN_MMIO_MOVS:
-		return handle_mmio_write(&insn, mmio, size, regs, ve);
+		ret = handle_mmio_write(&insn, mmio, size, regs, ve);
+		break;
 	case INSN_MMIO_READ:
 	case INSN_MMIO_READ_ZERO_EXTEND:
 	case INSN_MMIO_READ_SIGN_EXTEND:
-		return handle_mmio_read(&insn, mmio, size, regs, ve);
+		ret = handle_mmio_read(&insn, mmio, size, regs, ve);
+		break;
 	case INSN_MMIO_DECODE_FAILED:
 		/*
 		 * MMIO was accessed with an instruction that could not be
 		 * decoded or handled properly. It was likely not using io.h
 		 * helpers or accessed MMIO accidentally.
 		 */
-		return -EINVAL;
+		ret = -EINVAL;
+		break;
 	default:
 		WARN_ONCE(1, "Unknown insn_decode_mmio() decode value?");
-		return -EINVAL;
+		ret = -EINVAL;
 	}
+unlock:
+	if (user_mode(regs))
+		mmap_read_unlock(current->mm);
+
+	return ret;
 }
 
 static bool handle_in(struct pt_regs *regs, int size, int port)
@@ -681,11 +783,6 @@ static int virt_exception_user(struct pt_regs *regs, struct ve_info *ve)
 	}
 }
 
-static inline bool is_private_gpa(u64 gpa)
-{
-	return gpa == cc_mkenc(gpa);
-}
-
 /*
  * Handle the kernel #VE.
  *
@@ -723,6 +820,14 @@ bool tdx_handle_virt_exception(struct pt_regs *regs, struct ve_info *ve)
 		insn_len = virt_exception_user(regs, ve);
 	else
 		insn_len = virt_exception_kernel(regs, ve);
+
+	/*
+	 * A special case to return to userspace without increasing regs->ip
+	 * to repeat the instruction once again.
+	 */
+	if (insn_len == -EAGAIN)
+		return true;
+
 	if (insn_len < 0)
 		return false;
 
-- 
2.46.0


  parent reply	other threads:[~2024-08-28 10:45 UTC|newest]

Thread overview: 109+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-30 17:35 [PATCH v1 0/4] x86/tdx: Allow MMIO instructions from userspace Alexey Gladkov (Intel)
2024-07-30 17:35 ` [PATCH v1 1/4] x86/tdx: Split MMIO read and write operations Alexey Gladkov (Intel)
2024-07-30 18:31   ` Thomas Gleixner
2024-08-05 12:48     ` Alexey Gladkov
2024-07-30 17:35 ` [PATCH v1 2/4] x86/tdx: Add validation of userspace MMIO instructions Alexey Gladkov (Intel)
2024-07-30 18:34   ` Thomas Gleixner
2024-08-02  7:41   ` Kirill A. Shutemov
2024-08-05 12:50     ` Alexey Gladkov
2024-07-30 17:35 ` [PATCH v1 3/4] x86/tdx: Allow MMIO from userspace Alexey Gladkov (Intel)
2024-07-30 18:36   ` Thomas Gleixner
2024-07-30 17:35 ` [PATCH v1 4/4] x86/tdx: Implement movs for MMIO Alexey Gladkov (Intel)
2024-07-30 18:41   ` Thomas Gleixner
2024-08-05 12:51     ` Alexey Gladkov
2024-08-05 13:29 ` [PATCH v2 0/5] x86/tdx: Allow MMIO instructions from userspace Alexey Gladkov (Intel)
2024-08-05 13:29   ` [PATCH v2 1/5] x86/tdx: Split MMIO read and write operations Alexey Gladkov (Intel)
2024-08-05 13:29   ` [PATCH v2 2/5] x86/tdx: Add validation of userspace MMIO instructions Alexey Gladkov (Intel)
2024-08-05 22:40     ` Edgecombe, Rick P
2024-08-06  7:18       ` kirill.shutemov
2024-08-06 11:11         ` Alexey Gladkov
2024-08-06 11:41           ` Reshetova, Elena
2024-08-08 15:56             ` Alexey Gladkov
2024-08-08 15:53       ` Alexey Gladkov
2024-08-08 15:42     ` [PATCH v3 6/7] x86/tdx: Add a restriction on access to MMIO address Alexey Gladkov (Intel)
2024-08-08 15:42     ` [PATCH v3 7/7] x86/tdx: Avoid crossing the page boundary Alexey Gladkov (Intel)
2024-08-05 13:29   ` [PATCH v2 3/5] x86/tdx: Allow MMIO from userspace Alexey Gladkov (Intel)
2024-08-05 13:29   ` [PATCH v2 4/5] x86/tdx: Move MMIO helpers to common library Alexey Gladkov (Intel)
2024-08-05 13:29   ` [PATCH v2 5/5] x86/tdx: Implement movs for MMIO Alexey Gladkov (Intel)
2024-08-08 13:48     ` Tom Lendacky
2024-08-08 15:42       ` Alexey Gladkov
2024-08-08 16:53       ` Alexey Gladkov
2024-08-16 13:43   ` [PATCH v3 00/10] x86/tdx: Allow MMIO instructions from userspace Alexey Gladkov
2024-08-16 13:43     ` [PATCH v3 01/10] x86/tdx: Split MMIO read and write operations Alexey Gladkov
2024-08-19 10:19       ` Kirill A. Shutemov
2024-08-16 13:43     ` [PATCH v3 02/10] x86/tdx: Add validation of userspace MMIO instructions Alexey Gladkov
2024-08-19 10:39       ` Kirill A. Shutemov
2024-08-19 11:48         ` Alexey Gladkov
2024-08-19 12:07           ` Kirill A. Shutemov
2024-08-19 12:39             ` Alexey Gladkov
2024-08-16 13:43     ` [PATCH v3 03/10] x86/tdx: Allow MMIO from userspace Alexey Gladkov
2024-08-19 10:46       ` Kirill A. Shutemov
2024-08-19 11:50         ` Alexey Gladkov
2024-08-16 13:43     ` [PATCH v3 04/10] x86/insn: Read and decode insn without crossing the page boundary Alexey Gladkov
2024-08-17  3:28       ` kernel test robot
2024-08-19 10:48       ` Kirill A. Shutemov
2024-08-19 11:56         ` Alexey Gladkov
2024-08-19 12:08           ` Kirill A. Shutemov
2024-08-16 13:43     ` [PATCH v3 05/10] x86/tdx: Avoid " Alexey Gladkov
2024-08-16 13:43     ` [PATCH v3 06/10] x86/sev: " Alexey Gladkov
2024-08-16 13:43     ` [PATCH v3 07/10] x86/umip: " Alexey Gladkov
2024-08-16 13:43     ` [PATCH v3 08/10] x86/tdx: Add a restriction on access to MMIO address Alexey Gladkov
2024-08-16 13:43     ` [PATCH v3 09/10] x86/tdx: Move MMIO helpers to common library Alexey Gladkov
2024-08-16 13:44     ` [PATCH v3 10/10] x86/tdx: Implement movs for MMIO Alexey Gladkov
2024-08-21 14:24     ` [PATCH v4 0/6] x86/tdx: Allow MMIO instructions from userspace Alexey Gladkov
2024-08-21 14:24       ` [PATCH v4 1/6] x86/tdx: Split MMIO read and write operations Alexey Gladkov
2024-08-21 14:24       ` [PATCH v4 2/6] x86/tdx: Add validation of userspace MMIO instructions Alexey Gladkov
2024-08-22  7:16         ` Kirill A. Shutemov
2024-08-21 14:24       ` [PATCH v4 3/6] x86/tdx: Allow MMIO from userspace Alexey Gladkov
2024-08-22  7:18         ` Kirill A. Shutemov
2024-08-21 14:24       ` [PATCH v4 4/6] x86/tdx: Add a restriction on access to MMIO address Alexey Gladkov
2024-08-22  8:18         ` Kirill A. Shutemov
2024-08-21 14:24       ` [PATCH v4 5/6] x86/tdx: Move MMIO helpers to common library Alexey Gladkov
2024-08-22  8:23         ` Kirill A. Shutemov
2024-08-21 14:24       ` [PATCH v4 6/6] x86/tdx: Implement movs for MMIO Alexey Gladkov
2024-08-22  8:28         ` Kirill A. Shutemov
2024-08-24 16:57           ` Alexey Gladkov
2024-08-28 10:44       ` [PATCH v5 0/6] x86/tdx: Allow MMIO instructions from userspace Alexey Gladkov
2024-08-28 10:44         ` [PATCH v5 1/6] x86/tdx: Split MMIO read and write operations Alexey Gladkov
2024-08-28 10:44         ` Alexey Gladkov [this message]
2024-08-28 10:44         ` [PATCH v5 3/6] x86/tdx: Allow MMIO from userspace Alexey Gladkov
2024-08-28 10:44         ` [PATCH v5 4/6] x86/tdx: Add a restriction on access to MMIO address Alexey Gladkov
2024-08-29 12:30           ` Kirill A. Shutemov
2024-08-28 10:44         ` [PATCH v5 5/6] x86/tdx: Move MMIO helpers to common library Alexey Gladkov
2024-08-28 10:44         ` [PATCH v5 6/6] x86/tdx: Implement MOVS for MMIO Alexey Gladkov
2024-08-29 12:44           ` Kirill A. Shutemov
2024-08-29 18:40             ` Alexey Gladkov
2024-09-09  9:17               ` Kirill A. Shutemov
2024-09-06 11:49         ` [PATCH v6 0/6] x86/tdx: Allow MMIO instructions from userspace Alexey Gladkov
2024-09-06 11:49           ` [PATCH v6 1/6] x86/tdx: Fix "in-kernel MMIO" check Alexey Gladkov
2024-09-10 19:54             ` Dave Hansen
2024-09-11 12:08               ` Alexey Gladkov
2024-09-11 13:03                 ` Kirill A. Shutemov
2024-09-10 19:59             ` Kirill A. Shutemov
2024-09-06 11:50           ` [PATCH v6 2/6] x86/tdx: Split MMIO read and write operations Alexey Gladkov
2024-09-06 11:50           ` [PATCH v6 3/6] x86/tdx: Add validation of userspace MMIO instructions Alexey Gladkov
2024-09-06 11:50           ` [PATCH v6 4/6] x86/tdx: Allow MMIO from userspace Alexey Gladkov
2024-09-06 11:50           ` [PATCH v6 5/6] x86/tdx: Move MMIO helpers to common library Alexey Gladkov
2024-09-09  9:19             ` Kirill A. Shutemov
2024-09-06 11:50           ` [PATCH v6 6/6] x86/tdx: Implement MOVS for MMIO Alexey Gladkov
2024-09-09  9:24             ` Kirill A. Shutemov
2024-09-06 16:19           ` [PATCH v6 0/6] x86/tdx: Allow MMIO instructions from userspace Dave Hansen
2024-09-06 21:13             ` Sean Christopherson
2024-09-11 15:38               ` Dave Hansen
2024-09-11 16:19                 ` Sean Christopherson
2024-09-12  9:45                   ` Kirill A. Shutemov
2024-09-12 15:49                     ` Dave Hansen
2024-09-13 15:53                       ` Kirill A. Shutemov
2024-09-13 16:01                         ` Dave Hansen
2024-09-13 16:28                           ` Sean Christopherson
2024-09-13 16:47                             ` Dave Hansen
2024-09-13 17:39                               ` Sean Christopherson
2024-09-13 17:05           ` [PATCH v7 " Alexey Gladkov
2024-09-13 17:05             ` [PATCH v7 1/6] x86/tdx: Fix "in-kernel MMIO" check Alexey Gladkov
2024-09-13 17:18               ` Dave Hansen
2024-09-13 17:23                 ` Dave Hansen
2024-09-13 17:05             ` [PATCH v7 2/6] x86/tdx: Split MMIO read and write operations Alexey Gladkov
2024-09-13 17:05             ` [PATCH v7 3/6] x86/tdx: Add validation of userspace MMIO instructions Alexey Gladkov
2024-09-13 17:05             ` [PATCH v7 4/6] x86/tdx: Allow MMIO from userspace Alexey Gladkov
2024-09-13 17:06             ` [PATCH v7 5/6] x86/tdx: Move MMIO helpers to common library Alexey Gladkov
2024-09-13 17:06             ` [PATCH v7 6/6] x86/tdx: Implement MOVS for MMIO Alexey Gladkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3204e85f96694e81817b22bf1d46ab3ff1ee5a91.1724837158.git.legion@kernel.org \
    --to=legion@kernel.org \
    --cc=John.Starks@microsoft.com \
    --cc=akpm@linux-foundation.org \
    --cc=bhe@redhat.com \
    --cc=bp@alien8.de \
    --cc=cho@microsoft.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=decui@microsoft.com \
    --cc=geert@linux-m68k.org \
    --cc=hpa@zytor.com \
    --cc=kai.huang@intel.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=ytcoode@gmail.com \
    --cc=yuan.yao@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).