| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f65bf8705f1e3a6e5055773cf3831f3dc85cd7d0.1725622408.git.legion@kernel.org>
Date: Fri, 6 Sep 2024 13:50:00 +0200
From: Alexey Gladkov <legion@...nel.org>
To: linux-kernel@...r.kernel.org,
linux-coco@...ts.linux.dev
Cc: "Alexey Gladkov (Intel)" <legion@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Yuan Yao <yuan.yao@...el.com>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Yuntao Wang <ytcoode@...il.com>,
Kai Huang <kai.huang@...el.com>,
Baoquan He <bhe@...hat.com>,
Oleg Nesterov <oleg@...hat.com>,
cho@...rosoft.com,
decui@...rosoft.com,
John.Starks@...rosoft.com
Subject: [PATCH v6 2/6] x86/tdx: Split MMIO read and write operations
From: "Alexey Gladkov (Intel)" <legion@...nel.org>
To implement MMIO in userspace, additional memory checks need to be
implemented. To avoid overly complicating the handle_mmio() function
and to separate checks from actions, it would be better to split this
function into two separate functions to handle read and write
operations.
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
Signed-off-by: Alexey Gladkov (Intel) <legion@...nel.org>
---
arch/x86/coco/tdx/tdx.c | 136 ++++++++++++++++++++++++----------------
1 file changed, 83 insertions(+), 53 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index c90d2fdb5fc4..eee97dff1eca 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -410,14 +410,91 @@ static inline bool is_kernel_addr(unsigned long addr)
return (long)addr < 0;
}
+static int handle_mmio_write(struct insn *insn, enum insn_mmio_type mmio, int size,
+ struct pt_regs *regs, struct ve_info *ve)
+{
+ unsigned long *reg, val;
+
+ switch (mmio) {
+ case INSN_MMIO_WRITE:
+ reg = insn_get_modrm_reg_ptr(insn, regs);
+ if (!reg)
+ return -EINVAL;
+ memcpy(&val, reg, size);
+ if (!mmio_write(size, ve->gpa, val))
+ return -EIO;
+ return insn->length;
+ case INSN_MMIO_WRITE_IMM:
+ val = insn->immediate.value;
+ if (!mmio_write(size, ve->gpa, val))
+ return -EIO;
+ return insn->length;
+ case INSN_MMIO_MOVS:
+ /*
+ * 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;
+ default:
+ WARN_ON_ONCE(1);
+ return -EINVAL;
+ }
+
+ return insn->length;
+}
+
+static int handle_mmio_read(struct insn *insn, enum insn_mmio_type mmio, int size,
+ struct pt_regs *regs, struct ve_info *ve)
+{
+ unsigned long *reg, val;
+ int extend_size;
+ u8 extend_val;
+
+ reg = insn_get_modrm_reg_ptr(insn, regs);
+ if (!reg)
+ return -EINVAL;
+
+ if (!mmio_read(size, ve->gpa, &val))
+ return -EIO;
+
+ extend_val = 0;
+
+ switch (mmio) {
+ case INSN_MMIO_READ:
+ /* Zero-extend for 32-bit operation */
+ extend_size = size == 4 ? sizeof(*reg) : 0;
+ break;
+ case INSN_MMIO_READ_ZERO_EXTEND:
+ /* Zero extend based on operand size */
+ extend_size = insn->opnd_bytes;
+ break;
+ case INSN_MMIO_READ_SIGN_EXTEND:
+ /* Sign extend based on operand size */
+ extend_size = insn->opnd_bytes;
+ if (size == 1 && val & BIT(7))
+ extend_val = 0xFF;
+ else if (size > 1 && val & BIT(15))
+ extend_val = 0xFF;
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ return -EINVAL;
+ }
+
+ if (extend_size)
+ memset(reg, extend_val, extend_size);
+ memcpy(reg, &val, size);
+ return insn->length;
+}
+
static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
{
- unsigned long *reg, val, vaddr;
char buffer[MAX_INSN_SIZE];
enum insn_mmio_type mmio;
struct insn insn = {};
- int size, extend_size;
- u8 extend_val = 0;
+ unsigned long vaddr;
+ int size;
/* Only in-kernel MMIO is supported */
if (WARN_ON_ONCE(user_mode(regs)))
@@ -433,12 +510,6 @@ static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
if (WARN_ON_ONCE(mmio == INSN_MMIO_DECODE_FAILED))
return -EINVAL;
- if (mmio != INSN_MMIO_WRITE_IMM && mmio != INSN_MMIO_MOVS) {
- reg = insn_get_modrm_reg_ptr(&insn, regs);
- if (!reg)
- return -EINVAL;
- }
-
if (!user_mode(regs) && !is_kernel_addr(ve->gla)) {
WARN_ONCE(1, "Access to userspace address is not supported");
return -EINVAL;
@@ -457,24 +528,15 @@ static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
if (vaddr / PAGE_SIZE != (vaddr + size - 1) / PAGE_SIZE)
return -EFAULT;
- /* Handle writes first */
switch (mmio) {
case INSN_MMIO_WRITE:
- memcpy(&val, reg, size);
- if (!mmio_write(size, ve->gpa, val))
- return -EIO;
- return insn.length;
case INSN_MMIO_WRITE_IMM:
- val = insn.immediate.value;
- if (!mmio_write(size, ve->gpa, val))
- return -EIO;
- return insn.length;
+ case INSN_MMIO_MOVS:
+ return handle_mmio_write(&insn, mmio, size, regs, ve);
case INSN_MMIO_READ:
case INSN_MMIO_READ_ZERO_EXTEND:
case INSN_MMIO_READ_SIGN_EXTEND:
- /* Reads are handled below */
- break;
- case INSN_MMIO_MOVS:
+ return handle_mmio_read(&insn, mmio, size, regs, ve);
case INSN_MMIO_DECODE_FAILED:
/*
* MMIO was accessed with an instruction that could not be
@@ -486,38 +548,6 @@ static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
WARN_ONCE(1, "Unknown insn_decode_mmio() decode value?");
return -EINVAL;
}
-
- /* Handle reads */
- if (!mmio_read(size, ve->gpa, &val))
- return -EIO;
-
- switch (mmio) {
- case INSN_MMIO_READ:
- /* Zero-extend for 32-bit operation */
- extend_size = size == 4 ? sizeof(*reg) : 0;
- break;
- case INSN_MMIO_READ_ZERO_EXTEND:
- /* Zero extend based on operand size */
- extend_size = insn.opnd_bytes;
- break;
- case INSN_MMIO_READ_SIGN_EXTEND:
- /* Sign extend based on operand size */
- extend_size = insn.opnd_bytes;
- if (size == 1 && val & BIT(7))
- extend_val = 0xFF;
- else if (size > 1 && val & BIT(15))
- extend_val = 0xFF;
- break;
- default:
- /* All other cases has to be covered with the first switch() */
- WARN_ON_ONCE(1);
- return -EINVAL;
- }
-
- if (extend_size)
- memset(reg, extend_val, extend_size);
- memcpy(reg, &val, size);
- return insn.length;
}
static bool handle_in(struct pt_regs *regs, int size, int port)
--
2.46.0
Powered by blists - more mailing lists