Skip to contentSkip to conversation

x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes

PATCH Superseded Masami Hiramatsu 2 participants 8 messages Sep 6, 2019

Subsystems (4)
x86 perf events Xen hypervisor x86 objtool

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Masami Hiramatsu

author
#0 lore raw

v3 cover x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes

Hi,

Here is the 3rd version of patches to handle Xen/KVM emulate prefix by x86 instruction decoder.

These patches allow x86 instruction decoder to decode Xen and KVM emulate prefix correctly, and prohibit kprobes to probe on it.

Josh reported that the objtool can not decode such special prefixed instructions, and I found that we also have to prohibit kprobes to probe on such instruction.

This series can be applied on -tip master branch which has merged Josh's objtool/perf sharing common x86 insn decoder series.

In the 2nd version, I added KVM emulate prefix support and generalized the interface. (insn_has_xen_prefix -> insn_has_emulate_prefix) Also, I added insn.emulate_prefix_size for those prefixes because that prefix is NOT an x86 instruction prefix, and the next instruction of those emulate prefixes can have x86 instruction prefix. So we can not use insn.prefix for it.

In this 3rd version, I just fixed tools/perf/check-headers.sh so that it can ignore the difference of xen/prefix header path.

Thank you,

---

Masami Hiramatsu (2):
x86: xen: insn: Decode Xen and KVM emulate-prefix signature
x86: kprobes: Prohibit probing on instruction which has emulate prefix

 arch/x86/include/asm/insn.h             |    6 +++++
 arch/x86/include/asm/xen/interface.h    |    7 ++++--
 arch/x86/include/asm/xen/prefix.h       |   10 +++++++++
 arch/x86/kernel/kprobes/core.c          |    4 +++
 arch/x86/lib/insn.c                     |   36 +++++++++++++++++++++++++++++++
 tools/arch/x86/include/asm/insn.h       |    6 +++++
 tools/arch/x86/include/asm/xen/prefix.h |   10 +++++++++
 tools/arch/x86/lib/insn.c               |   36 +++++++++++++++++++++++++++++++
 tools/objtool/sync-check.sh             |    3 ++-
 tools/perf/check-headers.sh             |    2 +-
 10 files changed, 116 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/include/asm/xen/prefix.h
 create mode 100644 tools/arch/x86/include/asm/xen/prefix.h

--
Masami Hiramatsu (Linaro) <[email protected]>

author
#1 lore raw

PATCH v3 1/2 x86: xen: insn: Decode Xen and KVM emulate-prefix signature

Decode Xen and KVM's emulate-prefix signature by x86 insn decoder. It is called "prefix" but actually not x86 instruction prefix, so this adds insn.emulate_prefix_size field instead of reusing insn.prefixes.

If x86 decoder finds a special sequence of instructions of XEN_EMULATE_PREFIX and 'ud2a; .ascii "kvm"', it just counts the length, set insn.emulate_prefix_size and fold it with the next instruction. In other words, the signature and the next instruction is treated as a single instruction.

Reported-by Josh Poimboeuf <[email protected]>
Signed-off-by Masami Hiramatsu <[email protected]>
Acked-by Josh Poimboeuf <[email protected]>

---
Changes in v3:
- Fix perf's check script too.
Changes in v2:
- Generalize the emulate-prefix handling not only for Xen but KVM.
- Introduce insn.emulate_prefix_size instead of using insn.prefixes.

---
 arch/x86/include/asm/insn.h             |    6 +++++
 arch/x86/include/asm/xen/interface.h    |    7 ++++--
 arch/x86/include/asm/xen/prefix.h       |   10 +++++++++
 arch/x86/lib/insn.c                     |   36 +++++++++++++++++++++++++++++++
 tools/arch/x86/include/asm/insn.h       |    6 +++++
 tools/arch/x86/include/asm/xen/prefix.h |   10 +++++++++
 tools/arch/x86/lib/insn.c               |   36 +++++++++++++++++++++++++++++++
 tools/objtool/sync-check.sh             |    3 ++-
 tools/perf/check-headers.sh             |    2 +-
 9 files changed, 112 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/include/asm/xen/prefix.h
 create mode 100644 tools/arch/x86/include/asm/xen/prefix.h
arch/x86/include/asm/insn.h +6 -0
index 154f27be8bfc..5c1ae3eff9d4 100644
@@ -45,6 +45,7 @@ struct insn {
 		struct insn_field immediate2;	/* for 64bit imm or seg16 */
 	};
 
+	int	emulate_prefix_size;
 	insn_attr_t attr;
 	unsigned char opnd_bytes;
 	unsigned char addr_bytes;

@@ -128,6 +129,11 @@ static inline int insn_is_evex(struct insn *insn)
 	return (insn->vex_prefix.nbytes == 4);
 }
 
+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)
 {
arch/x86/include/asm/xen/interface.h +5 -2
index 62ca03ef5c65..fe33a9798708 100644
@@ -379,12 +379,15 @@ struct xen_pmu_arch {
  * Prefix forces emulation of some non-trapping instructions.
  * Currently only CPUID.
  */
+#include <asm/xen/prefix.h>
+
 #ifdef __ASSEMBLY__
-#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ;
+#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ;
 #define XEN_CPUID          XEN_EMULATE_PREFIX cpuid
 #else
-#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; "
+#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; "
 #define XEN_CPUID          XEN_EMULATE_PREFIX "cpuid"
+
 #endif
 
 #endif /* _ASM_X86_XEN_INTERFACE_H */
arch/x86/include/asm/xen/prefix.h new file +10 -0
new file mode 100644
index 000000000000..f901be0d7a95
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H
+#define _TOOLS_ASM_X86_XEN_PREFIX_H
+
+#include <linux/stringify.h>
+
+#define __XEN_EMULATE_PREFIX  0x0f,0x0b,0x78,0x65,0x6e
+#define __XEN_EMULATE_PREFIX_STR  __stringify(__XEN_EMULATE_PREFIX)
+
+#endif
arch/x86/lib/insn.c +36 -0
index 0b5862ba6a75..b7eb50187db9 100644
@@ -13,6 +13,9 @@
 #include <asm/inat.h>
 #include <asm/insn.h>
 
+/* For special Xen prefix */
+#include <asm/xen/prefix.h>
+
 /* Verify next sizeof(t) bytes can be on the same instruction */
 #define validate_next(t, insn, n)	\
 	((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)

@@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
 		insn->addr_bytes = 4;
 }
 
+static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX };
+/* See handle_ud()@arch/x86/kvm/x86.c */
+static const insn_byte_t kvm_prefix[] = "\xf\xbkvm";
+
+static int __insn_get_emulate_prefix(struct insn *insn,
+				     const insn_byte_t *prefix, size_t len)
+{
+	size_t i;
+
+	for (i = 0; i < len; i++) {
+		if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
+			goto err_out;
+	}
+
+	insn->emulate_prefix_size = len;
+	insn->next_byte += len;
+
+	return 1;
+
+err_out:
+	return 0;
+}
+
+static void insn_get_emulate_prefix(struct insn *insn)
+{
+	if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix)))
+		return;
+
+	__insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix));
+}
+
 /**
  * insn_get_prefixes - scan x86 instruction prefix bytes
  * @insn:	&struct insn containing instruction

@@ -76,6 +110,8 @@ void insn_get_prefixes(struct insn *insn)
 	if (prefixes->got)
 		return;
 
+	insn_get_emulate_prefix(insn);
+
 	nb = 0;
 	lb = 0;
 	b = peek_next(insn_byte_t, insn);
tools/arch/x86/include/asm/insn.h +6 -0
index 37a4c390750b..568854b14d0a 100644
@@ -45,6 +45,7 @@ struct insn {
 		struct insn_field immediate2;	/* for 64bit imm or seg16 */
 	};
 
+	int	emulate_prefix_size;
 	insn_attr_t attr;
 	unsigned char opnd_bytes;
 	unsigned char addr_bytes;

@@ -128,6 +129,11 @@ static inline int insn_is_evex(struct insn *insn)
 	return (insn->vex_prefix.nbytes == 4);
 }
 
+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)
 {
tools/arch/x86/include/asm/xen/prefix.h new file +10 -0
new file mode 100644
index 000000000000..f901be0d7a95
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H
+#define _TOOLS_ASM_X86_XEN_PREFIX_H
+
+#include <linux/stringify.h>
+
+#define __XEN_EMULATE_PREFIX  0x0f,0x0b,0x78,0x65,0x6e
+#define __XEN_EMULATE_PREFIX_STR  __stringify(__XEN_EMULATE_PREFIX)
+
+#endif
tools/arch/x86/lib/insn.c +36 -0
index 79e048f1d902..ce04e43e0749 100644
@@ -13,6 +13,9 @@
 #include "../include/asm/inat.h"
 #include "../include/asm/insn.h"
 
+/* For special Xen prefix */
+#include "../include/asm/xen/prefix.h"
+
 /* Verify next sizeof(t) bytes can be on the same instruction */
 #define validate_next(t, insn, n)	\
 	((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)

@@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
 		insn->addr_bytes = 4;
 }
 
+static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX };
+/* See handle_ud()@arch/x86/kvm/x86.c */
+static const insn_byte_t kvm_prefix[] = "\xf\xbkvm";
+
+static int __insn_get_emulate_prefix(struct insn *insn,
+				     const insn_byte_t *prefix, size_t len)
+{
+	size_t i;
+
+	for (i = 0; i < len; i++) {
+		if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
+			goto err_out;
+	}
+
+	insn->emulate_prefix_size = len;
+	insn->next_byte += len;
+
+	return 1;
+
+err_out:
+	return 0;
+}
+
+static void insn_get_emulate_prefix(struct insn *insn)
+{
+	if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix)))
+		return;
+
+	__insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix));
+}
+
 /**
  * insn_get_prefixes - scan x86 instruction prefix bytes
  * @insn:	&struct insn containing instruction

@@ -76,6 +110,8 @@ void insn_get_prefixes(struct insn *insn)
 	if (prefixes->got)
 		return;
 
+	insn_get_emulate_prefix(insn);
+
 	nb = 0;
 	lb = 0;
 	b = peek_next(insn_byte_t, insn);
tools/objtool/sync-check.sh +2 -1
index 0a832e265a50..34143ea3d477 100755
@@ -4,6 +4,7 @@
 FILES='
 arch/x86/include/asm/inat_types.h
 arch/x86/include/asm/orc_types.h
+arch/x86/include/asm/xen/prefix.h
 arch/x86/lib/x86-opcode-map.txt
 arch/x86/tools/gen-insn-attr-x86.awk
 '

@@ -46,6 +47,6 @@ done
 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[\">]"'
+check arch/x86/lib/insn.c             '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]" -I "^#include [\"<]\(../include/\)*asm/xen/prefix.h[\">]"'
 
 cd -
tools/perf/check-headers.sh +1 -1
index e2e0f06c97d0..edcffa55a826 100755
@@ -115,7 +115,7 @@ 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[\">]"'
+check arch/x86/lib/insn.c             '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]" -I "^#include [\"<]\(../include/\)*asm/xen/prefix.h[\">]"'
 
 # diff non-symmetric files
 check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
maintainer
#2 lore raw

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

Quoted 10 lines · Masami Hiramatsu
arch/x86/include/asm/xen/interface.h +2 -0
index 62ca03ef5c65..fe33a9798708 100644
@@ -379,12 +379,15 @@ struct xen_pmu_arch {
  * Prefix forces emulation of some non-trapping instructions.
  * Currently only CPUID.
  */
+#include <asm/xen/prefix.h>
+

arch/x86/include/asm/xen/interface.h struct xen_pmu_arch { · line 391

Quoted 10 lines
arch/x86/include/asm/xen/interface.h +3 -2
 #ifdef __ASSEMBLY__
-#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ;
+#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ;
 #define XEN_CPUID          XEN_EMULATE_PREFIX cpuid
 #else
-#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; "
+#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; "
 #define XEN_CPUID          XEN_EMULATE_PREFIX "cpuid"
+
 #endif

Possibly you can do something like:

#define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;)
#define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid)

Quoted 7 lines
arch/x86/include/asm/xen/interface.h +0 -0
 #endif /* _ASM_X86_XEN_INTERFACE_H */
arch/x86/include/asm/xen/prefix.h new file +0 -0
new file mode 100644
index 000000000000..f901be0d7a95

arch/x86/include/asm/xen/prefix.h line 10

Quoted 10 lines
arch/x86/include/asm/xen/prefix.h new file +10 -0
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H
+#define _TOOLS_ASM_X86_XEN_PREFIX_H
+
+#include <linux/stringify.h>
+
+#define __XEN_EMULATE_PREFIX  0x0f,0x0b,0x78,0x65,0x6e
+#define __XEN_EMULATE_PREFIX_STR  __stringify(__XEN_EMULATE_PREFIX)
+
+#endif

How about we make this asm/virt_prefix.h or something and include:

/*
* Virt escape sequences to trigger instruction emulation;
* ideally these would decode to 'whole' instruction and not destroy
* the instruction stream; sadly this is not true for the 'kvm' one :/ */

#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */
#define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */

arch/x86/lib/insn.c void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) · line 66

Quoted 12 lines
arch/x86/lib/insn.c +3 -0
index 0b5862ba6a75..b7eb50187db9 100644
@@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
 		insn->addr_bytes = 4;
 }
 
+static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX };
+/* See handle_ud()@arch/x86/kvm/x86.c */
+static const insn_byte_t kvm_prefix[] = "\xf\xbkvm";

Then you can make this consistent; maybe even something like:

static const insn_byte_t *virt_prefix[] = {
	{ __XEN_EMULATE_PREFIX },
	{ __KVM_EMULATE_PREFIX },
	{ NULL },
};

And then change emulate_prefix_size to emulate_prefix_index ?

author
#3 lore raw

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

Quoted 25 lines · On Fri, 6 Sep 2019 09:34:36 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 10 lines · Masami Hiramatsu
arch/x86/include/asm/xen/interface.h +2 -0
index 62ca03ef5c65..fe33a9798708 100644
@@ -379,12 +379,15 @@ struct xen_pmu_arch {
  * Prefix forces emulation of some non-trapping instructions.
  * Currently only CPUID.
  */
+#include <asm/xen/prefix.h>
+

arch/x86/include/asm/xen/interface.h struct xen_pmu_arch { · line 391

Quoted 10 lines
arch/x86/include/asm/xen/interface.h +3 -2
 #ifdef __ASSEMBLY__
-#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ;
+#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ;
 #define XEN_CPUID          XEN_EMULATE_PREFIX cpuid
 #else
-#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; "
+#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; "
 #define XEN_CPUID          XEN_EMULATE_PREFIX "cpuid"
+
 #endif

Possibly you can do something like:

Quoted 2 lines

#define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;)
#define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid)

Hmm, OK. But should I split this change from insn decoder change?

Quoted 21 lines
Quoted 8 lines
arch/x86/include/asm/xen/interface.h +0 -0
 #endif /* _ASM_X86_XEN_INTERFACE_H */
arch/x86/include/asm/xen/prefix.h new file +0 -0
new file mode 100644
index 000000000000..f901be0d7a95

arch/x86/include/asm/xen/prefix.h line 10

Quoted 10 lines
arch/x86/include/asm/xen/prefix.h new file +10 -0
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H
+#define _TOOLS_ASM_X86_XEN_PREFIX_H
+
+#include <linux/stringify.h>
+
+#define __XEN_EMULATE_PREFIX  0x0f,0x0b,0x78,0x65,0x6e
+#define __XEN_EMULATE_PREFIX_STR  __stringify(__XEN_EMULATE_PREFIX)
+
+#endif

How about we make this asm/virt_prefix.h or something and include:

Quoted 8 lines

/*
* Virt escape sequences to trigger instruction emulation;
* ideally these would decode to 'whole' instruction and not destroy
* the instruction stream; sadly this is not true for the 'kvm' one :/ */

#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */
#define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */

OK, and in that case I think we should do this in separated patch from this change.

Quoted 16 lines

arch/x86/lib/insn.c void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) · line 66

Quoted 13 lines
arch/x86/lib/insn.c +3 -0
index 0b5862ba6a75..b7eb50187db9 100644
@@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64)
 		insn->addr_bytes = 4;
 }
 
+static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX };
+/* See handle_ud()@arch/x86/kvm/x86.c */
+static const insn_byte_t kvm_prefix[] = "\xf\xbkvm";

Then you can make this consistent; maybe even something like:

Quoted 7 lines
static const insn_byte_t *virt_prefix[] = {
	{ __XEN_EMULATE_PREFIX },
	{ __KVM_EMULATE_PREFIX },
	{ NULL },
};

And then change emulate_prefix_size to emulate_prefix_index ?

Hmm, how we can get the length of those emulate prefixes? For struct insn, since the size information is important, other sub fields have "nbyte" field so that caller can find the actual bytes from original byte stream.

Maybe we can have struct emulate_prefix { .nbyte, .type }; and define enum etc.. but for me, it seems a bit over engineering. (since no one use that feature)

Thank you,

Signature · 1 line
Masami Hiramatsu <[email protected]>
author
#4 lore raw

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

Quoted 11 lines · On Fri, 6 Sep 2019 17:45:19 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 11 lines

How about we make this asm/virt_prefix.h or something and include:

/*
* Virt escape sequences to trigger instruction emulation;
* ideally these would decode to 'whole' instruction and not destroy
* the instruction stream; sadly this is not true for the 'kvm' one :/ */

#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */
#define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */

BTW, what should we call it, "emulate prefix" or "virt prefix"? "virt prefix" sounds too generic to me. So I rather like emulate_prefix.h.

Thank you,

Signature · 1 line
Masami Hiramatsu <[email protected]>
maintainer
#5 lore raw

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

Quoted 12 lines · Masami Hiramatsu
Quoted 9 lines · On Fri, 6 Sep 2019 17:45:19 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 9 lines

How about we make this asm/virt_prefix.h or something and include:

/*
* Virt escape sequences to trigger instruction emulation;
* ideally these would decode to 'whole' instruction and not destroy
* the instruction stream; sadly this is not true for the 'kvm' one :/ */

Quoted 5 lines
Quoted 2 lines
Quoted 2 lines

#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */
#define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */

BTW, what should we call it, "emulate prefix" or "virt prefix"? "virt prefix" sounds too generic to me. So I rather like emulate_prefix.h.

Works for me; and yeah, just see what is best for the other things. I only started down that road because the Xen and KVM 'prefixes' were initialized so inconsistently.

author
#6 lore raw

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

In reply to Peter Zijlstra
Quoted 25 lines · On Fri, 6 Sep 2019 09:34:36 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 10 lines · Masami Hiramatsu
arch/x86/include/asm/xen/interface.h +2 -0
index 62ca03ef5c65..fe33a9798708 100644
@@ -379,12 +379,15 @@ struct xen_pmu_arch {
  * Prefix forces emulation of some non-trapping instructions.
  * Currently only CPUID.
  */
+#include <asm/xen/prefix.h>
+

arch/x86/include/asm/xen/interface.h struct xen_pmu_arch { · line 391

Quoted 10 lines
arch/x86/include/asm/xen/interface.h +3 -2
 #ifdef __ASSEMBLY__
-#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ;
+#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ;
 #define XEN_CPUID          XEN_EMULATE_PREFIX cpuid
 #else
-#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; "
+#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; "
 #define XEN_CPUID          XEN_EMULATE_PREFIX "cpuid"
+
 #endif

Possibly you can do something like:

Quoted 2 lines

#define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;)
#define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid)

Oops, this doesn't work, since __ASM_FORM(x) uses #x directly

# define __ASM_FORM(x)  " " #x " "

which doesn't expand "x" if x is a macro. We have to use __stringify like

# include <linux/stringify.h>
# define __ASM_FORM(x)  " " __stringify(x) " "

So this needs aonther patch in the series :)

Thank you,

Signature · 1 line
Masami Hiramatsu <[email protected]>
author
#7 lore raw

PATCH v3 2/2 x86: kprobes: Prohibit probing on instruction which has emulate prefix

In reply to Masami Hiramatsu

Prohibit probing on instruction which has XEN_EMULATE_PREFIX or KVM's emulate prefix. Since that prefix is a marker for Xen and KVM, if we modify the marker by kprobe's int3, that doesn't work as expected.

Signed-off-by Masami Hiramatsu <[email protected]>
---
 arch/x86/kernel/kprobes/core.c |    4 ++++
 1 file changed, 4 insertions(+)
arch/x86/kernel/kprobes/core.c +4 -0
index 43fc13c831af..4f13af7cbcdb 100644
@@ -351,6 +351,10 @@ int __copy_instruction(u8 *dest, u8 *src, u8 *real, struct insn *insn)
 	kernel_insn_init(insn, dest, MAX_INSN_SIZE);
 	insn_get_length(insn);
 
+	/* We can not probe force emulate prefixed instruction */
+	if (insn_has_emulate_prefix(insn))
+		return 0;
+
 	/* Another subsystem puts a breakpoint, failed to recover */
 	if (insn->opcode.bytes[0] == BREAKPOINT_INSTRUCTION)
 		return 0;