linux-coco.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Joerg Roedel <joro@8bytes.org>
To: x86@kernel.org, Hyunwook Baek <baekhw@google.com>
Cc: Joerg Roedel <joro@8bytes.org>, Joerg Roedel <jroedel@suse.de>,
	hpa@zytor.com, Andy Lutomirski <luto@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Jiri Slaby <jslaby@suse.cz>,
	Dan Williams <dan.j.williams@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Juergen Gross <jgross@suse.com>,
	Kees Cook <keescook@chromium.org>,
	David Rientjes <rientjes@google.com>,
	Cfir Cohen <cfir@google.com>, Erdem Aktas <erdemaktas@google.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Mike Stunes <mstunes@vmware.com>,
	Sean Christopherson <seanjc@google.com>,
	Martin Radev <martin.b.radev@gmail.com>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	linux-coco@lists.linux.dev, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, virtualization@lists.linux-foundation.org
Subject: [PATCH v2 7/8] x86/insn: Extend error reporting from insn_fetch_from_user[_inatomic]()
Date: Wed, 19 May 2021 15:52:50 +0200	[thread overview]
Message-ID: <20210519135251.30093-8-joro@8bytes.org> (raw)
In-Reply-To: <20210519135251.30093-1-joro@8bytes.org>

From: Joerg Roedel <jroedel@suse.de>

The error reporting from the insn_fetch_from_user*() functions is not
very verbose. Extend it to include information on whether the linear
RIP could not be calculated or whether the memory access faulted.

This will be used in the SEV-ES code to propagate the correct
exception depending on what went wrong during instruction fetch.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 arch/x86/include/asm/insn-eval.h |  6 +++--
 arch/x86/kernel/sev.c            |  8 +++----
 arch/x86/kernel/umip.c           | 10 ++++-----
 arch/x86/lib/insn-eval.c         | 38 +++++++++++++++++++++++---------
 4 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
index 91d7182ad2d6..8362f1ce4b00 100644
--- a/arch/x86/include/asm/insn-eval.h
+++ b/arch/x86/include/asm/insn-eval.h
@@ -22,9 +22,11 @@ int insn_get_modrm_reg_off(struct insn *insn, struct pt_regs *regs);
 unsigned long insn_get_seg_base(struct pt_regs *regs, int seg_reg_idx);
 int insn_get_code_seg_params(struct pt_regs *regs);
 int insn_fetch_from_user(struct pt_regs *regs,
-			 unsigned char buf[MAX_INSN_SIZE]);
+			 unsigned char buf[MAX_INSN_SIZE],
+			 int *copied);
 int insn_fetch_from_user_inatomic(struct pt_regs *regs,
-				  unsigned char buf[MAX_INSN_SIZE]);
+				  unsigned char buf[MAX_INSN_SIZE],
+				  int *copied);
 bool insn_decode_from_regs(struct insn *insn, struct pt_regs *regs,
 			   unsigned char buf[MAX_INSN_SIZE], int buf_size);
 
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 9a64030e74c0..1edb6cd5e308 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -258,17 +258,17 @@ static int vc_fetch_insn_kernel(struct es_em_ctxt *ctxt,
 static enum es_result __vc_decode_user_insn(struct es_em_ctxt *ctxt)
 {
 	char buffer[MAX_INSN_SIZE];
-	int res;
+	int insn_bytes = 0, res;
 
-	res = insn_fetch_from_user_inatomic(ctxt->regs, buffer);
-	if (!res) {
+	res = insn_fetch_from_user_inatomic(ctxt->regs, buffer, &insn_bytes);
+	if (res) {
 		ctxt->fi.vector     = X86_TRAP_PF;
 		ctxt->fi.error_code = X86_PF_INSTR | X86_PF_USER;
 		ctxt->fi.cr2        = ctxt->regs->ip;
 		return ES_EXCEPTION;
 	}
 
-	if (!insn_decode_from_regs(&ctxt->insn, ctxt->regs, buffer, res))
+	if (!insn_decode_from_regs(&ctxt->insn, ctxt->regs, buffer, insn_bytes))
 		return ES_DECODE_FAILED;
 
 	if (ctxt->insn.immediate.got)
diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
index 8daa70b0d2da..b005ef42682e 100644
--- a/arch/x86/kernel/umip.c
+++ b/arch/x86/kernel/umip.c
@@ -346,13 +346,13 @@ bool fixup_umip_exception(struct pt_regs *regs)
 	if (!regs)
 		return false;
 
-	nr_copied = insn_fetch_from_user(regs, buf);
-
 	/*
-	 * The insn_fetch_from_user above could have failed if user code
-	 * is protected by a memory protection key. Give up on emulation
-	 * in such a case.  Should we issue a page fault?
+	 * Give up on emulation if fetching the instruction failed. Should we
+	 * issue a page fault or a #GP?
 	 */
+	if (!insn_fetch_from_user(regs, buf, NULL))
+		return false;
+
 	if (!nr_copied)
 		return false;
 
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index 4eecb9c7c6a0..d8a057ba0895 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -1442,27 +1442,36 @@ static int insn_get_effective_ip(struct pt_regs *regs, unsigned long *ip)
  * insn_fetch_from_user() - Copy instruction bytes from user-space memory
  * @regs:	Structure with register values as seen when entering kernel mode
  * @buf:	Array to store the fetched instruction
+ * @copied:	Pointer to an int where the number of copied instruction bytes
+ *		is stored. Can be NULL.
  *
  * Gets the linear address of the instruction and copies the instruction bytes
  * to the buf.
  *
  * Returns:
  *
- * Number of instruction bytes copied.
+ * -EINVAL if the linear address of the instruction could not be calculated
+ * -EFAULT if nothing was copied
+ *       0 on success
  *
- * 0 if nothing was copied.
  */
-int insn_fetch_from_user(struct pt_regs *regs, unsigned char buf[MAX_INSN_SIZE])
+int insn_fetch_from_user(struct pt_regs *regs, unsigned char buf[MAX_INSN_SIZE],
+			 int *copied)
 {
 	unsigned long ip;
 	int not_copied;
+	int bytes;
 
 	if (insn_get_effective_ip(regs, &ip))
-		return 0;
+		return -EINVAL;
 
 	not_copied = copy_from_user(buf, (void __user *)ip, MAX_INSN_SIZE);
 
-	return MAX_INSN_SIZE - not_copied;
+	bytes = MAX_INSN_SIZE - not_copied;
+	if (copied)
+		*copied = bytes;
+
+	return bytes ? 0 : -EFAULT;
 }
 
 /**
@@ -1470,27 +1479,36 @@ int insn_fetch_from_user(struct pt_regs *regs, unsigned char buf[MAX_INSN_SIZE])
  *                                   while in atomic code
  * @regs:	Structure with register values as seen when entering kernel mode
  * @buf:	Array to store the fetched instruction
+ * @copied:	Pointer to an int where the number of copied instruction bytes
+ *		is stored. Can be NULL.
  *
  * Gets the linear address of the instruction and copies the instruction bytes
  * to the buf. This function must be used in atomic context.
  *
  * Returns:
  *
- * Number of instruction bytes copied.
+ * -EINVAL if the linear address of the instruction could not be calculated
+ * -EFAULT if nothing was copied
+ *       0 on success
  *
- * 0 if nothing was copied.
  */
-int insn_fetch_from_user_inatomic(struct pt_regs *regs, unsigned char buf[MAX_INSN_SIZE])
+int insn_fetch_from_user_inatomic(struct pt_regs *regs, unsigned char buf[MAX_INSN_SIZE],
+				  int *copied)
 {
 	unsigned long ip;
 	int not_copied;
+	int bytes;
 
 	if (insn_get_effective_ip(regs, &ip))
-		return 0;
+		return -EINVAL;
 
 	not_copied = __copy_from_user_inatomic(buf, (void __user *)ip, MAX_INSN_SIZE);
 
-	return MAX_INSN_SIZE - not_copied;
+	bytes = MAX_INSN_SIZE - not_copied;
+	if (copied)
+		*copied = bytes;
+
+	return bytes ? 0 : -EFAULT;
 }
 
 /**
-- 
2.31.1


  parent reply	other threads:[~2021-05-19 13:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-19 13:52 [PATCH v2 0/8] x86/sev-es: Fixes for SEV-ES Guest Support Joerg Roedel
2021-05-19 13:52 ` [PATCH v2 1/8] x86/sev-es: Don't return NULL from sev_es_get_ghcb() Joerg Roedel
2021-05-19 13:52 ` [PATCH v2 2/8] x86/sev-es: Forward page-faults which happen during emulation Joerg Roedel
2021-05-19 13:52 ` [PATCH v2 3/8] x86/sev-es: Use __put_user()/__get_user() for data accesses Joerg Roedel
2021-05-19 13:52 ` [PATCH v2 4/8] x86/sev-es: Fix error message in runtime #VC handler Joerg Roedel
2021-05-19 13:52 ` [PATCH v2 5/8] x86/sev-es: Leave NMI-mode before sending signals Joerg Roedel
2021-05-19 17:54   ` Peter Zijlstra
2021-05-19 19:13     ` Joerg Roedel
2021-05-19 19:31       ` Peter Zijlstra
2021-05-19 13:52 ` [PATCH v2 6/8] x86/insn-eval: Make 0 a valid RIP for insn_get_effective_ip() Joerg Roedel
2021-05-19 13:52 ` Joerg Roedel [this message]
2021-05-21 14:34   ` [PATCH v2 7/8] x86/insn: Extend error reporting from insn_fetch_from_user[_inatomic]() Borislav Petkov
2021-05-19 13:52 ` [PATCH v2 8/8] x86/sev-es: Propagate #GP if getting linear instruction address failed Joerg Roedel

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=20210519135251.30093-8-joro@8bytes.org \
    --to=joro@8bytes.org \
    --cc=baekhw@google.com \
    --cc=cfir@google.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=erdemaktas@google.com \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=jroedel@suse.de \
    --cc=jslaby@suse.cz \
    --cc=keescook@chromium.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=martin.b.radev@gmail.com \
    --cc=mhiramat@kernel.org \
    --cc=mstunes@vmware.com \
    --cc=nivedita@alum.mit.edu \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=seanjc@google.com \
    --cc=thomas.lendacky@amd.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH v2 7/8] x86/insn: Extend error reporting from insn_fetch_from_user[_inatomic]()' \
    /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

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).