[Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX

Masami Hiramatsu posted 2 patches 4 years, 7 months ago
Failed in applying to current master (apply log)
arch/x86/include/asm/insn.h             |    2 +
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                     |   43 +++++++++++++++++++++++++++++++
tools/arch/x86/include/asm/insn.h       |    2 +
tools/arch/x86/include/asm/xen/prefix.h |   10 +++++++
tools/arch/x86/lib/insn.c               |   43 +++++++++++++++++++++++++++++++
tools/objtool/sync-check.sh             |    3 +-
9 files changed, 121 insertions(+), 3 deletions(-)
create mode 100644 arch/x86/include/asm/xen/prefix.h
create mode 100644 tools/arch/x86/include/asm/xen/prefix.h
[Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Masami Hiramatsu 4 years, 7 months ago
Hi,

These patches allow x86 instruction decoder to decode
xen-cpuid which has XEN_EMULATE_PREFIX, 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.


Thank you,

---

Masami Hiramatsu (2):
      x86: xen: insn: Decode XEN_EMULATE_PREFIX correctly
      x86: kprobes: Prohibit probing on instruction which has Xen prefix


 arch/x86/include/asm/insn.h             |    2 +
 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                     |   43 +++++++++++++++++++++++++++++++
 tools/arch/x86/include/asm/insn.h       |    2 +
 tools/arch/x86/include/asm/xen/prefix.h |   10 +++++++
 tools/arch/x86/lib/insn.c               |   43 +++++++++++++++++++++++++++++++
 tools/objtool/sync-check.sh             |    3 +-
 9 files changed, 121 insertions(+), 3 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) <mhiramat@kernel.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Andrew Cooper 4 years, 7 months ago
On 04/09/2019 12:45, Masami Hiramatsu wrote:
> Hi,
>
> These patches allow x86 instruction decoder to decode
> xen-cpuid which has XEN_EMULATE_PREFIX, 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.

The paravirtualised xen-cpuid is were you'll see it most in a regular
kernel, but be aware that it is also used for testing purposes in other
circumstances, and there is an equivalent KVM prefix which is used for
KVM testing.

It might be better to generalise the decode support to "virtualisation
escape prefix" or something slightly more generic.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Masami Hiramatsu 4 years, 7 months ago
On Wed, 4 Sep 2019 12:54:55 +0100
Andrew Cooper <andrew.cooper3@citrix.com> wrote:

> On 04/09/2019 12:45, Masami Hiramatsu wrote:
> > Hi,
> >
> > These patches allow x86 instruction decoder to decode
> > xen-cpuid which has XEN_EMULATE_PREFIX, 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.
> 
> The paravirtualised xen-cpuid is were you'll see it most in a regular
> kernel, but be aware that it is also used for testing purposes in other
> circumstances, and there is an equivalent KVM prefix which is used for
> KVM testing.

Good catch! I didn't notice that. Is that really same sequance or KVM uses
another sequence of instructions for KVM prefix?

> 
> It might be better to generalise the decode support to "virtualisation
> escape prefix" or something slightly more generic.

Agreed, it is easy to expand it, we can switch the prefix template.
Could you tell me where I should look? I will add it.

Thank you,


> 
> ~Andrew


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Andrew Cooper 4 years, 7 months ago
On 05/09/2019 02:49, Masami Hiramatsu wrote:
> On Wed, 4 Sep 2019 12:54:55 +0100
> Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
>> On 04/09/2019 12:45, Masami Hiramatsu wrote:
>>> Hi,
>>>
>>> These patches allow x86 instruction decoder to decode
>>> xen-cpuid which has XEN_EMULATE_PREFIX, 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.
>> The paravirtualised xen-cpuid is were you'll see it most in a regular
>> kernel, but be aware that it is also used for testing purposes in other
>> circumstances, and there is an equivalent KVM prefix which is used for
>> KVM testing.
> Good catch! I didn't notice that. Is that really same sequance or KVM uses
> another sequence of instructions for KVM prefix?

I don't know if you've spotted, but the prefix is a ud2a instruction
followed by 'xen' in ascii.

The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2

>
>> It might be better to generalise the decode support to "virtualisation
>> escape prefix" or something slightly more generic.
> Agreed, it is easy to expand it, we can switch the prefix template.
> Could you tell me where I should look? I will add it.

These are the only two I'm aware of.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Peter Zijlstra 4 years, 7 months ago
On Thu, Sep 05, 2019 at 08:54:17AM +0100, Andrew Cooper wrote:

> I don't know if you've spotted, but the prefix is a ud2a instruction
> followed by 'xen' in ascii.
>
> The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2

While the Xen one disassebles to valid instructions, that KVM one does
not:

	.text
xen:
	ud2; .ascii "xen"
kvm:
	ud2; .ascii "kvm"

disassembles like:

0000000000000000 <xen>:
   0:   0f 0b                   ud2
   2:   78 65                   js     69 <kvm+0x64>
   4:   6e                      outsb  %ds:(%rsi),(%dx)
0000000000000005 <kvm>:
   5:   0f 0b                   ud2
   7:   6b                      .byte 0x6b
   8:   76 6d                   jbe    77 <kvm+0x72>

Which is a bit unfortunate I suppose. At least they don't appear to
consume further bytes.

I know it is water under the bridge at this point; but you could've used
UD1 with a displacement with some 'unlikely' values. That way it
would've decoded to a single instruction.

Something like:

	ud1    0x6e6578(%rax),%rax

which spells out "xen\0" in the displacement:

	48 0f b9 80 78 65 6e 00

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Andrew Cooper 4 years, 7 months ago
On 05/09/2019 09:26, Peter Zijlstra wrote:
> On Thu, Sep 05, 2019 at 08:54:17AM +0100, Andrew Cooper wrote:
>
>> I don't know if you've spotted, but the prefix is a ud2a instruction
>> followed by 'xen' in ascii.
>>
>> The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2
> While the Xen one disassebles to valid instructions, that KVM one does
> not:
>
> 	.text
> xen:
> 	ud2; .ascii "xen"
> kvm:
> 	ud2; .ascii "kvm"
>
> disassembles like:
>
> 0000000000000000 <xen>:
>    0:   0f 0b                   ud2
>    2:   78 65                   js     69 <kvm+0x64>
>    4:   6e                      outsb  %ds:(%rsi),(%dx)
> 0000000000000005 <kvm>:
>    5:   0f 0b                   ud2
>    7:   6b                      .byte 0x6b
>    8:   76 6d                   jbe    77 <kvm+0x72>
>
> Which is a bit unfortunate I suppose. At least they don't appear to
> consume further bytes.

It does when you give objdump one extra byte to look at.

0000000000000005 <kvm>:
   5:    0f 0b                    ud2   
   7:    6b 76 6d 00              imul   $0x0,0x6d(%rsi),%esi

I did try to point out that this property should have been checked
before settling on 'kvm' as the string.

As for the Xen prefix, it's introduction pre-dates me substantially, and
I don't know whether the disassembly was considered, or we just got lucky.

IMO, the string 'Xen' would would have been sightly nicer

0000000000000005 <Xen>:
   5:    0f 0b                    ud2   
   7:    58                       pop    %rax
   8:    65 6e                    outsb  %gs:(%rsi),(%dx)

but we're 13 years too late to amend this.

> I know it is water under the bridge at this point; but you could've used
> UD1 with a displacement with some 'unlikely' values. That way it
> would've decoded to a single instruction.
>
> Something like:
>
> 	ud1    0x6e6578(%rax),%rax
>
> which spells out "xen\0" in the displacement:
>
> 	48 0f b9 80 78 65 6e 00

:)

I seem to recall UD0 and UD1 being very late to the documentation party.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Peter Zijlstra 4 years, 7 months ago
On Thu, Sep 05, 2019 at 09:53:32AM +0100, Andrew Cooper wrote:
> On 05/09/2019 09:26, Peter Zijlstra wrote:
> > On Thu, Sep 05, 2019 at 08:54:17AM +0100, Andrew Cooper wrote:
> >
> >> I don't know if you've spotted, but the prefix is a ud2a instruction
> >> followed by 'xen' in ascii.
> >>
> >> The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2
> > While the Xen one disassebles to valid instructions, that KVM one does
> > not:
> >
> > 	.text
> > xen:
> > 	ud2; .ascii "xen"
> > kvm:
> > 	ud2; .ascii "kvm"
> >
> > disassembles like:
> >
> > 0000000000000000 <xen>:
> >    0:   0f 0b                   ud2
> >    2:   78 65                   js     69 <kvm+0x64>
> >    4:   6e                      outsb  %ds:(%rsi),(%dx)
> > 0000000000000005 <kvm>:
> >    5:   0f 0b                   ud2
> >    7:   6b                      .byte 0x6b
> >    8:   76 6d                   jbe    77 <kvm+0x72>
> >
> > Which is a bit unfortunate I suppose. At least they don't appear to
> > consume further bytes.
> 
> It does when you give objdump one extra byte to look at.
> 
> 0000000000000005 <kvm>:
>    5:    0f 0b                    ud2   
>    7:    6b 76 6d 00              imul   $0x0,0x6d(%rsi),%esi
> 
> I did try to point out that this property should have been checked
> before settling on 'kvm' as the string.

Bah you're right; when I write:

	ud2; .ascii "kvm"; cpuid

The output is gibberish :/

> but we're 13 years too late to amend this.

Yah, I figured :/

> > I know it is water under the bridge at this point; but you could've used
> > UD1 with a displacement with some 'unlikely' values. That way it
> > would've decoded to a single instruction.
> >
> > Something like:
> >
> > 	ud1    0x6e6578(%rax),%rax
> >
> > which spells out "xen\0" in the displacement:
> >
> > 	48 0f b9 80 78 65 6e 00
> 
> :)
> 
> I seem to recall UD0 and UD1 being very late to the documentation party.

They were; and the documentation for still UD0 differs between vendors :/

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Andrew Cooper 4 years, 7 months ago
On 05/09/2019 10:26, Peter Zijlstra wrote:
> On Thu, Sep 05, 2019 at 09:53:32AM +0100, Andrew Cooper wrote:
>> On 05/09/2019 09:26, Peter Zijlstra wrote:
>>> On Thu, Sep 05, 2019 at 08:54:17AM +0100, Andrew Cooper wrote:
>>>
>>>> I don't know if you've spotted, but the prefix is a ud2a instruction
>>>> followed by 'xen' in ascii.
>>>>
>>>> The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2
>>> While the Xen one disassebles to valid instructions, that KVM one does
>>> not:
>>>
>>> 	.text
>>> xen:
>>> 	ud2; .ascii "xen"
>>> kvm:
>>> 	ud2; .ascii "kvm"
>>>
>>> disassembles like:
>>>
>>> 0000000000000000 <xen>:
>>>    0:   0f 0b                   ud2
>>>    2:   78 65                   js     69 <kvm+0x64>
>>>    4:   6e                      outsb  %ds:(%rsi),(%dx)
>>> 0000000000000005 <kvm>:
>>>    5:   0f 0b                   ud2
>>>    7:   6b                      .byte 0x6b
>>>    8:   76 6d                   jbe    77 <kvm+0x72>
>>>
>>> Which is a bit unfortunate I suppose. At least they don't appear to
>>> consume further bytes.
>> It does when you give objdump one extra byte to look at.
>>
>> 0000000000000005 <kvm>:
>>    5:    0f 0b                    ud2   
>>    7:    6b 76 6d 00              imul   $0x0,0x6d(%rsi),%esi
>>
>> I did try to point out that this property should have been checked
>> before settling on 'kvm' as the string.
> Bah you're right; when I write:
>
> 	ud2; .ascii "kvm"; cpuid
>
> The output is gibberish :/

KVM could possibly amend this.  It is an off-by-default testing-only
interface.

Xen's is part of the ABI for ring-deprivielged guests, to force CPUID to
be the virtualised view on hardware without CPUID Faulting.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [OT] Re: [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Masami Hiramatsu 4 years, 7 months ago
On Thu, 5 Sep 2019 09:53:32 +0100
Andrew Cooper <andrew.cooper3@citrix.com> wrote:

> On 05/09/2019 09:26, Peter Zijlstra wrote:
> > On Thu, Sep 05, 2019 at 08:54:17AM +0100, Andrew Cooper wrote:
> >
> >> I don't know if you've spotted, but the prefix is a ud2a instruction
> >> followed by 'xen' in ascii.
> >>
> >> The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2
> > While the Xen one disassebles to valid instructions, that KVM one does
> > not:
> >
> > 	.text
> > xen:
> > 	ud2; .ascii "xen"
> > kvm:
> > 	ud2; .ascii "kvm"
> >
> > disassembles like:
> >
> > 0000000000000000 <xen>:
> >    0:   0f 0b                   ud2
> >    2:   78 65                   js     69 <kvm+0x64>
> >    4:   6e                      outsb  %ds:(%rsi),(%dx)
> > 0000000000000005 <kvm>:
> >    5:   0f 0b                   ud2
> >    7:   6b                      .byte 0x6b
> >    8:   76 6d                   jbe    77 <kvm+0x72>
> >
> > Which is a bit unfortunate I suppose. At least they don't appear to
> > consume further bytes.
> 
> It does when you give objdump one extra byte to look at.
> 
> 0000000000000005 <kvm>:
>    5:    0f 0b                    ud2   
>    7:    6b 76 6d 00              imul   $0x0,0x6d(%rsi),%esi
> 

Hmm, that consumes the first byte of the next instruction.
For example, 

  .text
xen:
  ud2; .ascii "xen"; cpuid
kvm:
  ud2; .ascii "kvm"; cpuid

0000000000000000 <xen>:
   0:	0f 0b                	ud2    
   2:	78 65                	js     69 <kvm+0x62>
   4:	6e                   	outsb  %ds:(%rsi),(%dx)
   5:	0f a2                	cpuid  

0000000000000007 <kvm>:
   7:	0f 0b                	ud2    
   9:	6b 76 6d 0f          	imul   $0xf,0x6d(%rsi),%esi
   d:	a2                   	.byte 0xa2

This will disturbe decoding bytestream. Anyway, with the next version
it will be fixed in x86 insn decoder.

Thanks,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Masami Hiramatsu 4 years, 7 months ago
On Thu, 5 Sep 2019 08:54:17 +0100
Andrew Cooper <andrew.cooper3@citrix.com> wrote:

> On 05/09/2019 02:49, Masami Hiramatsu wrote:
> > On Wed, 4 Sep 2019 12:54:55 +0100
> > Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >
> >> On 04/09/2019 12:45, Masami Hiramatsu wrote:
> >>> Hi,
> >>>
> >>> These patches allow x86 instruction decoder to decode
> >>> xen-cpuid which has XEN_EMULATE_PREFIX, 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.
> >> The paravirtualised xen-cpuid is were you'll see it most in a regular
> >> kernel, but be aware that it is also used for testing purposes in other
> >> circumstances, and there is an equivalent KVM prefix which is used for
> >> KVM testing.
> > Good catch! I didn't notice that. Is that really same sequance or KVM uses
> > another sequence of instructions for KVM prefix?
> 
> I don't know if you've spotted, but the prefix is a ud2a instruction
> followed by 'xen' in ascii.
> 
> The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2
> 

Ah, OK. I see it. But it seems that another ud0/ud1 can be used by
other (new) virtualization. So at this moment I will just add a sequence
as a pattern of prefix. Not use a fixed ud2 + sig.

Thank you,

> >
> >> It might be better to generalise the decode support to "virtualisation
> >> escape prefix" or something slightly more generic.
> > Agreed, it is easy to expand it, we can switch the prefix template.
> > Could you tell me where I should look? I will add it.
> 
> These are the only two I'm aware of.



> 
> ~Andrew


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Masami Hiramatsu 4 years, 7 months ago
On Thu, 5 Sep 2019 20:32:24 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Thu, 5 Sep 2019 08:54:17 +0100
> Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> > On 05/09/2019 02:49, Masami Hiramatsu wrote:
> > > On Wed, 4 Sep 2019 12:54:55 +0100
> > > Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> > >
> > >> On 04/09/2019 12:45, Masami Hiramatsu wrote:
> > >>> Hi,
> > >>>
> > >>> These patches allow x86 instruction decoder to decode
> > >>> xen-cpuid which has XEN_EMULATE_PREFIX, 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.
> > >> The paravirtualised xen-cpuid is were you'll see it most in a regular
> > >> kernel, but be aware that it is also used for testing purposes in other
> > >> circumstances, and there is an equivalent KVM prefix which is used for
> > >> KVM testing.
> > > Good catch! I didn't notice that. Is that really same sequance or KVM uses
> > > another sequence of instructions for KVM prefix?
> > 
> > I don't know if you've spotted, but the prefix is a ud2a instruction
> > followed by 'xen' in ascii.
> > 
> > The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2

Hmm, I think I might misunderstand what the "emulate prefix"... that is not
a prefix which replace actual prefix, but just works like an escape sequence.
Thus the next instruction can have any x86 prefix, correct?

If so, this patch doesn't work. I have to add a new field in struct insn
like "insn.emulate_prefix_size" so that we can keep a room for the prefixes
for real instruction.

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Andrew Cooper 4 years, 7 months ago
On 05/09/2019 14:09, Masami Hiramatsu wrote:
> On Thu, 5 Sep 2019 20:32:24 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
>> On Thu, 5 Sep 2019 08:54:17 +0100
>> Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>>> On 05/09/2019 02:49, Masami Hiramatsu wrote:
>>>> On Wed, 4 Sep 2019 12:54:55 +0100
>>>> Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>>>
>>>>> On 04/09/2019 12:45, Masami Hiramatsu wrote:
>>>>>> Hi,
>>>>>>
>>>>>> These patches allow x86 instruction decoder to decode
>>>>>> xen-cpuid which has XEN_EMULATE_PREFIX, 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.
>>>>> The paravirtualised xen-cpuid is were you'll see it most in a regular
>>>>> kernel, but be aware that it is also used for testing purposes in other
>>>>> circumstances, and there is an equivalent KVM prefix which is used for
>>>>> KVM testing.
>>>> Good catch! I didn't notice that. Is that really same sequance or KVM uses
>>>> another sequence of instructions for KVM prefix?
>>> I don't know if you've spotted, but the prefix is a ud2a instruction
>>> followed by 'xen' in ascii.
>>>
>>> The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2
> Hmm, I think I might misunderstand what the "emulate prefix"... that is not
> a prefix which replace actual prefix, but just works like an escape sequence.
> Thus the next instruction can have any x86 prefix, correct?

There is a bit of history here :)

Originally, 13 years ago, Xen invented the "Force Emulate Prefix", which
was the sequence:

ud2a; .ascii 'xen'; cpuid

which hit the #UD handler and was recognised as a request for
virtualised CPUID information.  This was for ring-deprivileged
virtualisation, and is needed because the CPUID instruction itself
doesn't trap to the hypervisor.

Following some security issues in our instruction emulator, I reused
this prefix with VT-x/SVM guests for testing purposes.  It behaves in a
similar manner - when enabled, it is recognised in #UD exception
intercept, and causes Xen to add 5 to the instruction pointer, then
emulate the instruction starting there.

Then various folk thought that having the same kind of ability to test
KVM's instruction emulator would be a good idea, so they borrowed the idea.

From a behaviour point of view, it is an opaque 5 bytes which means
"break into the hypervisor, then emulate the following instruction".

The name "prefix" is unfortunate.  It was named thusly because from the
programmers point of view, it was something you put before the CPUID
instruction which wanted to be emulated.  It is not related to x86
instruction concept of a prefix.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Masami Hiramatsu 4 years, 7 months ago
On Thu, 5 Sep 2019 14:31:56 +0100
Andrew Cooper <andrew.cooper3@citrix.com> wrote:

> >>> The KVM version was added in c/s 6c86eedc206dd1f9d37a2796faa8e6f2278215d2
> > Hmm, I think I might misunderstand what the "emulate prefix"... that is not
> > a prefix which replace actual prefix, but just works like an escape sequence.
> > Thus the next instruction can have any x86 prefix, correct?
> 
> There is a bit of history here :)
> 
> Originally, 13 years ago, Xen invented the "Force Emulate Prefix", which
> was the sequence:
> 
> ud2a; .ascii 'xen'; cpuid
> 
> which hit the #UD handler and was recognised as a request for
> virtualised CPUID information.  This was for ring-deprivileged
> virtualisation, and is needed because the CPUID instruction itself
> doesn't trap to the hypervisor.
> 
> Following some security issues in our instruction emulator, I reused
> this prefix with VT-x/SVM guests for testing purposes.  It behaves in a
> similar manner - when enabled, it is recognised in #UD exception
> intercept, and causes Xen to add 5 to the instruction pointer, then
> emulate the instruction starting there.
> 
> Then various folk thought that having the same kind of ability to test
> KVM's instruction emulator would be a good idea, so they borrowed the idea.
> 
> From a behaviour point of view, it is an opaque 5 bytes which means
> "break into the hypervisor, then emulate the following instruction".
> 
> The name "prefix" is unfortunate.  It was named thusly because from the
> programmers point of view, it was something you put before the CPUID
> instruction which wanted to be emulated.  It is not related to x86
> instruction concept of a prefix.

OK, then we should not use the insn->prefixes for those escape sequences.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH -tip 0/2] x86: Prohibit kprobes on XEN_EMULATE_PREFIX
Posted by Peter Zijlstra 4 years, 7 months ago
On Wed, Sep 04, 2019 at 08:45:47PM +0900, Masami Hiramatsu wrote:
> Hi,
> 
> These patches allow x86 instruction decoder to decode
> xen-cpuid which has XEN_EMULATE_PREFIX, 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.
> 
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (2):
>       x86: xen: insn: Decode XEN_EMULATE_PREFIX correctly
>       x86: kprobes: Prohibit probing on instruction which has Xen prefix
> 
> 
>  arch/x86/include/asm/insn.h             |    2 +
>  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                     |   43 +++++++++++++++++++++++++++++++
>  tools/arch/x86/include/asm/insn.h       |    2 +
>  tools/arch/x86/include/asm/xen/prefix.h |   10 +++++++
>  tools/arch/x86/lib/insn.c               |   43 +++++++++++++++++++++++++++++++
>  tools/objtool/sync-check.sh             |    3 +-
>  9 files changed, 121 insertions(+), 3 deletions(-)
>  create mode 100644 arch/x86/include/asm/xen/prefix.h
>  create mode 100644 tools/arch/x86/include/asm/xen/prefix.h

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel