* [PATCH v2 01/21] x86/insn: Rename insn_decode() to insn_decode_from_regs()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 02/21] x86/insn: Add @buf_len param to insn_init() kernel-doc comment Borislav Petkov
` (19 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Rename insn_decode() to insn_decode_from_regs() to denote that it
receives regs as param and uses registers from there during decoding.
Free the former name for a more generic version of the function.
No functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/insn-eval.h | 4 ++--
arch/x86/kernel/sev-es.c | 2 +-
arch/x86/kernel/umip.c | 2 +-
arch/x86/lib/insn-eval.c | 6 +++---
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
index a0f839aa144d..c755f1a2618b 100644
--- a/arch/x86/include/asm/insn-eval.h
+++ b/arch/x86/include/asm/insn-eval.h
@@ -23,7 +23,7 @@ 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]);
-bool insn_decode(struct insn *insn, struct pt_regs *regs,
- unsigned char buf[MAX_INSN_SIZE], int buf_size);
+bool insn_decode_from_regs(struct insn *insn, struct pt_regs *regs,
+ unsigned char buf[MAX_INSN_SIZE], int buf_size);
#endif /* _ASM_X86_INSN_EVAL_H */
diff --git a/arch/x86/kernel/sev-es.c b/arch/x86/kernel/sev-es.c
index 84c1821819af..248d14ae5602 100644
--- a/arch/x86/kernel/sev-es.c
+++ b/arch/x86/kernel/sev-es.c
@@ -256,7 +256,7 @@ static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt)
return ES_EXCEPTION;
}
- if (!insn_decode(&ctxt->insn, ctxt->regs, buffer, res))
+ if (!insn_decode_from_regs(&ctxt->insn, ctxt->regs, buffer, res))
return ES_DECODE_FAILED;
} else {
res = vc_fetch_insn_kernel(ctxt, buffer);
diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
index f6225bf22c02..8032f5f7eef9 100644
--- a/arch/x86/kernel/umip.c
+++ b/arch/x86/kernel/umip.c
@@ -356,7 +356,7 @@ bool fixup_umip_exception(struct pt_regs *regs)
if (!nr_copied)
return false;
- if (!insn_decode(&insn, regs, buf, nr_copied))
+ if (!insn_decode_from_regs(&insn, regs, buf, nr_copied))
return false;
umip_inst = identify_insn(&insn);
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index 4229950a5d78..d0ebf9e93e35 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -1454,7 +1454,7 @@ int insn_fetch_from_user(struct pt_regs *regs, unsigned char buf[MAX_INSN_SIZE])
}
/**
- * insn_decode() - Decode an instruction
+ * insn_decode_from_regs() - Decode an instruction
* @insn: Structure to store decoded instruction
* @regs: Structure with register values as seen when entering kernel mode
* @buf: Buffer containing the instruction bytes
@@ -1467,8 +1467,8 @@ int insn_fetch_from_user(struct pt_regs *regs, unsigned char buf[MAX_INSN_SIZE])
*
* True if instruction was decoded, False otherwise.
*/
-bool insn_decode(struct insn *insn, struct pt_regs *regs,
- unsigned char buf[MAX_INSN_SIZE], int buf_size)
+bool insn_decode_from_regs(struct insn *insn, struct pt_regs *regs,
+ unsigned char buf[MAX_INSN_SIZE], int buf_size)
{
int seg_defs;
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 02/21] x86/insn: Add @buf_len param to insn_init() kernel-doc comment
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 01/21] x86/insn: Rename insn_decode() to insn_decode_from_regs() Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 03/21] x86/insn: Add a __ignore_sync_check__ marker Borislav Petkov
` (18 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
It wasn't documented so add it. No functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
arch/x86/lib/insn.c | 1 +
tools/arch/x86/lib/insn.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 404279563891..1ba994862b56 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -37,6 +37,7 @@
* insn_init() - initialize struct insn
* @insn: &struct insn to be initialized
* @kaddr: address (in kernel memory) of instruction (or copy thereof)
+ * @buf_len: length of the insn buffer at @kaddr
* @x86_64: !0 for 64-bit kernel or 64-bit app
*/
void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index 0151dfc6da61..f3277d6e4ef2 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -37,6 +37,7 @@
* insn_init() - initialize struct insn
* @insn: &struct insn to be initialized
* @kaddr: address (in kernel memory) of instruction (or copy thereof)
+ * @buf_len: length of the insn buffer at @kaddr
* @x86_64: !0 for 64-bit kernel or 64-bit app
*/
void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 03/21] x86/insn: Add a __ignore_sync_check__ marker
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 01/21] x86/insn: Rename insn_decode() to insn_decode_from_regs() Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 02/21] x86/insn: Add @buf_len param to insn_init() kernel-doc comment Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 04/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (17 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Add an explicit __ignore_sync_check__ marker which will be used to mark
lines which are supposed to be ignored by file synchronization check
scripts, its advantage being that it explicitly denotes such lines in
the code.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/inat.h | 2 +-
arch/x86/include/asm/insn.h | 2 +-
arch/x86/lib/inat.c | 2 +-
arch/x86/lib/insn.c | 6 +++---
tools/arch/x86/include/asm/inat.h | 2 +-
tools/arch/x86/include/asm/insn.h | 2 +-
tools/arch/x86/lib/inat.c | 2 +-
tools/arch/x86/lib/insn.c | 6 +++---
tools/objtool/sync-check.sh | 17 +++++++++++++----
| 15 +++++++++++----
10 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/arch/x86/include/asm/inat.h b/arch/x86/include/asm/inat.h
index 4cf2ad521f65..b56c5741581a 100644
--- a/arch/x86/include/asm/inat.h
+++ b/arch/x86/include/asm/inat.h
@@ -6,7 +6,7 @@
*
* Written by Masami Hiramatsu <mhiramat@redhat.com>
*/
-#include <asm/inat_types.h>
+#include <asm/inat_types.h> /* __ignore_sync_check__ */
/*
* Internal bits. Don't use bitmasks directly, because these bits are
diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index a8c3d284fa46..17c130f1ba57 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -8,7 +8,7 @@
*/
/* insn_attr_t is defined in inat.h */
-#include <asm/inat.h>
+#include <asm/inat.h> /* __ignore_sync_check__ */
struct insn_field {
union {
diff --git a/arch/x86/lib/inat.c b/arch/x86/lib/inat.c
index 12539fca75c4..b0f3b2a62ae2 100644
--- a/arch/x86/lib/inat.c
+++ b/arch/x86/lib/inat.c
@@ -4,7 +4,7 @@
*
* Written by Masami Hiramatsu <mhiramat@redhat.com>
*/
-#include <asm/insn.h>
+#include <asm/insn.h> /* __ignore_sync_check__ */
/* Attribute tables are generated from opcode map */
#include "inat-tables.c"
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 1ba994862b56..745c704f7c78 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -10,10 +10,10 @@
#else
#include <string.h>
#endif
-#include <asm/inat.h>
-#include <asm/insn.h>
+#include <asm/inat.h> /*__ignore_sync_check__ */
+#include <asm/insn.h> /* __ignore_sync_check__ */
-#include <asm/emulate_prefix.h>
+#include <asm/emulate_prefix.h> /* __ignore_sync_check__ */
/* Verify next sizeof(t) bytes can be on the same instruction */
#define validate_next(t, insn, n) \
diff --git a/tools/arch/x86/include/asm/inat.h b/tools/arch/x86/include/asm/inat.h
index 877827b7c2c3..a61051400311 100644
--- a/tools/arch/x86/include/asm/inat.h
+++ b/tools/arch/x86/include/asm/inat.h
@@ -6,7 +6,7 @@
*
* Written by Masami Hiramatsu <mhiramat@redhat.com>
*/
-#include "inat_types.h"
+#include "inat_types.h" /* __ignore_sync_check__ */
/*
* Internal bits. Don't use bitmasks directly, because these bits are
diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h
index 52c6262e6bfd..33d41982a5dd 100644
--- a/tools/arch/x86/include/asm/insn.h
+++ b/tools/arch/x86/include/asm/insn.h
@@ -8,7 +8,7 @@
*/
/* insn_attr_t is defined in inat.h */
-#include "inat.h"
+#include "inat.h" /* __ignore_sync_check__ */
struct insn_field {
union {
diff --git a/tools/arch/x86/lib/inat.c b/tools/arch/x86/lib/inat.c
index 4f5ed49e1b4e..dfbcc6405941 100644
--- a/tools/arch/x86/lib/inat.c
+++ b/tools/arch/x86/lib/inat.c
@@ -4,7 +4,7 @@
*
* Written by Masami Hiramatsu <mhiramat@redhat.com>
*/
-#include "../include/asm/insn.h"
+#include "../include/asm/insn.h" /* __ignore_sync_check__ */
/* Attribute tables are generated from opcode map */
#include "inat-tables.c"
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index f3277d6e4ef2..dedcfd67f90c 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -10,10 +10,10 @@
#else
#include <string.h>
#endif
-#include "../include/asm/inat.h"
-#include "../include/asm/insn.h"
+#include "../include/asm/inat.h" /* __ignore_sync_check__ */
+#include "../include/asm/insn.h" /* __ignore_sync_check__ */
-#include "../include/asm/emulate_prefix.h"
+#include "../include/asm/emulate_prefix.h" /* __ignore_sync_check__ */
/* Verify next sizeof(t) bytes can be on the same instruction */
#define validate_next(t, insn, n) \
diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh
index 606a4b5e929f..4bbabaecab14 100755
--- a/tools/objtool/sync-check.sh
+++ b/tools/objtool/sync-check.sh
@@ -16,11 +16,14 @@ arch/x86/include/asm/emulate_prefix.h
arch/x86/lib/x86-opcode-map.txt
arch/x86/tools/gen-insn-attr-x86.awk
include/linux/static_call_types.h
-arch/x86/include/asm/inat.h -I '^#include [\"<]\(asm/\)*inat_types.h[\">]'
-arch/x86/include/asm/insn.h -I '^#include [\"<]\(asm/\)*inat.h[\">]'
-arch/x86/lib/inat.c -I '^#include [\"<]\(../include/\)*asm/insn.h[\">]'
-arch/x86/lib/insn.c -I '^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]' -I '^#include [\"<]\(../include/\)*asm/emulate_prefix.h[\">]'
"
+
+SYNC_CHECK_FILES='
+arch/x86/include/asm/inat.h
+arch/x86/include/asm/insn.h
+arch/x86/lib/inat.c
+arch/x86/lib/insn.c
+'
fi
check_2 () {
@@ -63,3 +66,9 @@ while read -r file_entry; do
done <<EOF
$FILES
EOF
+
+if [ "$SRCARCH" = "x86" ]; then
+ for i in $SYNC_CHECK_FILES; do
+ check $i '-I "^.*\/\*.*__ignore_sync_check__.*\*\/.*$"'
+ done
+fi
--git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index dded93a2bc89..07857dfb4d91 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -75,6 +75,13 @@ include/uapi/asm-generic/mman-common.h
include/uapi/asm-generic/unistd.h
'
+SYNC_CHECK_FILES='
+arch/x86/include/asm/inat.h
+arch/x86/include/asm/insn.h
+arch/x86/lib/inat.c
+arch/x86/lib/insn.c
+'
+
# These copies are under tools/perf/trace/beauty/ as they are not used to in
# building object files only by scripts in tools/perf/trace/beauty/ to generate
# tables that then gets included in .c files for things like id->string syscall
@@ -129,6 +136,10 @@ for i in $FILES; do
check $i -B
done
+for i in $SYNC_CHECK_FILES; do
+ check $i '-I "^.*\/\*.*__ignore_sync_check__.*\*\/.*$"'
+done
+
# diff with extra ignore lines
check arch/x86/lib/memcpy_64.S '-I "^EXPORT_SYMBOL" -I "^#include <asm/export.h>" -I"^SYM_FUNC_START\(_LOCAL\)*(memcpy_\(erms\|orig\))"'
check arch/x86/lib/memset_64.S '-I "^EXPORT_SYMBOL" -I "^#include <asm/export.h>" -I"^SYM_FUNC_START\(_LOCAL\)*(memset_\(erms\|orig\))"'
@@ -137,10 +148,6 @@ check include/uapi/linux/mman.h '-I "^#include <\(uapi/\)*asm/mman.h>"'
check include/linux/build_bug.h '-I "^#\(ifndef\|endif\)\( \/\/\)* static_assert$"'
check include/linux/ctype.h '-I "isdigit("'
check lib/ctype.c '-I "^EXPORT_SYMBOL" -I "^#include <linux/export.h>" -B'
-check arch/x86/include/asm/inat.h '-I "^#include [\"<]\(asm/\)*inat_types.h[\">]"'
-check arch/x86/include/asm/insn.h '-I "^#include [\"<]\(asm/\)*inat.h[\">]"'
-check arch/x86/lib/inat.c '-I "^#include [\"<]\(../include/\)*asm/insn.h[\">]"'
-check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]" -I "^#include [\"<]\(../include/\)*asm/emulate_prefix.h[\">]"'
# diff non-symmetric files
check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 04/21] x86/insn: Add an insn_decode() API
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (2 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 03/21] x86/insn: Add a __ignore_sync_check__ marker Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-26 15:45 ` Masami Hiramatsu
2021-02-24 11:02 ` [PATCH v2 05/21] x86/insn-eval: Handle return values from the decoder Borislav Petkov
` (16 subsequent siblings)
20 siblings, 1 reply; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Users of the instruction decoder should use this to decode instruction
bytes. For that, have insn*() helpers return an int value to denote
success/failure. When there's an error fetching the next insn byte and
the insn falls short, return -ENODATA to denote that.
While at it, make insn_get_opcode() more stricter as to whether what has
seen so far is a valid insn and if not.
Copy linux/kconfig.h for the tools-version of the decoder so that it can
use IS_ENABLED().
Also, cast the INSN_MODE_KERN dummy define value to (enum insn_mode)
for tools use of the decoder because perf tool builds with -Werror and
errors out with -Werror=sign-compare otherwise.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/insn.h | 24 ++-
arch/x86/lib/insn.c | 241 +++++++++++++++++++++++-------
tools/arch/x86/include/asm/insn.h | 24 ++-
tools/arch/x86/lib/insn.c | 241 +++++++++++++++++++++++-------
tools/include/linux/kconfig.h | 73 +++++++++
5 files changed, 479 insertions(+), 124 deletions(-)
create mode 100644 tools/include/linux/kconfig.h
diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index 17c130f1ba57..546436b3c215 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -87,13 +87,23 @@ struct insn {
#define X86_VEX_M_MAX 0x1f /* VEX3.M Maximum value */
extern void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64);
-extern void insn_get_prefixes(struct insn *insn);
-extern void insn_get_opcode(struct insn *insn);
-extern void insn_get_modrm(struct insn *insn);
-extern void insn_get_sib(struct insn *insn);
-extern void insn_get_displacement(struct insn *insn);
-extern void insn_get_immediate(struct insn *insn);
-extern void insn_get_length(struct insn *insn);
+extern int insn_get_prefixes(struct insn *insn);
+extern int insn_get_opcode(struct insn *insn);
+extern int insn_get_modrm(struct insn *insn);
+extern int insn_get_sib(struct insn *insn);
+extern int insn_get_displacement(struct insn *insn);
+extern int insn_get_immediate(struct insn *insn);
+extern int insn_get_length(struct insn *insn);
+
+enum insn_mode {
+ INSN_MODE_32,
+ INSN_MODE_64,
+ /* Mode is determined by the current kernel build. */
+ INSN_MODE_KERN,
+ INSN_NUM_MODES,
+};
+
+extern int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m);
/* Attribute will be determined after getting ModRM (for opcode groups) */
static inline void insn_get_attribute(struct insn *insn)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 745c704f7c78..07ee5b14ae38 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -13,6 +13,9 @@
#include <asm/inat.h> /*__ignore_sync_check__ */
#include <asm/insn.h> /* __ignore_sync_check__ */
+#include <linux/errno.h>
+#include <linux/kconfig.h>
+
#include <asm/emulate_prefix.h> /* __ignore_sync_check__ */
/* Verify next sizeof(t) bytes can be on the same instruction */
@@ -80,12 +83,15 @@ static int __insn_get_emulate_prefix(struct insn *insn,
return 1;
err_out:
- return 0;
+ return -ENODATA;
}
static void insn_get_emulate_prefix(struct insn *insn)
{
- if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix)))
+ int ret;
+
+ ret = __insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix));
+ if (ret > 0)
return;
__insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix));
@@ -98,8 +104,12 @@ static void insn_get_emulate_prefix(struct insn *insn)
* Populates the @insn->prefixes bitmap, and updates @insn->next_byte
* to point to the (first) opcode. No effect if @insn->prefixes.got
* is already set.
+ *
+ * * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_prefixes(struct insn *insn)
+int insn_get_prefixes(struct insn *insn)
{
struct insn_field *prefixes = &insn->prefixes;
insn_attr_t attr;
@@ -107,7 +117,7 @@ void insn_get_prefixes(struct insn *insn)
int i, nb;
if (prefixes->got)
- return;
+ return 0;
insn_get_emulate_prefix(insn);
@@ -218,8 +228,10 @@ void insn_get_prefixes(struct insn *insn)
prefixes->got = 1;
+ return 0;
+
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -231,16 +243,25 @@ void insn_get_prefixes(struct insn *insn)
* If necessary, first collects any preceding (prefix) bytes.
* Sets @insn->opcode.value = opcode1. No effect if @insn->opcode.got
* is already 1.
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_opcode(struct insn *insn)
+int insn_get_opcode(struct insn *insn)
{
struct insn_field *opcode = &insn->opcode;
+ int pfx_id, ret;
insn_byte_t op;
- int pfx_id;
+
if (opcode->got)
- return;
- if (!insn->prefixes.got)
- insn_get_prefixes(insn);
+ return 0;
+
+ if (!insn->prefixes.got) {
+ ret = insn_get_prefixes(insn);
+ if (ret)
+ return ret;
+ }
/* Get first opcode */
op = get_next(insn_byte_t, insn);
@@ -255,9 +276,13 @@ void insn_get_opcode(struct insn *insn)
insn->attr = inat_get_avx_attribute(op, m, p);
if ((inat_must_evex(insn->attr) && !insn_is_evex(insn)) ||
(!inat_accept_vex(insn->attr) &&
- !inat_is_group(insn->attr)))
- insn->attr = 0; /* This instruction is bad */
- goto end; /* VEX has only 1 byte for opcode */
+ !inat_is_group(insn->attr))) {
+ /* This instruction is bad */
+ insn->attr = 0;
+ return 1;
+ }
+ /* VEX has only 1 byte for opcode */
+ goto end;
}
insn->attr = inat_get_opcode_attribute(op);
@@ -268,13 +293,18 @@ void insn_get_opcode(struct insn *insn)
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_escape_attribute(op, pfx_id, insn->attr);
}
- if (inat_must_vex(insn->attr))
- insn->attr = 0; /* This instruction is bad */
+
+ if (inat_must_vex(insn->attr)) {
+ /* This instruction is bad */
+ insn->attr = 0;
+ return 1;
+ }
end:
opcode->got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -284,15 +314,25 @@ void insn_get_opcode(struct insn *insn)
* Populates @insn->modrm and updates @insn->next_byte to point past the
* ModRM byte, if any. If necessary, first collects the preceding bytes
* (prefixes and opcode(s)). No effect if @insn->modrm.got is already 1.
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_modrm(struct insn *insn)
+int insn_get_modrm(struct insn *insn)
{
struct insn_field *modrm = &insn->modrm;
insn_byte_t pfx_id, mod;
+ int ret;
+
if (modrm->got)
- return;
- if (!insn->opcode.got)
- insn_get_opcode(insn);
+ return 0;
+
+ if (!insn->opcode.got) {
+ ret = insn_get_opcode(insn);
+ if (ret)
+ return ret;
+ }
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
@@ -302,17 +342,22 @@ void insn_get_modrm(struct insn *insn)
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_group_attribute(mod, pfx_id,
insn->attr);
- if (insn_is_avx(insn) && !inat_accept_vex(insn->attr))
- insn->attr = 0; /* This is bad */
+ if (insn_is_avx(insn) && !inat_accept_vex(insn->attr)) {
+ /* Bad insn */
+ insn->attr = 0;
+ return 1;
+ }
}
}
if (insn->x86_64 && inat_is_force64(insn->attr))
insn->opnd_bytes = 8;
+
modrm->got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
@@ -326,11 +371,16 @@ void insn_get_modrm(struct insn *insn)
int insn_rip_relative(struct insn *insn)
{
struct insn_field *modrm = &insn->modrm;
+ int ret;
if (!insn->x86_64)
return 0;
- if (!modrm->got)
- insn_get_modrm(insn);
+
+ if (!modrm->got) {
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
+ }
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
@@ -344,15 +394,25 @@ int insn_rip_relative(struct insn *insn)
*
* If necessary, first collects the instruction up to and including the
* ModRM byte.
+ *
+ * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
*/
-void insn_get_sib(struct insn *insn)
+int insn_get_sib(struct insn *insn)
{
insn_byte_t modrm;
+ int ret;
if (insn->sib.got)
- return;
- if (!insn->modrm.got)
- insn_get_modrm(insn);
+ return 0;
+
+ if (!insn->modrm.got) {
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
+ }
+
if (insn->modrm.nbytes) {
modrm = (insn_byte_t)insn->modrm.value;
if (insn->addr_bytes != 2 &&
@@ -363,8 +423,10 @@ void insn_get_sib(struct insn *insn)
}
insn->sib.got = 1;
+ return 0;
+
err_out:
- return;
+ return -ENODATA;
}
@@ -375,15 +437,25 @@ void insn_get_sib(struct insn *insn)
* If necessary, first collects the instruction up to and including the
* SIB byte.
* Displacement value is sign-expanded.
+ *
+ * * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
*/
-void insn_get_displacement(struct insn *insn)
+int insn_get_displacement(struct insn *insn)
{
insn_byte_t mod, rm, base;
+ int ret;
if (insn->displacement.got)
- return;
- if (!insn->sib.got)
- insn_get_sib(insn);
+ return 0;
+
+ if (!insn->sib.got) {
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
+ }
+
if (insn->modrm.nbytes) {
/*
* Interpreting the modrm byte:
@@ -426,12 +498,13 @@ void insn_get_displacement(struct insn *insn)
}
out:
insn->displacement.got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
-/* Decode moffset16/32/64. Return 0 if failed */
+/* Decode moffset16/32/64. Return a negative value if failed. */
static int __get_moffset(struct insn *insn)
{
switch (insn->addr_bytes) {
@@ -457,10 +530,10 @@ static int __get_moffset(struct insn *insn)
return 1;
err_out:
- return 0;
+ return -ENODATA;
}
-/* Decode imm v32(Iz). Return 0 if failed */
+/* Decode imm v32(Iz). Return a negative value if failed. */
static int __get_immv32(struct insn *insn)
{
switch (insn->opnd_bytes) {
@@ -480,10 +553,10 @@ static int __get_immv32(struct insn *insn)
return 1;
err_out:
- return 0;
+ return -ENODATA;
}
-/* Decode imm v64(Iv/Ov), Return 0 if failed */
+/* Decode imm v64(Iv/Ov). Return a negative value if failed. */
static int __get_immv(struct insn *insn)
{
switch (insn->opnd_bytes) {
@@ -507,11 +580,12 @@ static int __get_immv(struct insn *insn)
insn->immediate1.got = insn->immediate2.got = 1;
return 1;
+
err_out:
- return 0;
+ return -ENODATA;
}
-/* Decode ptr16:16/32(Ap) */
+/* Decode ptr16:16/32(Ap). Return a negative value if failed. */
static int __get_immptr(struct insn *insn)
{
switch (insn->opnd_bytes) {
@@ -534,25 +608,36 @@ static int __get_immptr(struct insn *insn)
insn->immediate1.got = insn->immediate2.got = 1;
return 1;
+
err_out:
- return 0;
+ return -ENODATA;
}
/**
- * insn_get_immediate() - Get the immediates of instruction
+ * insn_get_immediate() - Get the immediate in an instruction
* @insn: &struct insn containing instruction
*
* If necessary, first collects the instruction up to and including the
* displacement bytes.
* Basically, most of immediates are sign-expanded. Unsigned-value can be
- * get by bit masking with ((1 << (nbytes * 8)) - 1)
+ * computed by bit masking with ((1 << (nbytes * 8)) - 1)
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_immediate(struct insn *insn)
+int insn_get_immediate(struct insn *insn)
{
+ int ret;
+
if (insn->immediate.got)
- return;
- if (!insn->displacement.got)
- insn_get_displacement(insn);
+ return 0;
+
+ if (!insn->displacement.got) {
+ ret = insn_get_displacement(insn);
+ if (ret)
+ return ret;
+ }
if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
@@ -605,9 +690,10 @@ void insn_get_immediate(struct insn *insn)
}
done:
insn->immediate.got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -616,13 +702,58 @@ void insn_get_immediate(struct insn *insn)
*
* If necessary, first collects the instruction up to and including the
* immediates bytes.
- */
-void insn_get_length(struct insn *insn)
+ *
+ * Returns:
+ * - 0 on success
+ * - < 0 on error
+*/
+int insn_get_length(struct insn *insn)
{
+ int ret;
+
if (insn->length)
- return;
- if (!insn->immediate.got)
- insn_get_immediate(insn);
+ return 0;
+
+ if (!insn->immediate.got) {
+ ret = insn_get_immediate(insn);
+ if (ret)
+ return ret;
+ }
+
insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
+
+ return 0;
+}
+
+/**
+ * insn_decode() - Decode an x86 instruction
+ * @insn: &struct insn to be initialized
+ * @kaddr: address (in kernel memory) of instruction (or copy thereof)
+ * @buf_len: length of the insn buffer at @kaddr
+ * @m: insn mode, see enum insn_mode
+ *
+ * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
+ */
+int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m)
+{
+ int ret;
+
+/* #define INSN_MODE_KERN -1 __ignore_sync_check__ mode is only valid in the kernel */
+
+ if (m == INSN_MODE_KERN)
+ insn_init(insn, kaddr, buf_len, IS_ENABLED(CONFIG_X86_64));
+ else
+ insn_init(insn, kaddr, buf_len, m == INSN_MODE_64);
+
+ ret = insn_get_length(insn);
+ if (ret)
+ return ret;
+
+ if (insn_complete(insn))
+ return 0;
+
+ return -EINVAL;
}
diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h
index 33d41982a5dd..621ab64a6d27 100644
--- a/tools/arch/x86/include/asm/insn.h
+++ b/tools/arch/x86/include/asm/insn.h
@@ -87,13 +87,23 @@ struct insn {
#define X86_VEX_M_MAX 0x1f /* VEX3.M Maximum value */
extern void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64);
-extern void insn_get_prefixes(struct insn *insn);
-extern void insn_get_opcode(struct insn *insn);
-extern void insn_get_modrm(struct insn *insn);
-extern void insn_get_sib(struct insn *insn);
-extern void insn_get_displacement(struct insn *insn);
-extern void insn_get_immediate(struct insn *insn);
-extern void insn_get_length(struct insn *insn);
+extern int insn_get_prefixes(struct insn *insn);
+extern int insn_get_opcode(struct insn *insn);
+extern int insn_get_modrm(struct insn *insn);
+extern int insn_get_sib(struct insn *insn);
+extern int insn_get_displacement(struct insn *insn);
+extern int insn_get_immediate(struct insn *insn);
+extern int insn_get_length(struct insn *insn);
+
+enum insn_mode {
+ INSN_MODE_32,
+ INSN_MODE_64,
+ /* Mode is determined by the current kernel build. */
+ INSN_MODE_KERN,
+ INSN_NUM_MODES,
+};
+
+extern int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m);
/* Attribute will be determined after getting ModRM (for opcode groups) */
static inline void insn_get_attribute(struct insn *insn)
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index dedcfd67f90c..6c9c1648eef3 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -13,6 +13,9 @@
#include "../include/asm/inat.h" /* __ignore_sync_check__ */
#include "../include/asm/insn.h" /* __ignore_sync_check__ */
+#include <linux/errno.h>
+#include <linux/kconfig.h>
+
#include "../include/asm/emulate_prefix.h" /* __ignore_sync_check__ */
/* Verify next sizeof(t) bytes can be on the same instruction */
@@ -80,12 +83,15 @@ static int __insn_get_emulate_prefix(struct insn *insn,
return 1;
err_out:
- return 0;
+ return -ENODATA;
}
static void insn_get_emulate_prefix(struct insn *insn)
{
- if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix)))
+ int ret;
+
+ ret = __insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix));
+ if (ret > 0)
return;
__insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix));
@@ -98,8 +104,12 @@ static void insn_get_emulate_prefix(struct insn *insn)
* Populates the @insn->prefixes bitmap, and updates @insn->next_byte
* to point to the (first) opcode. No effect if @insn->prefixes.got
* is already set.
+ *
+ * * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_prefixes(struct insn *insn)
+int insn_get_prefixes(struct insn *insn)
{
struct insn_field *prefixes = &insn->prefixes;
insn_attr_t attr;
@@ -107,7 +117,7 @@ void insn_get_prefixes(struct insn *insn)
int i, nb;
if (prefixes->got)
- return;
+ return 0;
insn_get_emulate_prefix(insn);
@@ -218,8 +228,10 @@ void insn_get_prefixes(struct insn *insn)
prefixes->got = 1;
+ return 0;
+
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -231,16 +243,25 @@ void insn_get_prefixes(struct insn *insn)
* If necessary, first collects any preceding (prefix) bytes.
* Sets @insn->opcode.value = opcode1. No effect if @insn->opcode.got
* is already 1.
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_opcode(struct insn *insn)
+int insn_get_opcode(struct insn *insn)
{
struct insn_field *opcode = &insn->opcode;
+ int pfx_id, ret;
insn_byte_t op;
- int pfx_id;
+
if (opcode->got)
- return;
- if (!insn->prefixes.got)
- insn_get_prefixes(insn);
+ return 0;
+
+ if (!insn->prefixes.got) {
+ ret = insn_get_prefixes(insn);
+ if (ret)
+ return ret;
+ }
/* Get first opcode */
op = get_next(insn_byte_t, insn);
@@ -255,9 +276,13 @@ void insn_get_opcode(struct insn *insn)
insn->attr = inat_get_avx_attribute(op, m, p);
if ((inat_must_evex(insn->attr) && !insn_is_evex(insn)) ||
(!inat_accept_vex(insn->attr) &&
- !inat_is_group(insn->attr)))
- insn->attr = 0; /* This instruction is bad */
- goto end; /* VEX has only 1 byte for opcode */
+ !inat_is_group(insn->attr))) {
+ /* This instruction is bad */
+ insn->attr = 0;
+ return 1;
+ }
+ /* VEX has only 1 byte for opcode */
+ goto end;
}
insn->attr = inat_get_opcode_attribute(op);
@@ -268,13 +293,18 @@ void insn_get_opcode(struct insn *insn)
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_escape_attribute(op, pfx_id, insn->attr);
}
- if (inat_must_vex(insn->attr))
- insn->attr = 0; /* This instruction is bad */
+
+ if (inat_must_vex(insn->attr)) {
+ /* This instruction is bad */
+ insn->attr = 0;
+ return 1;
+ }
end:
opcode->got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -284,15 +314,25 @@ void insn_get_opcode(struct insn *insn)
* Populates @insn->modrm and updates @insn->next_byte to point past the
* ModRM byte, if any. If necessary, first collects the preceding bytes
* (prefixes and opcode(s)). No effect if @insn->modrm.got is already 1.
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_modrm(struct insn *insn)
+int insn_get_modrm(struct insn *insn)
{
struct insn_field *modrm = &insn->modrm;
insn_byte_t pfx_id, mod;
+ int ret;
+
if (modrm->got)
- return;
- if (!insn->opcode.got)
- insn_get_opcode(insn);
+ return 0;
+
+ if (!insn->opcode.got) {
+ ret = insn_get_opcode(insn);
+ if (ret)
+ return ret;
+ }
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
@@ -302,17 +342,22 @@ void insn_get_modrm(struct insn *insn)
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_group_attribute(mod, pfx_id,
insn->attr);
- if (insn_is_avx(insn) && !inat_accept_vex(insn->attr))
- insn->attr = 0; /* This is bad */
+ if (insn_is_avx(insn) && !inat_accept_vex(insn->attr)) {
+ /* Bad insn */
+ insn->attr = 0;
+ return 1;
+ }
}
}
if (insn->x86_64 && inat_is_force64(insn->attr))
insn->opnd_bytes = 8;
+
modrm->got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
@@ -326,11 +371,16 @@ void insn_get_modrm(struct insn *insn)
int insn_rip_relative(struct insn *insn)
{
struct insn_field *modrm = &insn->modrm;
+ int ret;
if (!insn->x86_64)
return 0;
- if (!modrm->got)
- insn_get_modrm(insn);
+
+ if (!modrm->got) {
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
+ }
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
@@ -344,15 +394,25 @@ int insn_rip_relative(struct insn *insn)
*
* If necessary, first collects the instruction up to and including the
* ModRM byte.
+ *
+ * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
*/
-void insn_get_sib(struct insn *insn)
+int insn_get_sib(struct insn *insn)
{
insn_byte_t modrm;
+ int ret;
if (insn->sib.got)
- return;
- if (!insn->modrm.got)
- insn_get_modrm(insn);
+ return 0;
+
+ if (!insn->modrm.got) {
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
+ }
+
if (insn->modrm.nbytes) {
modrm = (insn_byte_t)insn->modrm.value;
if (insn->addr_bytes != 2 &&
@@ -363,8 +423,10 @@ void insn_get_sib(struct insn *insn)
}
insn->sib.got = 1;
+ return 0;
+
err_out:
- return;
+ return -ENODATA;
}
@@ -375,15 +437,25 @@ void insn_get_sib(struct insn *insn)
* If necessary, first collects the instruction up to and including the
* SIB byte.
* Displacement value is sign-expanded.
+ *
+ * * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
*/
-void insn_get_displacement(struct insn *insn)
+int insn_get_displacement(struct insn *insn)
{
insn_byte_t mod, rm, base;
+ int ret;
if (insn->displacement.got)
- return;
- if (!insn->sib.got)
- insn_get_sib(insn);
+ return 0;
+
+ if (!insn->sib.got) {
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
+ }
+
if (insn->modrm.nbytes) {
/*
* Interpreting the modrm byte:
@@ -426,12 +498,13 @@ void insn_get_displacement(struct insn *insn)
}
out:
insn->displacement.got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
-/* Decode moffset16/32/64. Return 0 if failed */
+/* Decode moffset16/32/64. Return a negative value if failed. */
static int __get_moffset(struct insn *insn)
{
switch (insn->addr_bytes) {
@@ -457,10 +530,10 @@ static int __get_moffset(struct insn *insn)
return 1;
err_out:
- return 0;
+ return -ENODATA;
}
-/* Decode imm v32(Iz). Return 0 if failed */
+/* Decode imm v32(Iz). Return a negative value if failed. */
static int __get_immv32(struct insn *insn)
{
switch (insn->opnd_bytes) {
@@ -480,10 +553,10 @@ static int __get_immv32(struct insn *insn)
return 1;
err_out:
- return 0;
+ return -ENODATA;
}
-/* Decode imm v64(Iv/Ov), Return 0 if failed */
+/* Decode imm v64(Iv/Ov). Return a negative value if failed. */
static int __get_immv(struct insn *insn)
{
switch (insn->opnd_bytes) {
@@ -507,11 +580,12 @@ static int __get_immv(struct insn *insn)
insn->immediate1.got = insn->immediate2.got = 1;
return 1;
+
err_out:
- return 0;
+ return -ENODATA;
}
-/* Decode ptr16:16/32(Ap) */
+/* Decode ptr16:16/32(Ap). Return a negative value if failed. */
static int __get_immptr(struct insn *insn)
{
switch (insn->opnd_bytes) {
@@ -534,25 +608,36 @@ static int __get_immptr(struct insn *insn)
insn->immediate1.got = insn->immediate2.got = 1;
return 1;
+
err_out:
- return 0;
+ return -ENODATA;
}
/**
- * insn_get_immediate() - Get the immediates of instruction
+ * insn_get_immediate() - Get the immediate in an instruction
* @insn: &struct insn containing instruction
*
* If necessary, first collects the instruction up to and including the
* displacement bytes.
* Basically, most of immediates are sign-expanded. Unsigned-value can be
- * get by bit masking with ((1 << (nbytes * 8)) - 1)
+ * computed by bit masking with ((1 << (nbytes * 8)) - 1)
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_immediate(struct insn *insn)
+int insn_get_immediate(struct insn *insn)
{
+ int ret;
+
if (insn->immediate.got)
- return;
- if (!insn->displacement.got)
- insn_get_displacement(insn);
+ return 0;
+
+ if (!insn->displacement.got) {
+ ret = insn_get_displacement(insn);
+ if (ret)
+ return ret;
+ }
if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
@@ -605,9 +690,10 @@ void insn_get_immediate(struct insn *insn)
}
done:
insn->immediate.got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -616,13 +702,58 @@ void insn_get_immediate(struct insn *insn)
*
* If necessary, first collects the instruction up to and including the
* immediates bytes.
- */
-void insn_get_length(struct insn *insn)
+ *
+ * Returns:
+ * - 0 on success
+ * - < 0 on error
+*/
+int insn_get_length(struct insn *insn)
{
+ int ret;
+
if (insn->length)
- return;
- if (!insn->immediate.got)
- insn_get_immediate(insn);
+ return 0;
+
+ if (!insn->immediate.got) {
+ ret = insn_get_immediate(insn);
+ if (ret)
+ return ret;
+ }
+
insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
+
+ return 0;
+}
+
+/**
+ * insn_decode() - Decode an x86 instruction
+ * @insn: &struct insn to be initialized
+ * @kaddr: address (in kernel memory) of instruction (or copy thereof)
+ * @buf_len: length of the insn buffer at @kaddr
+ * @m: insn mode, see enum insn_mode
+ *
+ * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
+ */
+int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m)
+{
+ int ret;
+
+#define INSN_MODE_KERN (enum insn_mode)-1 /* __ignore_sync_check__ mode is only valid in the kernel */
+
+ if (m == INSN_MODE_KERN)
+ insn_init(insn, kaddr, buf_len, IS_ENABLED(CONFIG_X86_64));
+ else
+ insn_init(insn, kaddr, buf_len, m == INSN_MODE_64);
+
+ ret = insn_get_length(insn);
+ if (ret)
+ return ret;
+
+ if (insn_complete(insn))
+ return 0;
+
+ return -EINVAL;
}
diff --git a/tools/include/linux/kconfig.h b/tools/include/linux/kconfig.h
new file mode 100644
index 000000000000..1555a0c4f345
--- /dev/null
+++ b/tools/include/linux/kconfig.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TOOLS_LINUX_KCONFIG_H
+#define _TOOLS_LINUX_KCONFIG_H
+
+/* CONFIG_CC_VERSION_TEXT (Do not delete this comment. See help in Kconfig) */
+
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#define __BIG_ENDIAN 4321
+#else
+#define __LITTLE_ENDIAN 1234
+#endif
+
+#define __ARG_PLACEHOLDER_1 0,
+#define __take_second_arg(__ignored, val, ...) val
+
+/*
+ * The use of "&&" / "||" is limited in certain expressions.
+ * The following enable to calculate "and" / "or" with macro expansion only.
+ */
+#define __and(x, y) ___and(x, y)
+#define ___and(x, y) ____and(__ARG_PLACEHOLDER_##x, y)
+#define ____and(arg1_or_junk, y) __take_second_arg(arg1_or_junk y, 0)
+
+#define __or(x, y) ___or(x, y)
+#define ___or(x, y) ____or(__ARG_PLACEHOLDER_##x, y)
+#define ____or(arg1_or_junk, y) __take_second_arg(arg1_or_junk 1, y)
+
+/*
+ * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
+ * these only work with boolean and tristate options.
+ */
+
+/*
+ * Getting something that works in C and CPP for an arg that may or may
+ * not be defined is tricky. Here, if we have "#define CONFIG_BOOGER 1"
+ * we match on the placeholder define, insert the "0," for arg1 and generate
+ * the triplet (0, 1, 0). Then the last step cherry picks the 2nd arg (a one).
+ * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
+ * the last step cherry picks the 2nd arg, we get a zero.
+ */
+#define __is_defined(x) ___is_defined(x)
+#define ___is_defined(val) ____is_defined(__ARG_PLACEHOLDER_##val)
+#define ____is_defined(arg1_or_junk) __take_second_arg(arg1_or_junk 1, 0)
+
+/*
+ * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
+ * otherwise. For boolean options, this is equivalent to
+ * IS_ENABLED(CONFIG_FOO).
+ */
+#define IS_BUILTIN(option) __is_defined(option)
+
+/*
+ * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
+ * otherwise.
+ */
+#define IS_MODULE(option) __is_defined(option##_MODULE)
+
+/*
+ * IS_REACHABLE(CONFIG_FOO) evaluates to 1 if the currently compiled
+ * code can call a function defined in code compiled based on CONFIG_FOO.
+ * This is similar to IS_ENABLED(), but returns false when invoked from
+ * built-in code when CONFIG_FOO is set to 'm'.
+ */
+#define IS_REACHABLE(option) __or(IS_BUILTIN(option), \
+ __and(IS_MODULE(option), __is_defined(MODULE)))
+
+/*
+ * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
+ * 0 otherwise.
+ */
+#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
+
+#endif /* _TOOLS_LINUX_KCONFIG_H */
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v2 04/21] x86/insn: Add an insn_decode() API
2021-02-24 11:02 ` [PATCH v2 04/21] x86/insn: Add an insn_decode() API Borislav Petkov
@ 2021-02-26 15:45 ` Masami Hiramatsu
2021-02-26 18:30 ` Borislav Petkov
0 siblings, 1 reply; 25+ messages in thread
From: Masami Hiramatsu @ 2021-02-26 15:45 UTC (permalink / raw)
To: Borislav Petkov; +Cc: X86 ML, Masami Hiramatsu, LKML
Hi Borislav,
On Wed, 24 Feb 2021 12:02:16 +0100
Borislav Petkov <bp@alien8.de> wrote:
> From: Borislav Petkov <bp@suse.de>
>
> Users of the instruction decoder should use this to decode instruction
> bytes. For that, have insn*() helpers return an int value to denote
> success/failure. When there's an error fetching the next insn byte and
> the insn falls short, return -ENODATA to denote that.
>
> While at it, make insn_get_opcode() more stricter as to whether what has
> seen so far is a valid insn and if not.
>
OK, but I think it should return -EINVAL or -EILSEQ for bad instruction.
And I found a bug.
[...]
> @@ -231,16 +243,25 @@ void insn_get_prefixes(struct insn *insn)
> * If necessary, first collects any preceding (prefix) bytes.
> * Sets @insn->opcode.value = opcode1. No effect if @insn->opcode.got
> * is already 1.
> + *
> + * Returns:
> + * 0: on success
> + * < 0: on error
OK, but
> */
> -void insn_get_opcode(struct insn *insn)
> +int insn_get_opcode(struct insn *insn)
> {
> struct insn_field *opcode = &insn->opcode;
> + int pfx_id, ret;
> insn_byte_t op;
> - int pfx_id;
> +
> if (opcode->got)
> - return;
> - if (!insn->prefixes.got)
> - insn_get_prefixes(insn);
> + return 0;
> +
> + if (!insn->prefixes.got) {
> + ret = insn_get_prefixes(insn);
> + if (ret)
> + return ret;
> + }
>
> /* Get first opcode */
> op = get_next(insn_byte_t, insn);
> @@ -255,9 +276,13 @@ void insn_get_opcode(struct insn *insn)
> insn->attr = inat_get_avx_attribute(op, m, p);
> if ((inat_must_evex(insn->attr) && !insn_is_evex(insn)) ||
> (!inat_accept_vex(insn->attr) &&
> - !inat_is_group(insn->attr)))
> - insn->attr = 0; /* This instruction is bad */
> - goto end; /* VEX has only 1 byte for opcode */
> + !inat_is_group(insn->attr))) {
> + /* This instruction is bad */
> + insn->attr = 0;
> + return 1;
Here you return 1 for a bad opcode.
> + }
> + /* VEX has only 1 byte for opcode */
> + goto end;
> }
>
> insn->attr = inat_get_opcode_attribute(op);
> @@ -268,13 +293,18 @@ void insn_get_opcode(struct insn *insn)
> pfx_id = insn_last_prefix_id(insn);
> insn->attr = inat_get_escape_attribute(op, pfx_id, insn->attr);
> }
> - if (inat_must_vex(insn->attr))
> - insn->attr = 0; /* This instruction is bad */
> +
> + if (inat_must_vex(insn->attr)) {
> + /* This instruction is bad */
> + insn->attr = 0;
> + return 1;
Ditto.
Would you mean -EINVAL?
> + }
> end:
> opcode->got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /**
> @@ -284,15 +314,25 @@ void insn_get_opcode(struct insn *insn)
> * Populates @insn->modrm and updates @insn->next_byte to point past the
> * ModRM byte, if any. If necessary, first collects the preceding bytes
> * (prefixes and opcode(s)). No effect if @insn->modrm.got is already 1.
> + *
> + * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_modrm(struct insn *insn)
> +int insn_get_modrm(struct insn *insn)
> {
> struct insn_field *modrm = &insn->modrm;
> insn_byte_t pfx_id, mod;
> + int ret;
> +
> if (modrm->got)
> - return;
> - if (!insn->opcode.got)
> - insn_get_opcode(insn);
> + return 0;
> +
> + if (!insn->opcode.got) {
> + ret = insn_get_opcode(insn);
> + if (ret)
> + return ret;
> + }
>
> if (inat_has_modrm(insn->attr)) {
> mod = get_next(insn_byte_t, insn);
> @@ -302,17 +342,22 @@ void insn_get_modrm(struct insn *insn)
> pfx_id = insn_last_prefix_id(insn);
> insn->attr = inat_get_group_attribute(mod, pfx_id,
> insn->attr);
> - if (insn_is_avx(insn) && !inat_accept_vex(insn->attr))
> - insn->attr = 0; /* This is bad */
> + if (insn_is_avx(insn) && !inat_accept_vex(insn->attr)) {
> + /* Bad insn */
> + insn->attr = 0;
> + return 1;
Here is another return 1.
> + }
> }
> }
>
> if (insn->x86_64 && inat_is_force64(insn->attr))
> insn->opnd_bytes = 8;
> +
> modrm->got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
>
> @@ -326,11 +371,16 @@ void insn_get_modrm(struct insn *insn)
> int insn_rip_relative(struct insn *insn)
> {
> struct insn_field *modrm = &insn->modrm;
> + int ret;
>
> if (!insn->x86_64)
> return 0;
> - if (!modrm->got)
> - insn_get_modrm(insn);
> +
> + if (!modrm->got) {
> + ret = insn_get_modrm(insn);
> + if (ret)
> + return ret;
> + }
> /*
> * For rip-relative instructions, the mod field (top 2 bits)
> * is zero and the r/m field (bottom 3 bits) is 0x5.
> @@ -344,15 +394,25 @@ int insn_rip_relative(struct insn *insn)
> *
> * If necessary, first collects the instruction up to and including the
> * ModRM byte.
> + *
> + * Returns:
> + * 0: if decoding succeeded
> + * < 0: otherwise.
> */
> -void insn_get_sib(struct insn *insn)
> +int insn_get_sib(struct insn *insn)
> {
> insn_byte_t modrm;
> + int ret;
>
> if (insn->sib.got)
> - return;
> - if (!insn->modrm.got)
> - insn_get_modrm(insn);
> + return 0;
> +
> + if (!insn->modrm.got) {
> + ret = insn_get_modrm(insn);
> + if (ret)
> + return ret;
> + }
> +
> if (insn->modrm.nbytes) {
> modrm = (insn_byte_t)insn->modrm.value;
> if (insn->addr_bytes != 2 &&
> @@ -363,8 +423,10 @@ void insn_get_sib(struct insn *insn)
> }
> insn->sib.got = 1;
>
> + return 0;
> +
> err_out:
> - return;
> + return -ENODATA;
> }
>
>
> @@ -375,15 +437,25 @@ void insn_get_sib(struct insn *insn)
> * If necessary, first collects the instruction up to and including the
> * SIB byte.
> * Displacement value is sign-expanded.
> + *
> + * * Returns:
> + * 0: if decoding succeeded
> + * < 0: otherwise.
> */
> -void insn_get_displacement(struct insn *insn)
> +int insn_get_displacement(struct insn *insn)
> {
> insn_byte_t mod, rm, base;
> + int ret;
>
> if (insn->displacement.got)
> - return;
> - if (!insn->sib.got)
> - insn_get_sib(insn);
> + return 0;
> +
> + if (!insn->sib.got) {
> + ret = insn_get_sib(insn);
> + if (ret)
> + return ret;
> + }
> +
> if (insn->modrm.nbytes) {
> /*
> * Interpreting the modrm byte:
> @@ -426,12 +498,13 @@ void insn_get_displacement(struct insn *insn)
> }
> out:
> insn->displacement.got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
> -/* Decode moffset16/32/64. Return 0 if failed */
> +/* Decode moffset16/32/64. Return a negative value if failed. */
> static int __get_moffset(struct insn *insn)
> {
> switch (insn->addr_bytes) {
> @@ -457,10 +530,10 @@ static int __get_moffset(struct insn *insn)
> return 1;
>
> err_out:
> - return 0;
> + return -ENODATA;
Also, __get_*() functions are expected to return bool (1/0)
for checking bad data. See insn_get_immediate() INAT_IMM_PTR case for example.
> }
>
> -/* Decode imm v32(Iz). Return 0 if failed */
> +/* Decode imm v32(Iz). Return a negative value if failed. */
> static int __get_immv32(struct insn *insn)
> {
> switch (insn->opnd_bytes) {
> @@ -480,10 +553,10 @@ static int __get_immv32(struct insn *insn)
> return 1;
>
> err_out:
> - return 0;
> + return -ENODATA;
Ditto.
> }
>
> -/* Decode imm v64(Iv/Ov), Return 0 if failed */
> +/* Decode imm v64(Iv/Ov). Return a negative value if failed. */
> static int __get_immv(struct insn *insn)
> {
> switch (insn->opnd_bytes) {
> @@ -507,11 +580,12 @@ static int __get_immv(struct insn *insn)
> insn->immediate1.got = insn->immediate2.got = 1;
>
> return 1;
> +
> err_out:
> - return 0;
> + return -ENODATA;
Ditto.
> }
>
> -/* Decode ptr16:16/32(Ap) */
> +/* Decode ptr16:16/32(Ap). Return a negative value if failed. */
> static int __get_immptr(struct insn *insn)
> {
> switch (insn->opnd_bytes) {
> @@ -534,25 +608,36 @@ static int __get_immptr(struct insn *insn)
> insn->immediate1.got = insn->immediate2.got = 1;
>
> return 1;
> +
> err_out:
> - return 0;
> + return -ENODATA;
Ditto.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v2 04/21] x86/insn: Add an insn_decode() API
2021-02-26 15:45 ` Masami Hiramatsu
@ 2021-02-26 18:30 ` Borislav Petkov
2021-02-28 14:51 ` Masami Hiramatsu
0 siblings, 1 reply; 25+ messages in thread
From: Borislav Petkov @ 2021-02-26 18:30 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: X86 ML, LKML
On Sat, Feb 27, 2021 at 12:45:06AM +0900, Masami Hiramatsu wrote:
> OK, but I think it should return -EINVAL or -EILSEQ for bad instruction.
It does return -EINVAL when insn_complete() returns 0.
> Here you return 1 for a bad opcode.
Whoops, that's a leftover from the early version where it would return
!0 on error. Lemme fix that.
> Ditto.
> Would you mean -EINVAL?
Yap.
> Also, __get_*() functions are expected to return bool (1/0)
> for checking bad data. See insn_get_immediate() INAT_IMM_PTR case for example.
Yuck, I totally snafu'ed that, sorry ;-\
Ok, see below. The patterns look correct now, I'll give it another look
tomorrow again, to make sure I haven't missed anything.
Thx for catching those!
---
From: Borislav Petkov <bp@suse.de>
Date: Tue, 3 Nov 2020 17:28:30 +0100
Subject: [PATCH] x86/insn: Add an insn_decode() API
Users of the instruction decoder should use this to decode instruction
bytes. For that, have insn*() helpers return an int value to denote
success/failure. When there's an error fetching the next insn byte and
the insn falls short, return -ENODATA to denote that.
While at it, make insn_get_opcode() more stricter as to whether what has
seen so far is a valid insn and if not.
Copy linux/kconfig.h for the tools-version of the decoder so that it can
use IS_ENABLED().
Also, cast the INSN_MODE_KERN dummy define value to (enum insn_mode)
for tools use of the decoder because perf tool builds with -Werror and
errors out with -Werror=sign-compare otherwise.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/insn.h | 24 +++-
arch/x86/lib/insn.c | 216 +++++++++++++++++++++++------
tools/arch/x86/include/asm/insn.h | 24 +++-
tools/arch/x86/lib/insn.c | 222 +++++++++++++++++++++++-------
tools/include/linux/kconfig.h | 73 ++++++++++
5 files changed, 452 insertions(+), 107 deletions(-)
create mode 100644 tools/include/linux/kconfig.h
diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index 17c130f1ba57..546436b3c215 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -87,13 +87,23 @@ struct insn {
#define X86_VEX_M_MAX 0x1f /* VEX3.M Maximum value */
extern void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64);
-extern void insn_get_prefixes(struct insn *insn);
-extern void insn_get_opcode(struct insn *insn);
-extern void insn_get_modrm(struct insn *insn);
-extern void insn_get_sib(struct insn *insn);
-extern void insn_get_displacement(struct insn *insn);
-extern void insn_get_immediate(struct insn *insn);
-extern void insn_get_length(struct insn *insn);
+extern int insn_get_prefixes(struct insn *insn);
+extern int insn_get_opcode(struct insn *insn);
+extern int insn_get_modrm(struct insn *insn);
+extern int insn_get_sib(struct insn *insn);
+extern int insn_get_displacement(struct insn *insn);
+extern int insn_get_immediate(struct insn *insn);
+extern int insn_get_length(struct insn *insn);
+
+enum insn_mode {
+ INSN_MODE_32,
+ INSN_MODE_64,
+ /* Mode is determined by the current kernel build. */
+ INSN_MODE_KERN,
+ INSN_NUM_MODES,
+};
+
+extern int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m);
/* Attribute will be determined after getting ModRM (for opcode groups) */
static inline void insn_get_attribute(struct insn *insn)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 745c704f7c78..9a395c39a0c0 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -13,6 +13,9 @@
#include <asm/inat.h> /*__ignore_sync_check__ */
#include <asm/insn.h> /* __ignore_sync_check__ */
+#include <linux/errno.h>
+#include <linux/kconfig.h>
+
#include <asm/emulate_prefix.h> /* __ignore_sync_check__ */
/* Verify next sizeof(t) bytes can be on the same instruction */
@@ -98,8 +101,12 @@ static void insn_get_emulate_prefix(struct insn *insn)
* Populates the @insn->prefixes bitmap, and updates @insn->next_byte
* to point to the (first) opcode. No effect if @insn->prefixes.got
* is already set.
+ *
+ * * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_prefixes(struct insn *insn)
+int insn_get_prefixes(struct insn *insn)
{
struct insn_field *prefixes = &insn->prefixes;
insn_attr_t attr;
@@ -107,7 +114,7 @@ void insn_get_prefixes(struct insn *insn)
int i, nb;
if (prefixes->got)
- return;
+ return 0;
insn_get_emulate_prefix(insn);
@@ -218,8 +225,10 @@ void insn_get_prefixes(struct insn *insn)
prefixes->got = 1;
+ return 0;
+
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -231,16 +240,25 @@ void insn_get_prefixes(struct insn *insn)
* If necessary, first collects any preceding (prefix) bytes.
* Sets @insn->opcode.value = opcode1. No effect if @insn->opcode.got
* is already 1.
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_opcode(struct insn *insn)
+int insn_get_opcode(struct insn *insn)
{
struct insn_field *opcode = &insn->opcode;
+ int pfx_id, ret;
insn_byte_t op;
- int pfx_id;
+
if (opcode->got)
- return;
- if (!insn->prefixes.got)
- insn_get_prefixes(insn);
+ return 0;
+
+ if (!insn->prefixes.got) {
+ ret = insn_get_prefixes(insn);
+ if (ret)
+ return ret;
+ }
/* Get first opcode */
op = get_next(insn_byte_t, insn);
@@ -255,9 +273,13 @@ void insn_get_opcode(struct insn *insn)
insn->attr = inat_get_avx_attribute(op, m, p);
if ((inat_must_evex(insn->attr) && !insn_is_evex(insn)) ||
(!inat_accept_vex(insn->attr) &&
- !inat_is_group(insn->attr)))
- insn->attr = 0; /* This instruction is bad */
- goto end; /* VEX has only 1 byte for opcode */
+ !inat_is_group(insn->attr))) {
+ /* This instruction is bad */
+ insn->attr = 0;
+ return -EINVAL;
+ }
+ /* VEX has only 1 byte for opcode */
+ goto end;
}
insn->attr = inat_get_opcode_attribute(op);
@@ -268,13 +290,18 @@ void insn_get_opcode(struct insn *insn)
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_escape_attribute(op, pfx_id, insn->attr);
}
- if (inat_must_vex(insn->attr))
- insn->attr = 0; /* This instruction is bad */
+
+ if (inat_must_vex(insn->attr)) {
+ /* This instruction is bad */
+ insn->attr = 0;
+ return -EINVAL;
+ }
end:
opcode->got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -284,15 +311,25 @@ void insn_get_opcode(struct insn *insn)
* Populates @insn->modrm and updates @insn->next_byte to point past the
* ModRM byte, if any. If necessary, first collects the preceding bytes
* (prefixes and opcode(s)). No effect if @insn->modrm.got is already 1.
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_modrm(struct insn *insn)
+int insn_get_modrm(struct insn *insn)
{
struct insn_field *modrm = &insn->modrm;
insn_byte_t pfx_id, mod;
+ int ret;
+
if (modrm->got)
- return;
- if (!insn->opcode.got)
- insn_get_opcode(insn);
+ return 0;
+
+ if (!insn->opcode.got) {
+ ret = insn_get_opcode(insn);
+ if (ret)
+ return ret;
+ }
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
@@ -302,17 +339,22 @@ void insn_get_modrm(struct insn *insn)
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_group_attribute(mod, pfx_id,
insn->attr);
- if (insn_is_avx(insn) && !inat_accept_vex(insn->attr))
- insn->attr = 0; /* This is bad */
+ if (insn_is_avx(insn) && !inat_accept_vex(insn->attr)) {
+ /* Bad insn */
+ insn->attr = 0;
+ return -EINVAL;
+ }
}
}
if (insn->x86_64 && inat_is_force64(insn->attr))
insn->opnd_bytes = 8;
+
modrm->got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
@@ -326,11 +368,16 @@ void insn_get_modrm(struct insn *insn)
int insn_rip_relative(struct insn *insn)
{
struct insn_field *modrm = &insn->modrm;
+ int ret;
if (!insn->x86_64)
return 0;
- if (!modrm->got)
- insn_get_modrm(insn);
+
+ if (!modrm->got) {
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return 0;
+ }
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
@@ -344,15 +391,25 @@ int insn_rip_relative(struct insn *insn)
*
* If necessary, first collects the instruction up to and including the
* ModRM byte.
+ *
+ * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
*/
-void insn_get_sib(struct insn *insn)
+int insn_get_sib(struct insn *insn)
{
insn_byte_t modrm;
+ int ret;
if (insn->sib.got)
- return;
- if (!insn->modrm.got)
- insn_get_modrm(insn);
+ return 0;
+
+ if (!insn->modrm.got) {
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
+ }
+
if (insn->modrm.nbytes) {
modrm = (insn_byte_t)insn->modrm.value;
if (insn->addr_bytes != 2 &&
@@ -363,8 +420,10 @@ void insn_get_sib(struct insn *insn)
}
insn->sib.got = 1;
+ return 0;
+
err_out:
- return;
+ return -ENODATA;
}
@@ -375,15 +434,25 @@ void insn_get_sib(struct insn *insn)
* If necessary, first collects the instruction up to and including the
* SIB byte.
* Displacement value is sign-expanded.
+ *
+ * * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
*/
-void insn_get_displacement(struct insn *insn)
+int insn_get_displacement(struct insn *insn)
{
insn_byte_t mod, rm, base;
+ int ret;
if (insn->displacement.got)
- return;
- if (!insn->sib.got)
- insn_get_sib(insn);
+ return 0;
+
+ if (!insn->sib.got) {
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
+ }
+
if (insn->modrm.nbytes) {
/*
* Interpreting the modrm byte:
@@ -426,9 +495,10 @@ void insn_get_displacement(struct insn *insn)
}
out:
insn->displacement.got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/* Decode moffset16/32/64. Return 0 if failed */
@@ -539,20 +609,30 @@ static int __get_immptr(struct insn *insn)
}
/**
- * insn_get_immediate() - Get the immediates of instruction
+ * insn_get_immediate() - Get the immediate in an instruction
* @insn: &struct insn containing instruction
*
* If necessary, first collects the instruction up to and including the
* displacement bytes.
* Basically, most of immediates are sign-expanded. Unsigned-value can be
- * get by bit masking with ((1 << (nbytes * 8)) - 1)
+ * computed by bit masking with ((1 << (nbytes * 8)) - 1)
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_immediate(struct insn *insn)
+int insn_get_immediate(struct insn *insn)
{
+ int ret;
+
if (insn->immediate.got)
- return;
- if (!insn->displacement.got)
- insn_get_displacement(insn);
+ return 0;
+
+ if (!insn->displacement.got) {
+ ret = insn_get_displacement(insn);
+ if (ret)
+ return ret;
+ }
if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
@@ -605,9 +685,10 @@ void insn_get_immediate(struct insn *insn)
}
done:
insn->immediate.got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -616,13 +697,58 @@ void insn_get_immediate(struct insn *insn)
*
* If necessary, first collects the instruction up to and including the
* immediates bytes.
- */
-void insn_get_length(struct insn *insn)
+ *
+ * Returns:
+ * - 0 on success
+ * - < 0 on error
+*/
+int insn_get_length(struct insn *insn)
{
+ int ret;
+
if (insn->length)
- return;
- if (!insn->immediate.got)
- insn_get_immediate(insn);
+ return 0;
+
+ if (!insn->immediate.got) {
+ ret = insn_get_immediate(insn);
+ if (ret)
+ return ret;
+ }
+
insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
+
+ return 0;
+}
+
+/**
+ * insn_decode() - Decode an x86 instruction
+ * @insn: &struct insn to be initialized
+ * @kaddr: address (in kernel memory) of instruction (or copy thereof)
+ * @buf_len: length of the insn buffer at @kaddr
+ * @m: insn mode, see enum insn_mode
+ *
+ * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
+ */
+int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m)
+{
+ int ret;
+
+/* #define INSN_MODE_KERN -1 __ignore_sync_check__ mode is only valid in the kernel */
+
+ if (m == INSN_MODE_KERN)
+ insn_init(insn, kaddr, buf_len, IS_ENABLED(CONFIG_X86_64));
+ else
+ insn_init(insn, kaddr, buf_len, m == INSN_MODE_64);
+
+ ret = insn_get_length(insn);
+ if (ret)
+ return ret;
+
+ if (insn_complete(insn))
+ return 0;
+
+ return -EINVAL;
}
diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h
index 33d41982a5dd..621ab64a6d27 100644
--- a/tools/arch/x86/include/asm/insn.h
+++ b/tools/arch/x86/include/asm/insn.h
@@ -87,13 +87,23 @@ struct insn {
#define X86_VEX_M_MAX 0x1f /* VEX3.M Maximum value */
extern void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64);
-extern void insn_get_prefixes(struct insn *insn);
-extern void insn_get_opcode(struct insn *insn);
-extern void insn_get_modrm(struct insn *insn);
-extern void insn_get_sib(struct insn *insn);
-extern void insn_get_displacement(struct insn *insn);
-extern void insn_get_immediate(struct insn *insn);
-extern void insn_get_length(struct insn *insn);
+extern int insn_get_prefixes(struct insn *insn);
+extern int insn_get_opcode(struct insn *insn);
+extern int insn_get_modrm(struct insn *insn);
+extern int insn_get_sib(struct insn *insn);
+extern int insn_get_displacement(struct insn *insn);
+extern int insn_get_immediate(struct insn *insn);
+extern int insn_get_length(struct insn *insn);
+
+enum insn_mode {
+ INSN_MODE_32,
+ INSN_MODE_64,
+ /* Mode is determined by the current kernel build. */
+ INSN_MODE_KERN,
+ INSN_NUM_MODES,
+};
+
+extern int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m);
/* Attribute will be determined after getting ModRM (for opcode groups) */
static inline void insn_get_attribute(struct insn *insn)
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index dedcfd67f90c..155e50aefd84 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -10,10 +10,13 @@
#else
#include <string.h>
#endif
-#include "../include/asm/inat.h" /* __ignore_sync_check__ */
-#include "../include/asm/insn.h" /* __ignore_sync_check__ */
+#include <asm/inat.h> /*__ignore_sync_check__ */
+#include <asm/insn.h> /* __ignore_sync_check__ */
-#include "../include/asm/emulate_prefix.h" /* __ignore_sync_check__ */
+#include <linux/errno.h>
+#include <linux/kconfig.h>
+
+#include <asm/emulate_prefix.h> /* __ignore_sync_check__ */
/* Verify next sizeof(t) bytes can be on the same instruction */
#define validate_next(t, insn, n) \
@@ -98,8 +101,12 @@ static void insn_get_emulate_prefix(struct insn *insn)
* Populates the @insn->prefixes bitmap, and updates @insn->next_byte
* to point to the (first) opcode. No effect if @insn->prefixes.got
* is already set.
+ *
+ * * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_prefixes(struct insn *insn)
+int insn_get_prefixes(struct insn *insn)
{
struct insn_field *prefixes = &insn->prefixes;
insn_attr_t attr;
@@ -107,7 +114,7 @@ void insn_get_prefixes(struct insn *insn)
int i, nb;
if (prefixes->got)
- return;
+ return 0;
insn_get_emulate_prefix(insn);
@@ -218,8 +225,10 @@ void insn_get_prefixes(struct insn *insn)
prefixes->got = 1;
+ return 0;
+
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -231,16 +240,25 @@ void insn_get_prefixes(struct insn *insn)
* If necessary, first collects any preceding (prefix) bytes.
* Sets @insn->opcode.value = opcode1. No effect if @insn->opcode.got
* is already 1.
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_opcode(struct insn *insn)
+int insn_get_opcode(struct insn *insn)
{
struct insn_field *opcode = &insn->opcode;
+ int pfx_id, ret;
insn_byte_t op;
- int pfx_id;
+
if (opcode->got)
- return;
- if (!insn->prefixes.got)
- insn_get_prefixes(insn);
+ return 0;
+
+ if (!insn->prefixes.got) {
+ ret = insn_get_prefixes(insn);
+ if (ret)
+ return ret;
+ }
/* Get first opcode */
op = get_next(insn_byte_t, insn);
@@ -255,9 +273,13 @@ void insn_get_opcode(struct insn *insn)
insn->attr = inat_get_avx_attribute(op, m, p);
if ((inat_must_evex(insn->attr) && !insn_is_evex(insn)) ||
(!inat_accept_vex(insn->attr) &&
- !inat_is_group(insn->attr)))
- insn->attr = 0; /* This instruction is bad */
- goto end; /* VEX has only 1 byte for opcode */
+ !inat_is_group(insn->attr))) {
+ /* This instruction is bad */
+ insn->attr = 0;
+ return -EINVAL;
+ }
+ /* VEX has only 1 byte for opcode */
+ goto end;
}
insn->attr = inat_get_opcode_attribute(op);
@@ -268,13 +290,18 @@ void insn_get_opcode(struct insn *insn)
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_escape_attribute(op, pfx_id, insn->attr);
}
- if (inat_must_vex(insn->attr))
- insn->attr = 0; /* This instruction is bad */
+
+ if (inat_must_vex(insn->attr)) {
+ /* This instruction is bad */
+ insn->attr = 0;
+ return -EINVAL;
+ }
end:
opcode->got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -284,15 +311,25 @@ void insn_get_opcode(struct insn *insn)
* Populates @insn->modrm and updates @insn->next_byte to point past the
* ModRM byte, if any. If necessary, first collects the preceding bytes
* (prefixes and opcode(s)). No effect if @insn->modrm.got is already 1.
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_modrm(struct insn *insn)
+int insn_get_modrm(struct insn *insn)
{
struct insn_field *modrm = &insn->modrm;
insn_byte_t pfx_id, mod;
+ int ret;
+
if (modrm->got)
- return;
- if (!insn->opcode.got)
- insn_get_opcode(insn);
+ return 0;
+
+ if (!insn->opcode.got) {
+ ret = insn_get_opcode(insn);
+ if (ret)
+ return ret;
+ }
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
@@ -302,17 +339,22 @@ void insn_get_modrm(struct insn *insn)
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_group_attribute(mod, pfx_id,
insn->attr);
- if (insn_is_avx(insn) && !inat_accept_vex(insn->attr))
- insn->attr = 0; /* This is bad */
+ if (insn_is_avx(insn) && !inat_accept_vex(insn->attr)) {
+ /* Bad insn */
+ insn->attr = 0;
+ return -EINVAL;
+ }
}
}
if (insn->x86_64 && inat_is_force64(insn->attr))
insn->opnd_bytes = 8;
+
modrm->got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
@@ -326,11 +368,16 @@ void insn_get_modrm(struct insn *insn)
int insn_rip_relative(struct insn *insn)
{
struct insn_field *modrm = &insn->modrm;
+ int ret;
if (!insn->x86_64)
return 0;
- if (!modrm->got)
- insn_get_modrm(insn);
+
+ if (!modrm->got) {
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return 0;
+ }
/*
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
@@ -344,15 +391,25 @@ int insn_rip_relative(struct insn *insn)
*
* If necessary, first collects the instruction up to and including the
* ModRM byte.
+ *
+ * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
*/
-void insn_get_sib(struct insn *insn)
+int insn_get_sib(struct insn *insn)
{
insn_byte_t modrm;
+ int ret;
if (insn->sib.got)
- return;
- if (!insn->modrm.got)
- insn_get_modrm(insn);
+ return 0;
+
+ if (!insn->modrm.got) {
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
+ }
+
if (insn->modrm.nbytes) {
modrm = (insn_byte_t)insn->modrm.value;
if (insn->addr_bytes != 2 &&
@@ -363,8 +420,10 @@ void insn_get_sib(struct insn *insn)
}
insn->sib.got = 1;
+ return 0;
+
err_out:
- return;
+ return -ENODATA;
}
@@ -375,15 +434,25 @@ void insn_get_sib(struct insn *insn)
* If necessary, first collects the instruction up to and including the
* SIB byte.
* Displacement value is sign-expanded.
+ *
+ * * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
*/
-void insn_get_displacement(struct insn *insn)
+int insn_get_displacement(struct insn *insn)
{
insn_byte_t mod, rm, base;
+ int ret;
if (insn->displacement.got)
- return;
- if (!insn->sib.got)
- insn_get_sib(insn);
+ return 0;
+
+ if (!insn->sib.got) {
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
+ }
+
if (insn->modrm.nbytes) {
/*
* Interpreting the modrm byte:
@@ -426,9 +495,10 @@ void insn_get_displacement(struct insn *insn)
}
out:
insn->displacement.got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/* Decode moffset16/32/64. Return 0 if failed */
@@ -539,20 +609,30 @@ static int __get_immptr(struct insn *insn)
}
/**
- * insn_get_immediate() - Get the immediates of instruction
+ * insn_get_immediate() - Get the immediate in an instruction
* @insn: &struct insn containing instruction
*
* If necessary, first collects the instruction up to and including the
* displacement bytes.
* Basically, most of immediates are sign-expanded. Unsigned-value can be
- * get by bit masking with ((1 << (nbytes * 8)) - 1)
+ * computed by bit masking with ((1 << (nbytes * 8)) - 1)
+ *
+ * Returns:
+ * 0: on success
+ * < 0: on error
*/
-void insn_get_immediate(struct insn *insn)
+int insn_get_immediate(struct insn *insn)
{
+ int ret;
+
if (insn->immediate.got)
- return;
- if (!insn->displacement.got)
- insn_get_displacement(insn);
+ return 0;
+
+ if (!insn->displacement.got) {
+ ret = insn_get_displacement(insn);
+ if (ret)
+ return ret;
+ }
if (inat_has_moffset(insn->attr)) {
if (!__get_moffset(insn))
@@ -605,9 +685,10 @@ void insn_get_immediate(struct insn *insn)
}
done:
insn->immediate.got = 1;
+ return 0;
err_out:
- return;
+ return -ENODATA;
}
/**
@@ -616,13 +697,58 @@ void insn_get_immediate(struct insn *insn)
*
* If necessary, first collects the instruction up to and including the
* immediates bytes.
- */
-void insn_get_length(struct insn *insn)
+ *
+ * Returns:
+ * - 0 on success
+ * - < 0 on error
+*/
+int insn_get_length(struct insn *insn)
{
+ int ret;
+
if (insn->length)
- return;
- if (!insn->immediate.got)
- insn_get_immediate(insn);
+ return 0;
+
+ if (!insn->immediate.got) {
+ ret = insn_get_immediate(insn);
+ if (ret)
+ return ret;
+ }
+
insn->length = (unsigned char)((unsigned long)insn->next_byte
- (unsigned long)insn->kaddr);
+
+ return 0;
+}
+
+/**
+ * insn_decode() - Decode an x86 instruction
+ * @insn: &struct insn to be initialized
+ * @kaddr: address (in kernel memory) of instruction (or copy thereof)
+ * @buf_len: length of the insn buffer at @kaddr
+ * @m: insn mode, see enum insn_mode
+ *
+ * Returns:
+ * 0: if decoding succeeded
+ * < 0: otherwise.
+ */
+int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m)
+{
+ int ret;
+
+#define INSN_MODE_KERN (enum insn_mode)-1 /* __ignore_sync_check__ mode is only valid in the kernel */
+
+ if (m == INSN_MODE_KERN)
+ insn_init(insn, kaddr, buf_len, IS_ENABLED(CONFIG_X86_64));
+ else
+ insn_init(insn, kaddr, buf_len, m == INSN_MODE_64);
+
+ ret = insn_get_length(insn);
+ if (ret)
+ return ret;
+
+ if (insn_complete(insn))
+ return 0;
+
+ return -EINVAL;
}
diff --git a/tools/include/linux/kconfig.h b/tools/include/linux/kconfig.h
new file mode 100644
index 000000000000..1555a0c4f345
--- /dev/null
+++ b/tools/include/linux/kconfig.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TOOLS_LINUX_KCONFIG_H
+#define _TOOLS_LINUX_KCONFIG_H
+
+/* CONFIG_CC_VERSION_TEXT (Do not delete this comment. See help in Kconfig) */
+
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#define __BIG_ENDIAN 4321
+#else
+#define __LITTLE_ENDIAN 1234
+#endif
+
+#define __ARG_PLACEHOLDER_1 0,
+#define __take_second_arg(__ignored, val, ...) val
+
+/*
+ * The use of "&&" / "||" is limited in certain expressions.
+ * The following enable to calculate "and" / "or" with macro expansion only.
+ */
+#define __and(x, y) ___and(x, y)
+#define ___and(x, y) ____and(__ARG_PLACEHOLDER_##x, y)
+#define ____and(arg1_or_junk, y) __take_second_arg(arg1_or_junk y, 0)
+
+#define __or(x, y) ___or(x, y)
+#define ___or(x, y) ____or(__ARG_PLACEHOLDER_##x, y)
+#define ____or(arg1_or_junk, y) __take_second_arg(arg1_or_junk 1, y)
+
+/*
+ * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
+ * these only work with boolean and tristate options.
+ */
+
+/*
+ * Getting something that works in C and CPP for an arg that may or may
+ * not be defined is tricky. Here, if we have "#define CONFIG_BOOGER 1"
+ * we match on the placeholder define, insert the "0," for arg1 and generate
+ * the triplet (0, 1, 0). Then the last step cherry picks the 2nd arg (a one).
+ * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
+ * the last step cherry picks the 2nd arg, we get a zero.
+ */
+#define __is_defined(x) ___is_defined(x)
+#define ___is_defined(val) ____is_defined(__ARG_PLACEHOLDER_##val)
+#define ____is_defined(arg1_or_junk) __take_second_arg(arg1_or_junk 1, 0)
+
+/*
+ * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
+ * otherwise. For boolean options, this is equivalent to
+ * IS_ENABLED(CONFIG_FOO).
+ */
+#define IS_BUILTIN(option) __is_defined(option)
+
+/*
+ * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
+ * otherwise.
+ */
+#define IS_MODULE(option) __is_defined(option##_MODULE)
+
+/*
+ * IS_REACHABLE(CONFIG_FOO) evaluates to 1 if the currently compiled
+ * code can call a function defined in code compiled based on CONFIG_FOO.
+ * This is similar to IS_ENABLED(), but returns false when invoked from
+ * built-in code when CONFIG_FOO is set to 'm'.
+ */
+#define IS_REACHABLE(option) __or(IS_BUILTIN(option), \
+ __and(IS_MODULE(option), __is_defined(MODULE)))
+
+/*
+ * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
+ * 0 otherwise.
+ */
+#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
+
+#endif /* _TOOLS_LINUX_KCONFIG_H */
--
2.29.2
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v2 04/21] x86/insn: Add an insn_decode() API
2021-02-26 18:30 ` Borislav Petkov
@ 2021-02-28 14:51 ` Masami Hiramatsu
0 siblings, 0 replies; 25+ messages in thread
From: Masami Hiramatsu @ 2021-02-28 14:51 UTC (permalink / raw)
To: Borislav Petkov; +Cc: X86 ML, LKML
On Fri, 26 Feb 2021 19:30:06 +0100
Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Feb 27, 2021 at 12:45:06AM +0900, Masami Hiramatsu wrote:
> > OK, but I think it should return -EINVAL or -EILSEQ for bad instruction.
>
> It does return -EINVAL when insn_complete() returns 0.
>
> > Here you return 1 for a bad opcode.
>
> Whoops, that's a leftover from the early version where it would return
> !0 on error. Lemme fix that.
>
> > Ditto.
> > Would you mean -EINVAL?
>
> Yap.
>
> > Also, __get_*() functions are expected to return bool (1/0)
> > for checking bad data. See insn_get_immediate() INAT_IMM_PTR case for example.
>
> Yuck, I totally snafu'ed that, sorry ;-\
>
> Ok, see below. The patterns look correct now, I'll give it another look
> tomorrow again, to make sure I haven't missed anything.
>
> Thx for catching those!
Thank you for fixing!
>
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Tue, 3 Nov 2020 17:28:30 +0100
> Subject: [PATCH] x86/insn: Add an insn_decode() API
>
> Users of the instruction decoder should use this to decode instruction
> bytes. For that, have insn*() helpers return an int value to denote
> success/failure. When there's an error fetching the next insn byte and
> the insn falls short, return -ENODATA to denote that.
>
> While at it, make insn_get_opcode() more stricter as to whether what has
> seen so far is a valid insn and if not.
>
> Copy linux/kconfig.h for the tools-version of the decoder so that it can
> use IS_ENABLED().
>
> Also, cast the INSN_MODE_KERN dummy define value to (enum insn_mode)
> for tools use of the decoder because perf tool builds with -Werror and
> errors out with -Werror=sign-compare otherwise.
>
This version looks good to me.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you!
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
> arch/x86/include/asm/insn.h | 24 +++-
> arch/x86/lib/insn.c | 216 +++++++++++++++++++++++------
> tools/arch/x86/include/asm/insn.h | 24 +++-
> tools/arch/x86/lib/insn.c | 222 +++++++++++++++++++++++-------
> tools/include/linux/kconfig.h | 73 ++++++++++
> 5 files changed, 452 insertions(+), 107 deletions(-)
> create mode 100644 tools/include/linux/kconfig.h
>
> diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
> index 17c130f1ba57..546436b3c215 100644
> --- a/arch/x86/include/asm/insn.h
> +++ b/arch/x86/include/asm/insn.h
> @@ -87,13 +87,23 @@ struct insn {
> #define X86_VEX_M_MAX 0x1f /* VEX3.M Maximum value */
>
> extern void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64);
> -extern void insn_get_prefixes(struct insn *insn);
> -extern void insn_get_opcode(struct insn *insn);
> -extern void insn_get_modrm(struct insn *insn);
> -extern void insn_get_sib(struct insn *insn);
> -extern void insn_get_displacement(struct insn *insn);
> -extern void insn_get_immediate(struct insn *insn);
> -extern void insn_get_length(struct insn *insn);
> +extern int insn_get_prefixes(struct insn *insn);
> +extern int insn_get_opcode(struct insn *insn);
> +extern int insn_get_modrm(struct insn *insn);
> +extern int insn_get_sib(struct insn *insn);
> +extern int insn_get_displacement(struct insn *insn);
> +extern int insn_get_immediate(struct insn *insn);
> +extern int insn_get_length(struct insn *insn);
> +
> +enum insn_mode {
> + INSN_MODE_32,
> + INSN_MODE_64,
> + /* Mode is determined by the current kernel build. */
> + INSN_MODE_KERN,
> + INSN_NUM_MODES,
> +};
> +
> +extern int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m);
>
> /* Attribute will be determined after getting ModRM (for opcode groups) */
> static inline void insn_get_attribute(struct insn *insn)
> diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
> index 745c704f7c78..9a395c39a0c0 100644
> --- a/arch/x86/lib/insn.c
> +++ b/arch/x86/lib/insn.c
> @@ -13,6 +13,9 @@
> #include <asm/inat.h> /*__ignore_sync_check__ */
> #include <asm/insn.h> /* __ignore_sync_check__ */
>
> +#include <linux/errno.h>
> +#include <linux/kconfig.h>
> +
> #include <asm/emulate_prefix.h> /* __ignore_sync_check__ */
>
> /* Verify next sizeof(t) bytes can be on the same instruction */
> @@ -98,8 +101,12 @@ static void insn_get_emulate_prefix(struct insn *insn)
> * Populates the @insn->prefixes bitmap, and updates @insn->next_byte
> * to point to the (first) opcode. No effect if @insn->prefixes.got
> * is already set.
> + *
> + * * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_prefixes(struct insn *insn)
> +int insn_get_prefixes(struct insn *insn)
> {
> struct insn_field *prefixes = &insn->prefixes;
> insn_attr_t attr;
> @@ -107,7 +114,7 @@ void insn_get_prefixes(struct insn *insn)
> int i, nb;
>
> if (prefixes->got)
> - return;
> + return 0;
>
> insn_get_emulate_prefix(insn);
>
> @@ -218,8 +225,10 @@ void insn_get_prefixes(struct insn *insn)
>
> prefixes->got = 1;
>
> + return 0;
> +
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /**
> @@ -231,16 +240,25 @@ void insn_get_prefixes(struct insn *insn)
> * If necessary, first collects any preceding (prefix) bytes.
> * Sets @insn->opcode.value = opcode1. No effect if @insn->opcode.got
> * is already 1.
> + *
> + * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_opcode(struct insn *insn)
> +int insn_get_opcode(struct insn *insn)
> {
> struct insn_field *opcode = &insn->opcode;
> + int pfx_id, ret;
> insn_byte_t op;
> - int pfx_id;
> +
> if (opcode->got)
> - return;
> - if (!insn->prefixes.got)
> - insn_get_prefixes(insn);
> + return 0;
> +
> + if (!insn->prefixes.got) {
> + ret = insn_get_prefixes(insn);
> + if (ret)
> + return ret;
> + }
>
> /* Get first opcode */
> op = get_next(insn_byte_t, insn);
> @@ -255,9 +273,13 @@ void insn_get_opcode(struct insn *insn)
> insn->attr = inat_get_avx_attribute(op, m, p);
> if ((inat_must_evex(insn->attr) && !insn_is_evex(insn)) ||
> (!inat_accept_vex(insn->attr) &&
> - !inat_is_group(insn->attr)))
> - insn->attr = 0; /* This instruction is bad */
> - goto end; /* VEX has only 1 byte for opcode */
> + !inat_is_group(insn->attr))) {
> + /* This instruction is bad */
> + insn->attr = 0;
> + return -EINVAL;
> + }
> + /* VEX has only 1 byte for opcode */
> + goto end;
> }
>
> insn->attr = inat_get_opcode_attribute(op);
> @@ -268,13 +290,18 @@ void insn_get_opcode(struct insn *insn)
> pfx_id = insn_last_prefix_id(insn);
> insn->attr = inat_get_escape_attribute(op, pfx_id, insn->attr);
> }
> - if (inat_must_vex(insn->attr))
> - insn->attr = 0; /* This instruction is bad */
> +
> + if (inat_must_vex(insn->attr)) {
> + /* This instruction is bad */
> + insn->attr = 0;
> + return -EINVAL;
> + }
> end:
> opcode->got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /**
> @@ -284,15 +311,25 @@ void insn_get_opcode(struct insn *insn)
> * Populates @insn->modrm and updates @insn->next_byte to point past the
> * ModRM byte, if any. If necessary, first collects the preceding bytes
> * (prefixes and opcode(s)). No effect if @insn->modrm.got is already 1.
> + *
> + * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_modrm(struct insn *insn)
> +int insn_get_modrm(struct insn *insn)
> {
> struct insn_field *modrm = &insn->modrm;
> insn_byte_t pfx_id, mod;
> + int ret;
> +
> if (modrm->got)
> - return;
> - if (!insn->opcode.got)
> - insn_get_opcode(insn);
> + return 0;
> +
> + if (!insn->opcode.got) {
> + ret = insn_get_opcode(insn);
> + if (ret)
> + return ret;
> + }
>
> if (inat_has_modrm(insn->attr)) {
> mod = get_next(insn_byte_t, insn);
> @@ -302,17 +339,22 @@ void insn_get_modrm(struct insn *insn)
> pfx_id = insn_last_prefix_id(insn);
> insn->attr = inat_get_group_attribute(mod, pfx_id,
> insn->attr);
> - if (insn_is_avx(insn) && !inat_accept_vex(insn->attr))
> - insn->attr = 0; /* This is bad */
> + if (insn_is_avx(insn) && !inat_accept_vex(insn->attr)) {
> + /* Bad insn */
> + insn->attr = 0;
> + return -EINVAL;
> + }
> }
> }
>
> if (insn->x86_64 && inat_is_force64(insn->attr))
> insn->opnd_bytes = 8;
> +
> modrm->got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
>
> @@ -326,11 +368,16 @@ void insn_get_modrm(struct insn *insn)
> int insn_rip_relative(struct insn *insn)
> {
> struct insn_field *modrm = &insn->modrm;
> + int ret;
>
> if (!insn->x86_64)
> return 0;
> - if (!modrm->got)
> - insn_get_modrm(insn);
> +
> + if (!modrm->got) {
> + ret = insn_get_modrm(insn);
> + if (ret)
> + return 0;
> + }
> /*
> * For rip-relative instructions, the mod field (top 2 bits)
> * is zero and the r/m field (bottom 3 bits) is 0x5.
> @@ -344,15 +391,25 @@ int insn_rip_relative(struct insn *insn)
> *
> * If necessary, first collects the instruction up to and including the
> * ModRM byte.
> + *
> + * Returns:
> + * 0: if decoding succeeded
> + * < 0: otherwise.
> */
> -void insn_get_sib(struct insn *insn)
> +int insn_get_sib(struct insn *insn)
> {
> insn_byte_t modrm;
> + int ret;
>
> if (insn->sib.got)
> - return;
> - if (!insn->modrm.got)
> - insn_get_modrm(insn);
> + return 0;
> +
> + if (!insn->modrm.got) {
> + ret = insn_get_modrm(insn);
> + if (ret)
> + return ret;
> + }
> +
> if (insn->modrm.nbytes) {
> modrm = (insn_byte_t)insn->modrm.value;
> if (insn->addr_bytes != 2 &&
> @@ -363,8 +420,10 @@ void insn_get_sib(struct insn *insn)
> }
> insn->sib.got = 1;
>
> + return 0;
> +
> err_out:
> - return;
> + return -ENODATA;
> }
>
>
> @@ -375,15 +434,25 @@ void insn_get_sib(struct insn *insn)
> * If necessary, first collects the instruction up to and including the
> * SIB byte.
> * Displacement value is sign-expanded.
> + *
> + * * Returns:
> + * 0: if decoding succeeded
> + * < 0: otherwise.
> */
> -void insn_get_displacement(struct insn *insn)
> +int insn_get_displacement(struct insn *insn)
> {
> insn_byte_t mod, rm, base;
> + int ret;
>
> if (insn->displacement.got)
> - return;
> - if (!insn->sib.got)
> - insn_get_sib(insn);
> + return 0;
> +
> + if (!insn->sib.got) {
> + ret = insn_get_sib(insn);
> + if (ret)
> + return ret;
> + }
> +
> if (insn->modrm.nbytes) {
> /*
> * Interpreting the modrm byte:
> @@ -426,9 +495,10 @@ void insn_get_displacement(struct insn *insn)
> }
> out:
> insn->displacement.got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /* Decode moffset16/32/64. Return 0 if failed */
> @@ -539,20 +609,30 @@ static int __get_immptr(struct insn *insn)
> }
>
> /**
> - * insn_get_immediate() - Get the immediates of instruction
> + * insn_get_immediate() - Get the immediate in an instruction
> * @insn: &struct insn containing instruction
> *
> * If necessary, first collects the instruction up to and including the
> * displacement bytes.
> * Basically, most of immediates are sign-expanded. Unsigned-value can be
> - * get by bit masking with ((1 << (nbytes * 8)) - 1)
> + * computed by bit masking with ((1 << (nbytes * 8)) - 1)
> + *
> + * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_immediate(struct insn *insn)
> +int insn_get_immediate(struct insn *insn)
> {
> + int ret;
> +
> if (insn->immediate.got)
> - return;
> - if (!insn->displacement.got)
> - insn_get_displacement(insn);
> + return 0;
> +
> + if (!insn->displacement.got) {
> + ret = insn_get_displacement(insn);
> + if (ret)
> + return ret;
> + }
>
> if (inat_has_moffset(insn->attr)) {
> if (!__get_moffset(insn))
> @@ -605,9 +685,10 @@ void insn_get_immediate(struct insn *insn)
> }
> done:
> insn->immediate.got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /**
> @@ -616,13 +697,58 @@ void insn_get_immediate(struct insn *insn)
> *
> * If necessary, first collects the instruction up to and including the
> * immediates bytes.
> - */
> -void insn_get_length(struct insn *insn)
> + *
> + * Returns:
> + * - 0 on success
> + * - < 0 on error
> +*/
> +int insn_get_length(struct insn *insn)
> {
> + int ret;
> +
> if (insn->length)
> - return;
> - if (!insn->immediate.got)
> - insn_get_immediate(insn);
> + return 0;
> +
> + if (!insn->immediate.got) {
> + ret = insn_get_immediate(insn);
> + if (ret)
> + return ret;
> + }
> +
> insn->length = (unsigned char)((unsigned long)insn->next_byte
> - (unsigned long)insn->kaddr);
> +
> + return 0;
> +}
> +
> +/**
> + * insn_decode() - Decode an x86 instruction
> + * @insn: &struct insn to be initialized
> + * @kaddr: address (in kernel memory) of instruction (or copy thereof)
> + * @buf_len: length of the insn buffer at @kaddr
> + * @m: insn mode, see enum insn_mode
> + *
> + * Returns:
> + * 0: if decoding succeeded
> + * < 0: otherwise.
> + */
> +int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m)
> +{
> + int ret;
> +
> +/* #define INSN_MODE_KERN -1 __ignore_sync_check__ mode is only valid in the kernel */
> +
> + if (m == INSN_MODE_KERN)
> + insn_init(insn, kaddr, buf_len, IS_ENABLED(CONFIG_X86_64));
> + else
> + insn_init(insn, kaddr, buf_len, m == INSN_MODE_64);
> +
> + ret = insn_get_length(insn);
> + if (ret)
> + return ret;
> +
> + if (insn_complete(insn))
> + return 0;
> +
> + return -EINVAL;
> }
> diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h
> index 33d41982a5dd..621ab64a6d27 100644
> --- a/tools/arch/x86/include/asm/insn.h
> +++ b/tools/arch/x86/include/asm/insn.h
> @@ -87,13 +87,23 @@ struct insn {
> #define X86_VEX_M_MAX 0x1f /* VEX3.M Maximum value */
>
> extern void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64);
> -extern void insn_get_prefixes(struct insn *insn);
> -extern void insn_get_opcode(struct insn *insn);
> -extern void insn_get_modrm(struct insn *insn);
> -extern void insn_get_sib(struct insn *insn);
> -extern void insn_get_displacement(struct insn *insn);
> -extern void insn_get_immediate(struct insn *insn);
> -extern void insn_get_length(struct insn *insn);
> +extern int insn_get_prefixes(struct insn *insn);
> +extern int insn_get_opcode(struct insn *insn);
> +extern int insn_get_modrm(struct insn *insn);
> +extern int insn_get_sib(struct insn *insn);
> +extern int insn_get_displacement(struct insn *insn);
> +extern int insn_get_immediate(struct insn *insn);
> +extern int insn_get_length(struct insn *insn);
> +
> +enum insn_mode {
> + INSN_MODE_32,
> + INSN_MODE_64,
> + /* Mode is determined by the current kernel build. */
> + INSN_MODE_KERN,
> + INSN_NUM_MODES,
> +};
> +
> +extern int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m);
>
> /* Attribute will be determined after getting ModRM (for opcode groups) */
> static inline void insn_get_attribute(struct insn *insn)
> diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
> index dedcfd67f90c..155e50aefd84 100644
> --- a/tools/arch/x86/lib/insn.c
> +++ b/tools/arch/x86/lib/insn.c
> @@ -10,10 +10,13 @@
> #else
> #include <string.h>
> #endif
> -#include "../include/asm/inat.h" /* __ignore_sync_check__ */
> -#include "../include/asm/insn.h" /* __ignore_sync_check__ */
> +#include <asm/inat.h> /*__ignore_sync_check__ */
> +#include <asm/insn.h> /* __ignore_sync_check__ */
>
> -#include "../include/asm/emulate_prefix.h" /* __ignore_sync_check__ */
> +#include <linux/errno.h>
> +#include <linux/kconfig.h>
> +
> +#include <asm/emulate_prefix.h> /* __ignore_sync_check__ */
>
> /* Verify next sizeof(t) bytes can be on the same instruction */
> #define validate_next(t, insn, n) \
> @@ -98,8 +101,12 @@ static void insn_get_emulate_prefix(struct insn *insn)
> * Populates the @insn->prefixes bitmap, and updates @insn->next_byte
> * to point to the (first) opcode. No effect if @insn->prefixes.got
> * is already set.
> + *
> + * * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_prefixes(struct insn *insn)
> +int insn_get_prefixes(struct insn *insn)
> {
> struct insn_field *prefixes = &insn->prefixes;
> insn_attr_t attr;
> @@ -107,7 +114,7 @@ void insn_get_prefixes(struct insn *insn)
> int i, nb;
>
> if (prefixes->got)
> - return;
> + return 0;
>
> insn_get_emulate_prefix(insn);
>
> @@ -218,8 +225,10 @@ void insn_get_prefixes(struct insn *insn)
>
> prefixes->got = 1;
>
> + return 0;
> +
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /**
> @@ -231,16 +240,25 @@ void insn_get_prefixes(struct insn *insn)
> * If necessary, first collects any preceding (prefix) bytes.
> * Sets @insn->opcode.value = opcode1. No effect if @insn->opcode.got
> * is already 1.
> + *
> + * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_opcode(struct insn *insn)
> +int insn_get_opcode(struct insn *insn)
> {
> struct insn_field *opcode = &insn->opcode;
> + int pfx_id, ret;
> insn_byte_t op;
> - int pfx_id;
> +
> if (opcode->got)
> - return;
> - if (!insn->prefixes.got)
> - insn_get_prefixes(insn);
> + return 0;
> +
> + if (!insn->prefixes.got) {
> + ret = insn_get_prefixes(insn);
> + if (ret)
> + return ret;
> + }
>
> /* Get first opcode */
> op = get_next(insn_byte_t, insn);
> @@ -255,9 +273,13 @@ void insn_get_opcode(struct insn *insn)
> insn->attr = inat_get_avx_attribute(op, m, p);
> if ((inat_must_evex(insn->attr) && !insn_is_evex(insn)) ||
> (!inat_accept_vex(insn->attr) &&
> - !inat_is_group(insn->attr)))
> - insn->attr = 0; /* This instruction is bad */
> - goto end; /* VEX has only 1 byte for opcode */
> + !inat_is_group(insn->attr))) {
> + /* This instruction is bad */
> + insn->attr = 0;
> + return -EINVAL;
> + }
> + /* VEX has only 1 byte for opcode */
> + goto end;
> }
>
> insn->attr = inat_get_opcode_attribute(op);
> @@ -268,13 +290,18 @@ void insn_get_opcode(struct insn *insn)
> pfx_id = insn_last_prefix_id(insn);
> insn->attr = inat_get_escape_attribute(op, pfx_id, insn->attr);
> }
> - if (inat_must_vex(insn->attr))
> - insn->attr = 0; /* This instruction is bad */
> +
> + if (inat_must_vex(insn->attr)) {
> + /* This instruction is bad */
> + insn->attr = 0;
> + return -EINVAL;
> + }
> end:
> opcode->got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /**
> @@ -284,15 +311,25 @@ void insn_get_opcode(struct insn *insn)
> * Populates @insn->modrm and updates @insn->next_byte to point past the
> * ModRM byte, if any. If necessary, first collects the preceding bytes
> * (prefixes and opcode(s)). No effect if @insn->modrm.got is already 1.
> + *
> + * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_modrm(struct insn *insn)
> +int insn_get_modrm(struct insn *insn)
> {
> struct insn_field *modrm = &insn->modrm;
> insn_byte_t pfx_id, mod;
> + int ret;
> +
> if (modrm->got)
> - return;
> - if (!insn->opcode.got)
> - insn_get_opcode(insn);
> + return 0;
> +
> + if (!insn->opcode.got) {
> + ret = insn_get_opcode(insn);
> + if (ret)
> + return ret;
> + }
>
> if (inat_has_modrm(insn->attr)) {
> mod = get_next(insn_byte_t, insn);
> @@ -302,17 +339,22 @@ void insn_get_modrm(struct insn *insn)
> pfx_id = insn_last_prefix_id(insn);
> insn->attr = inat_get_group_attribute(mod, pfx_id,
> insn->attr);
> - if (insn_is_avx(insn) && !inat_accept_vex(insn->attr))
> - insn->attr = 0; /* This is bad */
> + if (insn_is_avx(insn) && !inat_accept_vex(insn->attr)) {
> + /* Bad insn */
> + insn->attr = 0;
> + return -EINVAL;
> + }
> }
> }
>
> if (insn->x86_64 && inat_is_force64(insn->attr))
> insn->opnd_bytes = 8;
> +
> modrm->got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
>
> @@ -326,11 +368,16 @@ void insn_get_modrm(struct insn *insn)
> int insn_rip_relative(struct insn *insn)
> {
> struct insn_field *modrm = &insn->modrm;
> + int ret;
>
> if (!insn->x86_64)
> return 0;
> - if (!modrm->got)
> - insn_get_modrm(insn);
> +
> + if (!modrm->got) {
> + ret = insn_get_modrm(insn);
> + if (ret)
> + return 0;
> + }
> /*
> * For rip-relative instructions, the mod field (top 2 bits)
> * is zero and the r/m field (bottom 3 bits) is 0x5.
> @@ -344,15 +391,25 @@ int insn_rip_relative(struct insn *insn)
> *
> * If necessary, first collects the instruction up to and including the
> * ModRM byte.
> + *
> + * Returns:
> + * 0: if decoding succeeded
> + * < 0: otherwise.
> */
> -void insn_get_sib(struct insn *insn)
> +int insn_get_sib(struct insn *insn)
> {
> insn_byte_t modrm;
> + int ret;
>
> if (insn->sib.got)
> - return;
> - if (!insn->modrm.got)
> - insn_get_modrm(insn);
> + return 0;
> +
> + if (!insn->modrm.got) {
> + ret = insn_get_modrm(insn);
> + if (ret)
> + return ret;
> + }
> +
> if (insn->modrm.nbytes) {
> modrm = (insn_byte_t)insn->modrm.value;
> if (insn->addr_bytes != 2 &&
> @@ -363,8 +420,10 @@ void insn_get_sib(struct insn *insn)
> }
> insn->sib.got = 1;
>
> + return 0;
> +
> err_out:
> - return;
> + return -ENODATA;
> }
>
>
> @@ -375,15 +434,25 @@ void insn_get_sib(struct insn *insn)
> * If necessary, first collects the instruction up to and including the
> * SIB byte.
> * Displacement value is sign-expanded.
> + *
> + * * Returns:
> + * 0: if decoding succeeded
> + * < 0: otherwise.
> */
> -void insn_get_displacement(struct insn *insn)
> +int insn_get_displacement(struct insn *insn)
> {
> insn_byte_t mod, rm, base;
> + int ret;
>
> if (insn->displacement.got)
> - return;
> - if (!insn->sib.got)
> - insn_get_sib(insn);
> + return 0;
> +
> + if (!insn->sib.got) {
> + ret = insn_get_sib(insn);
> + if (ret)
> + return ret;
> + }
> +
> if (insn->modrm.nbytes) {
> /*
> * Interpreting the modrm byte:
> @@ -426,9 +495,10 @@ void insn_get_displacement(struct insn *insn)
> }
> out:
> insn->displacement.got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /* Decode moffset16/32/64. Return 0 if failed */
> @@ -539,20 +609,30 @@ static int __get_immptr(struct insn *insn)
> }
>
> /**
> - * insn_get_immediate() - Get the immediates of instruction
> + * insn_get_immediate() - Get the immediate in an instruction
> * @insn: &struct insn containing instruction
> *
> * If necessary, first collects the instruction up to and including the
> * displacement bytes.
> * Basically, most of immediates are sign-expanded. Unsigned-value can be
> - * get by bit masking with ((1 << (nbytes * 8)) - 1)
> + * computed by bit masking with ((1 << (nbytes * 8)) - 1)
> + *
> + * Returns:
> + * 0: on success
> + * < 0: on error
> */
> -void insn_get_immediate(struct insn *insn)
> +int insn_get_immediate(struct insn *insn)
> {
> + int ret;
> +
> if (insn->immediate.got)
> - return;
> - if (!insn->displacement.got)
> - insn_get_displacement(insn);
> + return 0;
> +
> + if (!insn->displacement.got) {
> + ret = insn_get_displacement(insn);
> + if (ret)
> + return ret;
> + }
>
> if (inat_has_moffset(insn->attr)) {
> if (!__get_moffset(insn))
> @@ -605,9 +685,10 @@ void insn_get_immediate(struct insn *insn)
> }
> done:
> insn->immediate.got = 1;
> + return 0;
>
> err_out:
> - return;
> + return -ENODATA;
> }
>
> /**
> @@ -616,13 +697,58 @@ void insn_get_immediate(struct insn *insn)
> *
> * If necessary, first collects the instruction up to and including the
> * immediates bytes.
> - */
> -void insn_get_length(struct insn *insn)
> + *
> + * Returns:
> + * - 0 on success
> + * - < 0 on error
> +*/
> +int insn_get_length(struct insn *insn)
> {
> + int ret;
> +
> if (insn->length)
> - return;
> - if (!insn->immediate.got)
> - insn_get_immediate(insn);
> + return 0;
> +
> + if (!insn->immediate.got) {
> + ret = insn_get_immediate(insn);
> + if (ret)
> + return ret;
> + }
> +
> insn->length = (unsigned char)((unsigned long)insn->next_byte
> - (unsigned long)insn->kaddr);
> +
> + return 0;
> +}
> +
> +/**
> + * insn_decode() - Decode an x86 instruction
> + * @insn: &struct insn to be initialized
> + * @kaddr: address (in kernel memory) of instruction (or copy thereof)
> + * @buf_len: length of the insn buffer at @kaddr
> + * @m: insn mode, see enum insn_mode
> + *
> + * Returns:
> + * 0: if decoding succeeded
> + * < 0: otherwise.
> + */
> +int insn_decode(struct insn *insn, const void *kaddr, int buf_len, enum insn_mode m)
> +{
> + int ret;
> +
> +#define INSN_MODE_KERN (enum insn_mode)-1 /* __ignore_sync_check__ mode is only valid in the kernel */
> +
> + if (m == INSN_MODE_KERN)
> + insn_init(insn, kaddr, buf_len, IS_ENABLED(CONFIG_X86_64));
> + else
> + insn_init(insn, kaddr, buf_len, m == INSN_MODE_64);
> +
> + ret = insn_get_length(insn);
> + if (ret)
> + return ret;
> +
> + if (insn_complete(insn))
> + return 0;
> +
> + return -EINVAL;
> }
> diff --git a/tools/include/linux/kconfig.h b/tools/include/linux/kconfig.h
> new file mode 100644
> index 000000000000..1555a0c4f345
> --- /dev/null
> +++ b/tools/include/linux/kconfig.h
> @@ -0,0 +1,73 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _TOOLS_LINUX_KCONFIG_H
> +#define _TOOLS_LINUX_KCONFIG_H
> +
> +/* CONFIG_CC_VERSION_TEXT (Do not delete this comment. See help in Kconfig) */
> +
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +#define __BIG_ENDIAN 4321
> +#else
> +#define __LITTLE_ENDIAN 1234
> +#endif
> +
> +#define __ARG_PLACEHOLDER_1 0,
> +#define __take_second_arg(__ignored, val, ...) val
> +
> +/*
> + * The use of "&&" / "||" is limited in certain expressions.
> + * The following enable to calculate "and" / "or" with macro expansion only.
> + */
> +#define __and(x, y) ___and(x, y)
> +#define ___and(x, y) ____and(__ARG_PLACEHOLDER_##x, y)
> +#define ____and(arg1_or_junk, y) __take_second_arg(arg1_or_junk y, 0)
> +
> +#define __or(x, y) ___or(x, y)
> +#define ___or(x, y) ____or(__ARG_PLACEHOLDER_##x, y)
> +#define ____or(arg1_or_junk, y) __take_second_arg(arg1_or_junk 1, y)
> +
> +/*
> + * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
> + * these only work with boolean and tristate options.
> + */
> +
> +/*
> + * Getting something that works in C and CPP for an arg that may or may
> + * not be defined is tricky. Here, if we have "#define CONFIG_BOOGER 1"
> + * we match on the placeholder define, insert the "0," for arg1 and generate
> + * the triplet (0, 1, 0). Then the last step cherry picks the 2nd arg (a one).
> + * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
> + * the last step cherry picks the 2nd arg, we get a zero.
> + */
> +#define __is_defined(x) ___is_defined(x)
> +#define ___is_defined(val) ____is_defined(__ARG_PLACEHOLDER_##val)
> +#define ____is_defined(arg1_or_junk) __take_second_arg(arg1_or_junk 1, 0)
> +
> +/*
> + * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
> + * otherwise. For boolean options, this is equivalent to
> + * IS_ENABLED(CONFIG_FOO).
> + */
> +#define IS_BUILTIN(option) __is_defined(option)
> +
> +/*
> + * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
> + * otherwise.
> + */
> +#define IS_MODULE(option) __is_defined(option##_MODULE)
> +
> +/*
> + * IS_REACHABLE(CONFIG_FOO) evaluates to 1 if the currently compiled
> + * code can call a function defined in code compiled based on CONFIG_FOO.
> + * This is similar to IS_ENABLED(), but returns false when invoked from
> + * built-in code when CONFIG_FOO is set to 'm'.
> + */
> +#define IS_REACHABLE(option) __or(IS_BUILTIN(option), \
> + __and(IS_MODULE(option), __is_defined(MODULE)))
> +
> +/*
> + * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
> + * 0 otherwise.
> + */
> +#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
> +
> +#endif /* _TOOLS_LINUX_KCONFIG_H */
> --
> 2.29.2
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 05/21] x86/insn-eval: Handle return values from the decoder
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (3 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 04/21] x86/insn: Add an insn_decode() API Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 06/21] x86/boot/compressed/sev-es: Convert to insn_decode() Borislav Petkov
` (15 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Now that the different instruction-inspecting functions return a value,
test that and return early from callers if error has been encountered.
While at it, do not call insn_get_modrm() when calling
insn_get_displacement() because latter will make sure to call
insn_get_modrm() if ModRM hasn't been parsed yet.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/lib/insn-eval.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index d0ebf9e93e35..1a6c191a38f8 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -928,10 +928,11 @@ static int get_seg_base_limit(struct insn *insn, struct pt_regs *regs,
static int get_eff_addr_reg(struct insn *insn, struct pt_regs *regs,
int *regoff, long *eff_addr)
{
- insn_get_modrm(insn);
+ int ret;
- if (!insn->modrm.nbytes)
- return -EINVAL;
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
if (X86_MODRM_MOD(insn->modrm.value) != 3)
return -EINVAL;
@@ -977,14 +978,14 @@ static int get_eff_addr_modrm(struct insn *insn, struct pt_regs *regs,
int *regoff, long *eff_addr)
{
long tmp;
+ int ret;
if (insn->addr_bytes != 8 && insn->addr_bytes != 4)
return -EINVAL;
- insn_get_modrm(insn);
-
- if (!insn->modrm.nbytes)
- return -EINVAL;
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
if (X86_MODRM_MOD(insn->modrm.value) > 2)
return -EINVAL;
@@ -1106,18 +1107,21 @@ static int get_eff_addr_modrm_16(struct insn *insn, struct pt_regs *regs,
* @base_offset will have a register, as an offset from the base of pt_regs,
* that can be used to resolve the associated segment.
*
- * -EINVAL on error.
+ * Negative value on error.
*/
static int get_eff_addr_sib(struct insn *insn, struct pt_regs *regs,
int *base_offset, long *eff_addr)
{
long base, indx;
int indx_offset;
+ int ret;
if (insn->addr_bytes != 8 && insn->addr_bytes != 4)
return -EINVAL;
- insn_get_modrm(insn);
+ ret = insn_get_modrm(insn);
+ if (ret)
+ return ret;
if (!insn->modrm.nbytes)
return -EINVAL;
@@ -1125,7 +1129,9 @@ static int get_eff_addr_sib(struct insn *insn, struct pt_regs *regs,
if (X86_MODRM_MOD(insn->modrm.value) > 2)
return -EINVAL;
- insn_get_sib(insn);
+ ret = insn_get_sib(insn);
+ if (ret)
+ return ret;
if (!insn->sib.nbytes)
return -EINVAL;
@@ -1194,8 +1200,8 @@ static void __user *get_addr_ref_16(struct insn *insn, struct pt_regs *regs)
short eff_addr;
long tmp;
- insn_get_modrm(insn);
- insn_get_displacement(insn);
+ if (insn_get_displacement(insn))
+ goto out;
if (insn->addr_bytes != 2)
goto out;
@@ -1491,7 +1497,9 @@ bool insn_decode_from_regs(struct insn *insn, struct pt_regs *regs,
insn->addr_bytes = INSN_CODE_SEG_ADDR_SZ(seg_defs);
insn->opnd_bytes = INSN_CODE_SEG_OPND_SZ(seg_defs);
- insn_get_length(insn);
+ if (insn_get_length(insn))
+ return false;
+
if (buf_size < insn->length)
return false;
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 06/21] x86/boot/compressed/sev-es: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (4 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 05/21] x86/insn-eval: Handle return values from the decoder Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 07/21] perf/x86/intel/ds: Check insn_get_length() retval Borislav Petkov
` (14 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Other than simplifying the code there should be no functional changes
resulting from this.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/boot/compressed/sev-es.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/x86/boot/compressed/sev-es.c b/arch/x86/boot/compressed/sev-es.c
index 27826c265aab..801c626fc3d4 100644
--- a/arch/x86/boot/compressed/sev-es.c
+++ b/arch/x86/boot/compressed/sev-es.c
@@ -78,16 +78,15 @@ static inline void sev_es_wr_ghcb_msr(u64 val)
static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt)
{
char buffer[MAX_INSN_SIZE];
- enum es_result ret;
+ int ret;
memcpy(buffer, (unsigned char *)ctxt->regs->ip, MAX_INSN_SIZE);
- insn_init(&ctxt->insn, buffer, MAX_INSN_SIZE, 1);
- insn_get_length(&ctxt->insn);
+ ret = insn_decode(&ctxt->insn, buffer, MAX_INSN_SIZE, INSN_MODE_64);
+ if (ret < 0)
+ return ES_DECODE_FAILED;
- ret = ctxt->insn.immediate.got ? ES_OK : ES_DECODE_FAILED;
-
- return ret;
+ return ES_OK;
}
static enum es_result vc_write_mem(struct es_em_ctxt *ctxt,
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 07/21] perf/x86/intel/ds: Check insn_get_length() retval
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (5 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 06/21] x86/boot/compressed/sev-es: Convert to insn_decode() Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 08/21] perf/x86/intel/ds: Check return values of insn decoder functions Borislav Petkov
` (13 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
intel_pmu_pebs_fixup_ip() needs only the insn length so use the
appropriate helper instead of a full decode. A full decode differs only
in running insn_complete() on the decoded insn but that is not needed
here.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/events/intel/ds.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 67dbc91bccfe..c1c710f6057b 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -1265,14 +1265,13 @@ static int intel_pmu_pebs_fixup_ip(struct pt_regs *regs)
is_64bit = kernel_ip(to) || any_64bit_mode(regs);
#endif
insn_init(&insn, kaddr, size, is_64bit);
- insn_get_length(&insn);
+
/*
- * Make sure there was not a problem decoding the
- * instruction and getting the length. This is
- * doubly important because we have an infinite
- * loop if insn.length=0.
+ * Make sure there was not a problem decoding the instruction.
+ * This is doubly important because we have an infinite loop if
+ * insn.length=0.
*/
- if (!insn.length)
+ if (insn_get_length(&insn))
break;
to += insn.length;
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 08/21] perf/x86/intel/ds: Check return values of insn decoder functions
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (6 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 07/21] perf/x86/intel/ds: Check insn_get_length() retval Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 09/21] x86/alternative: Use insn_decode() Borislav Petkov
` (12 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
branch_type() doesn't need to call the full insn_decode() because it
doesn't need it in all cases thus leave the calls separate.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/events/intel/lbr.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 21890dacfcfe..9ecf5028fb8f 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1224,8 +1224,7 @@ static int branch_type(unsigned long from, unsigned long to, int abort)
is64 = kernel_ip((unsigned long)addr) || any_64bit_mode(current_pt_regs());
#endif
insn_init(&insn, addr, bytes_read, is64);
- insn_get_opcode(&insn);
- if (!insn.opcode.got)
+ if (insn_get_opcode(&insn))
return X86_BR_ABORT;
switch (insn.opcode.bytes[0]) {
@@ -1262,8 +1261,7 @@ static int branch_type(unsigned long from, unsigned long to, int abort)
ret = X86_BR_INT;
break;
case 0xe8: /* call near rel */
- insn_get_immediate(&insn);
- if (insn.immediate1.value == 0) {
+ if (insn_get_immediate(&insn) || insn.immediate1.value == 0) {
/* zero length call */
ret = X86_BR_ZERO_CALL;
break;
@@ -1279,7 +1277,9 @@ static int branch_type(unsigned long from, unsigned long to, int abort)
ret = X86_BR_JMP;
break;
case 0xff: /* call near absolute, call far absolute ind */
- insn_get_modrm(&insn);
+ if (insn_get_modrm(&insn))
+ return X86_BR_ABORT;
+
ext = (insn.modrm.bytes[0] >> 3) & 0x7;
switch (ext) {
case 2: /* near ind call */
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 09/21] x86/alternative: Use insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (7 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 08/21] perf/x86/intel/ds: Check return values of insn decoder functions Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 10/21] x86/mce: Convert to insn_decode() Borislav Petkov
` (11 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
No functional changes, just simplification.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/kernel/alternative.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 8d778e46725d..ce28c5c1deba 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1274,15 +1274,15 @@ static void text_poke_loc_init(struct text_poke_loc *tp, void *addr,
const void *opcode, size_t len, const void *emulate)
{
struct insn insn;
+ int ret;
memcpy((void *)tp->text, opcode, len);
if (!emulate)
emulate = opcode;
- kernel_insn_init(&insn, emulate, MAX_INSN_SIZE);
- insn_get_length(&insn);
+ ret = insn_decode(&insn, emulate, MAX_INSN_SIZE, INSN_MODE_KERN);
- BUG_ON(!insn_complete(&insn));
+ BUG_ON(ret < 0);
BUG_ON(len != insn.length);
tp->rel_addr = addr - (void *)_stext;
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 10/21] x86/mce: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (8 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 09/21] x86/alternative: Use insn_decode() Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 11/21] x86/kprobes: " Borislav Petkov
` (10 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Simplify code, no functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/kernel/cpu/mce/severity.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
index 83df991314c5..a2136ced9d73 100644
--- a/arch/x86/kernel/cpu/mce/severity.c
+++ b/arch/x86/kernel/cpu/mce/severity.c
@@ -218,15 +218,15 @@ static struct severity {
static bool is_copy_from_user(struct pt_regs *regs)
{
u8 insn_buf[MAX_INSN_SIZE];
- struct insn insn;
unsigned long addr;
+ struct insn insn;
+ int ret;
if (copy_from_kernel_nofault(insn_buf, (void *)regs->ip, MAX_INSN_SIZE))
return false;
- kernel_insn_init(&insn, insn_buf, MAX_INSN_SIZE);
- insn_get_opcode(&insn);
- if (!insn.opcode.got)
+ ret = insn_decode(&insn, insn_buf, MAX_INSN_SIZE, INSN_MODE_KERN);
+ if (ret < 0)
return false;
switch (insn.opcode.value) {
@@ -234,10 +234,6 @@ static bool is_copy_from_user(struct pt_regs *regs)
case 0x8A: case 0x8B:
/* MOVZ mem,reg */
case 0xB60F: case 0xB70F:
- insn_get_modrm(&insn);
- insn_get_sib(&insn);
- if (!insn.modrm.got || !insn.sib.got)
- return false;
addr = (unsigned long)insn_get_addr_ref(&insn, regs);
break;
/* REP MOVS */
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 11/21] x86/kprobes: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (9 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 10/21] x86/mce: Convert to insn_decode() Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 12/21] x86/sev-es: Split vc_decode_insn() Borislav Petkov
` (9 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Simplify code, improve decoding error checking.
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
arch/x86/kernel/kprobes/core.c | 17 +++++++++++------
arch/x86/kernel/kprobes/opt.c | 9 +++++++--
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index a65e9e97857f..1cf4b532b798 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -285,6 +285,8 @@ static int can_probe(unsigned long paddr)
/* Decode instructions */
addr = paddr - offset;
while (addr < paddr) {
+ int ret;
+
/*
* Check if the instruction has been modified by another
* kprobe, in which case we replace the breakpoint by the
@@ -296,8 +298,10 @@ static int can_probe(unsigned long paddr)
__addr = recover_probed_instruction(buf, addr);
if (!__addr)
return 0;
- kernel_insn_init(&insn, (void *)__addr, MAX_INSN_SIZE);
- insn_get_length(&insn);
+
+ ret = insn_decode(&insn, (void *)__addr, MAX_INSN_SIZE, INSN_MODE_KERN);
+ if (ret < 0)
+ return 0;
/*
* Another debugging subsystem might insert this breakpoint.
@@ -340,8 +344,8 @@ static int is_IF_modifier(kprobe_opcode_t *insn)
int __copy_instruction(u8 *dest, u8 *src, u8 *real, struct insn *insn)
{
kprobe_opcode_t buf[MAX_INSN_SIZE];
- unsigned long recovered_insn =
- recover_probed_instruction(buf, (unsigned long)src);
+ unsigned long recovered_insn = recover_probed_instruction(buf, (unsigned long)src);
+ int ret;
if (!recovered_insn || !insn)
return 0;
@@ -351,8 +355,9 @@ int __copy_instruction(u8 *dest, u8 *src, u8 *real, struct insn *insn)
MAX_INSN_SIZE))
return 0;
- kernel_insn_init(insn, dest, MAX_INSN_SIZE);
- insn_get_length(insn);
+ ret = insn_decode(insn, dest, MAX_INSN_SIZE, INSN_MODE_KERN);
+ if (ret < 0)
+ return 0;
/* We can not probe force emulate prefixed instruction */
if (insn_has_emulate_prefix(insn))
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 08eb23074f92..4299fc865732 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -312,6 +312,8 @@ static int can_optimize(unsigned long paddr)
addr = paddr - offset;
while (addr < paddr - offset + size) { /* Decode until function end */
unsigned long recovered_insn;
+ int ret;
+
if (search_exception_tables(addr))
/*
* Since some fixup code will jumps into this function,
@@ -321,8 +323,11 @@ static int can_optimize(unsigned long paddr)
recovered_insn = recover_probed_instruction(buf, addr);
if (!recovered_insn)
return 0;
- kernel_insn_init(&insn, (void *)recovered_insn, MAX_INSN_SIZE);
- insn_get_length(&insn);
+
+ ret = insn_decode(&insn, (void *)recovered_insn, MAX_INSN_SIZE, INSN_MODE_KERN);
+ if (ret < 0)
+ return 0;
+
/*
* In the case of detecting unknown breakpoint, this could be
* a padding INT3 between functions. Let's check that all the
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 12/21] x86/sev-es: Split vc_decode_insn()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (10 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 11/21] x86/kprobes: " Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 13/21] x86/sev-es: Convert to insn_decode() Borislav Petkov
` (8 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Split it into two helpers - a user- and a kernel-mode one for
readability. Yes, the original function body is not that convoluted but
splitting it makes following through that code trivial than having to
pay attention to each little difference when in user or in kernel mode.
No functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/kernel/sev-es.c | 59 ++++++++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 21 deletions(-)
diff --git a/arch/x86/kernel/sev-es.c b/arch/x86/kernel/sev-es.c
index 248d14ae5602..6513e94d53f2 100644
--- a/arch/x86/kernel/sev-es.c
+++ b/arch/x86/kernel/sev-es.c
@@ -241,41 +241,58 @@ static int vc_fetch_insn_kernel(struct es_em_ctxt *ctxt,
return copy_from_kernel_nofault(buffer, (unsigned char *)ctxt->regs->ip, MAX_INSN_SIZE);
}
-static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt)
+static enum es_result __vc_decode_user_insn(struct es_em_ctxt *ctxt)
{
char buffer[MAX_INSN_SIZE];
enum es_result ret;
int res;
- if (user_mode(ctxt->regs)) {
- res = insn_fetch_from_user(ctxt->regs, buffer);
- 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;
- }
+ res = insn_fetch_from_user(ctxt->regs, buffer);
+ 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))
- return ES_DECODE_FAILED;
- } else {
- res = vc_fetch_insn_kernel(ctxt, buffer);
- if (res) {
- ctxt->fi.vector = X86_TRAP_PF;
- ctxt->fi.error_code = X86_PF_INSTR;
- ctxt->fi.cr2 = ctxt->regs->ip;
- return ES_EXCEPTION;
- }
+ if (!insn_decode_from_regs(&ctxt->insn, ctxt->regs, buffer, res))
+ return ES_DECODE_FAILED;
+
+ ret = ctxt->insn.immediate.got ? ES_OK : ES_DECODE_FAILED;
+
+ return ret;
+}
- insn_init(&ctxt->insn, buffer, MAX_INSN_SIZE - res, 1);
- insn_get_length(&ctxt->insn);
+static enum es_result __vc_decode_kern_insn(struct es_em_ctxt *ctxt)
+{
+ char buffer[MAX_INSN_SIZE];
+ enum es_result ret;
+ int res;
+
+ res = vc_fetch_insn_kernel(ctxt, buffer);
+ if (res) {
+ ctxt->fi.vector = X86_TRAP_PF;
+ ctxt->fi.error_code = X86_PF_INSTR;
+ ctxt->fi.cr2 = ctxt->regs->ip;
+ return ES_EXCEPTION;
}
+ insn_init(&ctxt->insn, buffer, MAX_INSN_SIZE - res, 1);
+ insn_get_length(&ctxt->insn);
+
ret = ctxt->insn.immediate.got ? ES_OK : ES_DECODE_FAILED;
return ret;
}
+static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt)
+{
+ if (user_mode(ctxt->regs))
+ return __vc_decode_user_insn(ctxt);
+ else
+ return __vc_decode_kern_insn(ctxt);
+}
+
static enum es_result vc_write_mem(struct es_em_ctxt *ctxt,
char *dst, char *buf, size_t size)
{
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 13/21] x86/sev-es: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (11 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 12/21] x86/sev-es: Split vc_decode_insn() Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 14/21] x86/traps: " Borislav Petkov
` (7 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Simplify code, no functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/kernel/sev-es.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/sev-es.c b/arch/x86/kernel/sev-es.c
index 6513e94d53f2..d2880a305772 100644
--- a/arch/x86/kernel/sev-es.c
+++ b/arch/x86/kernel/sev-es.c
@@ -244,7 +244,6 @@ 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];
- enum es_result ret;
int res;
res = insn_fetch_from_user(ctxt->regs, buffer);
@@ -258,16 +257,16 @@ static enum es_result __vc_decode_user_insn(struct es_em_ctxt *ctxt)
if (!insn_decode_from_regs(&ctxt->insn, ctxt->regs, buffer, res))
return ES_DECODE_FAILED;
- ret = ctxt->insn.immediate.got ? ES_OK : ES_DECODE_FAILED;
-
- return ret;
+ if (ctxt->insn.immediate.got)
+ return ES_OK;
+ else
+ return ES_DECODE_FAILED;
}
static enum es_result __vc_decode_kern_insn(struct es_em_ctxt *ctxt)
{
char buffer[MAX_INSN_SIZE];
- enum es_result ret;
- int res;
+ int res, ret;
res = vc_fetch_insn_kernel(ctxt, buffer);
if (res) {
@@ -277,12 +276,11 @@ static enum es_result __vc_decode_kern_insn(struct es_em_ctxt *ctxt)
return ES_EXCEPTION;
}
- insn_init(&ctxt->insn, buffer, MAX_INSN_SIZE - res, 1);
- insn_get_length(&ctxt->insn);
-
- ret = ctxt->insn.immediate.got ? ES_OK : ES_DECODE_FAILED;
-
- return ret;
+ ret = insn_decode(&ctxt->insn, buffer, MAX_INSN_SIZE, INSN_MODE_64);
+ if (ret < 0)
+ return ES_DECODE_FAILED;
+ else
+ return ES_OK;
}
static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt)
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 14/21] x86/traps: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (12 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 13/21] x86/sev-es: Convert to insn_decode() Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 15/21] x86/uprobes: " Borislav Petkov
` (6 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Simplify code, no functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/kernel/traps.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 7f5aec758f0e..c2a08c4c1872 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -498,14 +498,15 @@ static enum kernel_gp_hint get_kernel_gp_address(struct pt_regs *regs,
{
u8 insn_buf[MAX_INSN_SIZE];
struct insn insn;
+ int ret;
if (copy_from_kernel_nofault(insn_buf, (void *)regs->ip,
MAX_INSN_SIZE))
return GP_NO_HINT;
- kernel_insn_init(&insn, insn_buf, MAX_INSN_SIZE);
- insn_get_modrm(&insn);
- insn_get_sib(&insn);
+ ret = insn_decode(&insn, insn_buf, MAX_INSN_SIZE, INSN_MODE_KERN);
+ if (ret < 0)
+ return GP_NO_HINT;
*addr = (unsigned long)insn_get_addr_ref(&insn, regs);
if (*addr == -1UL)
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 15/21] x86/uprobes: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (13 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 14/21] x86/traps: " Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 16/21] x86/tools/insn_decoder_test: " Borislav Petkov
` (5 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Simplify code, no functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/kernel/uprobes.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index a2b413394917..b63cf8f7745e 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -276,12 +276,12 @@ static bool is_prefix_bad(struct insn *insn)
static int uprobe_init_insn(struct arch_uprobe *auprobe, struct insn *insn, bool x86_64)
{
+ enum insn_mode m = x86_64 ? INSN_MODE_64 : INSN_MODE_32;
u32 volatile *good_insns;
+ int ret;
- insn_init(insn, auprobe->insn, sizeof(auprobe->insn), x86_64);
- /* has the side-effect of processing the entire instruction */
- insn_get_length(insn);
- if (!insn_complete(insn))
+ ret = insn_decode(insn, auprobe->insn, sizeof(auprobe->insn), m);
+ if (ret < 0)
return -ENOEXEC;
if (is_prefix_bad(insn))
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 16/21] x86/tools/insn_decoder_test: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (14 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 15/21] x86/uprobes: " Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 17/21] tools/objtool: " Borislav Petkov
` (4 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Simplify code, no functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/tools/insn_decoder_test.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/x86/tools/insn_decoder_test.c b/arch/x86/tools/insn_decoder_test.c
index 34eda63c124b..472540aeabc2 100644
--- a/arch/x86/tools/insn_decoder_test.c
+++ b/arch/x86/tools/insn_decoder_test.c
@@ -120,7 +120,7 @@ int main(int argc, char **argv)
while (fgets(line, BUFSIZE, stdin)) {
char copy[BUFSIZE], *s, *tab1, *tab2;
- int nb = 0;
+ int nb = 0, ret;
unsigned int b;
if (line[0] == '<') {
@@ -148,10 +148,12 @@ int main(int argc, char **argv)
} else
break;
}
+
/* Decode an instruction */
- insn_init(&insn, insn_buff, sizeof(insn_buff), x86_64);
- insn_get_length(&insn);
- if (insn.length != nb) {
+ ret = insn_decode(&insn, insn_buff, sizeof(insn_buff),
+ x86_64 ? INSN_MODE_64 : INSN_MODE_32);
+
+ if (ret < 0 || insn.length != nb) {
warnings++;
pr_warn("Found an x86 instruction decoder bug, "
"please report this.\n", sym);
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 17/21] tools/objtool: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (15 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 16/21] x86/tools/insn_decoder_test: " Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 18/21] x86/tools/insn_sanity: " Borislav Petkov
` (3 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Simplify code, no functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
tools/objtool/arch/x86/decode.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index cde9c36e40ae..67ee8d2a9e5c 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -90,7 +90,7 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec,
struct list_head *ops_list)
{
struct insn insn;
- int x86_64, sign;
+ int x86_64, sign, ret;
unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0,
rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
modrm_reg = 0, sib = 0;
@@ -101,10 +101,9 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec,
if (x86_64 == -1)
return -1;
- insn_init(&insn, sec->data->d_buf + offset, maxlen, x86_64);
- insn_get_length(&insn);
-
- if (!insn_complete(&insn)) {
+ ret = insn_decode(&insn, sec->data->d_buf + offset, maxlen,
+ x86_64 ? INSN_MODE_64 : INSN_MODE_32);
+ if (ret < 0) {
WARN("can't decode instruction at %s:0x%lx", sec->name, offset);
return -1;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 18/21] x86/tools/insn_sanity: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (16 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 17/21] tools/objtool: " Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 19/21] tools/perf: " Borislav Petkov
` (2 subsequent siblings)
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Simplify code, no functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/tools/insn_sanity.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/tools/insn_sanity.c b/arch/x86/tools/insn_sanity.c
index 185ceba9d289..51309df285b4 100644
--- a/arch/x86/tools/insn_sanity.c
+++ b/arch/x86/tools/insn_sanity.c
@@ -222,8 +222,8 @@ static void parse_args(int argc, char **argv)
int main(int argc, char **argv)
{
+ int insns = 0, ret;
struct insn insn;
- int insns = 0;
int errors = 0;
unsigned long i;
unsigned char insn_buff[MAX_INSN_SIZE * 2];
@@ -241,15 +241,15 @@ int main(int argc, char **argv)
continue;
/* Decode an instruction */
- insn_init(&insn, insn_buff, sizeof(insn_buff), x86_64);
- insn_get_length(&insn);
+ ret = insn_decode(&insn, insn_buff, sizeof(insn_buff),
+ x86_64 ? INSN_MODE_64 : INSN_MODE_32);
if (insn.next_byte <= insn.kaddr ||
insn.kaddr + MAX_INSN_SIZE < insn.next_byte) {
/* Access out-of-range memory */
dump_stream(stderr, "Error: Found an access violation", i, insn_buff, &insn);
errors++;
- } else if (verbose && !insn_complete(&insn))
+ } else if (verbose && ret < 0)
dump_stream(stdout, "Info: Found an undecodable input", i, insn_buff, &insn);
else if (verbose >= 2)
dump_insn(stdout, &insn);
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 19/21] tools/perf: Convert to insn_decode()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (17 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 18/21] x86/tools/insn_sanity: " Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 20/21] x86/insn: Remove kernel_insn_init() Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 21/21] x86/insn: Make insn_complete() static Borislav Petkov
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML, Arnaldo Carvalho de Melo
From: Borislav Petkov <bp@suse.de>
Simplify code, no functional changes.
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/arch/x86/tests/insn-x86.c | 9 ++++-----
tools/perf/arch/x86/util/archinsn.c | 9 +++++----
.../intel-pt-decoder/intel-pt-insn-decoder.c | 17 ++++++++++-------
3 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/tools/perf/arch/x86/tests/insn-x86.c b/tools/perf/arch/x86/tests/insn-x86.c
index 745f29adb14b..6c7970b3acb6 100644
--- a/tools/perf/arch/x86/tests/insn-x86.c
+++ b/tools/perf/arch/x86/tests/insn-x86.c
@@ -95,13 +95,12 @@ static int get_branch(const char *branch_str)
static int test_data_item(struct test_data *dat, int x86_64)
{
struct intel_pt_insn intel_pt_insn;
+ int op, branch, ret;
struct insn insn;
- int op, branch;
- insn_init(&insn, dat->data, MAX_INSN_SIZE, x86_64);
- insn_get_length(&insn);
-
- if (!insn_complete(&insn)) {
+ ret = insn_decode(&insn, dat->data, MAX_INSN_SIZE,
+ x86_64 ? INSN_MODE_64 : INSN_MODE_32);
+ if (ret < 0) {
pr_debug("Failed to decode: %s\n", dat->asm_rep);
return -1;
}
diff --git a/tools/perf/arch/x86/util/archinsn.c b/tools/perf/arch/x86/util/archinsn.c
index 3e6791531ca5..9fb12e8e67eb 100644
--- a/tools/perf/arch/x86/util/archinsn.c
+++ b/tools/perf/arch/x86/util/archinsn.c
@@ -11,7 +11,7 @@ void arch_fetch_insn(struct perf_sample *sample,
struct machine *machine)
{
struct insn insn;
- int len;
+ int len, ret;
bool is64bit = false;
if (!sample->ip)
@@ -19,8 +19,9 @@ void arch_fetch_insn(struct perf_sample *sample,
len = thread__memcpy(thread, machine, sample->insn, sample->ip, sizeof(sample->insn), &is64bit);
if (len <= 0)
return;
- insn_init(&insn, sample->insn, len, is64bit);
- insn_get_length(&insn);
- if (insn_complete(&insn) && insn.length <= len)
+
+ ret = insn_decode(&insn, sample->insn, len,
+ is64bit ? INSN_MODE_64 : INSN_MODE_32);
+ if (ret >= 0 && insn.length <= len)
sample->insn_len = insn.length;
}
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
index fb8a3558d3d5..56b42545946e 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
@@ -158,11 +158,13 @@ int intel_pt_get_insn(const unsigned char *buf, size_t len, int x86_64,
struct intel_pt_insn *intel_pt_insn)
{
struct insn insn;
+ int ret;
- insn_init(&insn, buf, len, x86_64);
- insn_get_length(&insn);
- if (!insn_complete(&insn) || insn.length > len)
+ ret = insn_decode(&insn, buf, len,
+ x86_64 ? INSN_MODE_64 : INSN_MODE_32);
+ if (ret < 0 || insn.length > len)
return -1;
+
intel_pt_insn_decoder(&insn, intel_pt_insn);
if (insn.length < INTEL_PT_INSN_BUF_SZ)
memcpy(intel_pt_insn->buf, buf, insn.length);
@@ -183,12 +185,13 @@ const char *dump_insn(struct perf_insn *x, uint64_t ip __maybe_unused,
u8 *inbuf, int inlen, int *lenp)
{
struct insn insn;
- int n, i;
+ int n, i, ret;
int left;
- insn_init(&insn, inbuf, inlen, x->is64bit);
- insn_get_length(&insn);
- if (!insn_complete(&insn) || insn.length > inlen)
+ ret = insn_decode(&insn, inbuf, inlen,
+ x->is64bit ? INSN_MODE_64 : INSN_MODE_32);
+
+ if (ret < 0 || insn.length > inlen)
return "<bad>";
if (lenp)
*lenp = insn.length;
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 20/21] x86/insn: Remove kernel_insn_init()
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (18 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 19/21] tools/perf: " Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
2021-02-24 11:02 ` [PATCH v2 21/21] x86/insn: Make insn_complete() static Borislav Petkov
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
Now that it is not needed anymore, drop it.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/insn.h | 11 -----------
tools/arch/x86/include/asm/insn.h | 11 -----------
2 files changed, 22 deletions(-)
diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index 546436b3c215..ec260f5a6dd5 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -114,17 +114,6 @@ static inline void insn_get_attribute(struct insn *insn)
/* Instruction uses RIP-relative addressing */
extern int insn_rip_relative(struct insn *insn);
-/* Init insn for kernel text */
-static inline void kernel_insn_init(struct insn *insn,
- const void *kaddr, int buf_len)
-{
-#ifdef CONFIG_X86_64
- insn_init(insn, kaddr, buf_len, 1);
-#else /* CONFIG_X86_32 */
- insn_init(insn, kaddr, buf_len, 0);
-#endif
-}
-
static inline int insn_is_avx(struct insn *insn)
{
if (!insn->prefixes.got)
diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h
index 621ab64a6d27..a54b4423b640 100644
--- a/tools/arch/x86/include/asm/insn.h
+++ b/tools/arch/x86/include/asm/insn.h
@@ -114,17 +114,6 @@ static inline void insn_get_attribute(struct insn *insn)
/* Instruction uses RIP-relative addressing */
extern int insn_rip_relative(struct insn *insn);
-/* Init insn for kernel text */
-static inline void kernel_insn_init(struct insn *insn,
- const void *kaddr, int buf_len)
-{
-#ifdef CONFIG_X86_64
- insn_init(insn, kaddr, buf_len, 1);
-#else /* CONFIG_X86_32 */
- insn_init(insn, kaddr, buf_len, 0);
-#endif
-}
-
static inline int insn_is_avx(struct insn *insn)
{
if (!insn->prefixes.got)
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 21/21] x86/insn: Make insn_complete() static
2021-02-24 11:02 [PATCH v2 00/21] x86/insn: Add an insn_decode() API Borislav Petkov
` (19 preceding siblings ...)
2021-02-24 11:02 ` [PATCH v2 20/21] x86/insn: Remove kernel_insn_init() Borislav Petkov
@ 2021-02-24 11:02 ` Borislav Petkov
20 siblings, 0 replies; 25+ messages in thread
From: Borislav Petkov @ 2021-02-24 11:02 UTC (permalink / raw)
To: X86 ML; +Cc: Masami Hiramatsu, LKML
From: Borislav Petkov <bp@suse.de>
... and move it above the only place it is used.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/insn.h | 7 -------
arch/x86/lib/insn.c | 7 +++++++
tools/arch/x86/include/asm/insn.h | 7 -------
tools/arch/x86/lib/insn.c | 7 +++++++
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index ec260f5a6dd5..6df0d3da0d86 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -133,13 +133,6 @@ static inline int insn_has_emulate_prefix(struct insn *insn)
return !!insn->emulate_prefix_size;
}
-/* Ensure this instruction is decoded completely */
-static inline int insn_complete(struct insn *insn)
-{
- return insn->opcode.got && insn->modrm.got && insn->sib.got &&
- insn->displacement.got && insn->immediate.got;
-}
-
static inline insn_byte_t insn_vex_m_bits(struct insn *insn)
{
if (insn->vex_prefix.nbytes == 2) /* 2 bytes VEX */
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 07ee5b14ae38..aa6ee796a987 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -726,6 +726,13 @@ int insn_get_length(struct insn *insn)
return 0;
}
+/* Ensure this instruction is decoded completely */
+static inline int insn_complete(struct insn *insn)
+{
+ return insn->opcode.got && insn->modrm.got && insn->sib.got &&
+ insn->displacement.got && insn->immediate.got;
+}
+
/**
* insn_decode() - Decode an x86 instruction
* @insn: &struct insn to be initialized
diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h
index a54b4423b640..4f219e3ae817 100644
--- a/tools/arch/x86/include/asm/insn.h
+++ b/tools/arch/x86/include/asm/insn.h
@@ -133,13 +133,6 @@ static inline int insn_has_emulate_prefix(struct insn *insn)
return !!insn->emulate_prefix_size;
}
-/* Ensure this instruction is decoded completely */
-static inline int insn_complete(struct insn *insn)
-{
- return insn->opcode.got && insn->modrm.got && insn->sib.got &&
- insn->displacement.got && insn->immediate.got;
-}
-
static inline insn_byte_t insn_vex_m_bits(struct insn *insn)
{
if (insn->vex_prefix.nbytes == 2) /* 2 bytes VEX */
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index 6c9c1648eef3..a14e78091c75 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -726,6 +726,13 @@ int insn_get_length(struct insn *insn)
return 0;
}
+/* Ensure this instruction is decoded completely */
+static inline int insn_complete(struct insn *insn)
+{
+ return insn->opcode.got && insn->modrm.got && insn->sib.got &&
+ insn->displacement.got && insn->immediate.got;
+}
+
/**
* insn_decode() - Decode an x86 instruction
* @insn: &struct insn to be initialized
--
2.29.2
^ permalink raw reply related [flat|nested] 25+ messages in thread