[PATCH 0/4] Fix some KVM/HYP interactions with kprobes

[PATCH 0/4] Fix some KVM/HYP interactions with kprobes

[James Morse]

Hi guys,

When looking at Masami Hiramatsu's kprobe cleanup series, it occurred
to me the 'no KVM' isn't just about the EL1/EL2  split on non-VHE systems,
but whether KVM is prepared to handle stepping on a breakpoint. It's not.

This moves all the VHE-only KVM functions that run during world switch
into the __kprobes section, and always blacklists __hyp_text for kprobes
to cover the common guest entry/exit code.

I anticipate patch 1 going via the KVM tree to avoid conflicts.
Patch 2 will conflict with [0].


I'm not sure what the best thing to do with the hyp-stub is.
Patch 3 moves it to __hyp_text, and patch 4 covers the hibernate fallout
from doing that. We don't have any other mmu-off but not idmap'd text.
Probing the hyp-stub has to be done by address as the symbol names
alias those in the __entry_text, which is blacklisted. (although this
might depend on link order). I think anyone doing this is trying to
shoot themselves in the foot.


Know issues:
* Other regions we should blacklist are the kexec and hibernate 'copy
  everything' code, as the vectors may have been overwritten by the time
  we step on the probe. cpu-suspend needs investigating...


Thanks,

James

[0] lore.kernel.org/r/154753341900.31541.8135985235882849464.stgit@devbox

James Morse (4):
  KVM: arm64: Forbid kprobing of the VHE world-switch code
  arm64: kprobe: Always blacklist the KVM world-switch code
  arm64: hyp-stub: Forbid kprobing of the hyp-stub
  arm64: hibernate: Clean the __hyp_text to PoC after resume

 arch/arm64/kernel/hibernate.c      |  4 +++-
 arch/arm64/kernel/hyp-stub.S       |  2 ++
 arch/arm64/kernel/probes/kprobes.c |  6 +++---
 arch/arm64/kvm/hyp/switch.c        | 11 ++++++-----
 arch/arm64/kvm/hyp/sysreg-sr.c     |  9 +++++----
 5 files changed, 19 insertions(+), 13 deletions(-)

-- 
2.20.1

[PATCH 0/4] Fix some KVM/HYP interactions with kprobes

[James Morse]

Hi guys,

When looking at Masami Hiramatsu's kprobe cleanup series, it occurred
to me the 'no KVM' isn't just about the EL1/EL2  split on non-VHE systems,
but whether KVM is prepared to handle stepping on a breakpoint. It's not.

This moves all the VHE-only KVM functions that run during world switch
into the __kprobes section, and always blacklists __hyp_text for kprobes
to cover the common guest entry/exit code.

I anticipate patch 1 going via the KVM tree to avoid conflicts.
Patch 2 will conflict with [0].


I'm not sure what the best thing to do with the hyp-stub is.
Patch 3 moves it to __hyp_text, and patch 4 covers the hibernate fallout
from doing that. We don't have any other mmu-off but not idmap'd text.
Probing the hyp-stub has to be done by address as the symbol names
alias those in the __entry_text, which is blacklisted. (although this
might depend on link order). I think anyone doing this is trying to
shoot themselves in the foot.


Know issues:
* Other regions we should blacklist are the kexec and hibernate 'copy
  everything' code, as the vectors may have been overwritten by the time
  we step on the probe. cpu-suspend needs investigating...


Thanks,

James

[0] lore.kernel.org/r/154753341900.31541.8135985235882849464.stgit@devbox

James Morse (4):
  KVM: arm64: Forbid kprobing of the VHE world-switch code
  arm64: kprobe: Always blacklist the KVM world-switch code
  arm64: hyp-stub: Forbid kprobing of the hyp-stub
  arm64: hibernate: Clean the __hyp_text to PoC after resume

 arch/arm64/kernel/hibernate.c      |  4 +++-
 arch/arm64/kernel/hyp-stub.S       |  2 ++
 arch/arm64/kernel/probes/kprobes.c |  6 +++---
 arch/arm64/kvm/hyp/switch.c        | 11 ++++++-----
 arch/arm64/kvm/hyp/sysreg-sr.c     |  9 +++++----
 5 files changed, 19 insertions(+), 13 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/4] KVM: arm64: Forbid kprobing of the VHE world-switch code

[James Morse]

On systems with VHE the kernel and KVM's world-switch code run at the
same exception level. Code that is only used on a VHE system does not
need to be annotated as __hyp_text as it can reside anywhere in the
kernel text.

__hyp_text was also used to prevent kprobes from patching breakpoint
instructions into this region, as this code runs at a different
exception level. While this is no longer true with VHE, KVM still
switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
world-switch code will cause a hyp-panic.

echo "p:weasel sysreg_save_guest_state_vhe" > /sys/kernel/debug/tracing/kprobe_events
echo 1 > /sys/kernel/debug/tracing/events/kprobes/weasel/enable
lkvm run -k /boot/Image --console serial -p "console=ttyS0 earlycon=uart,mmio,0x3f8"

  # lkvm run -k /boot/Image -m 384 -c 3 --name guest-1474
  Info: Placing fdt at 0x8fe00000 - 0x8fffffff
  Info: virtio-mmio.devices=0x200@0x10000:36

  Info: virtio-mmio.devices=0x200@0x10200:37

  Info: virtio-mmio.devices=0x200@0x10400:38

[  614.178186] Kernel panic - not syncing: HYP panic:
[  614.178186] PS:404003c9 PC:ffff0000100d70e0 ESR:f2000004
[  614.178186] FAR:0000000080080000 HPFAR:0000000000800800 PAR:1d00007edbadc0de
[  614.178186] VCPU:00000000f8de32f1
[  614.178383] CPU: 2 PID: 1482 Comm: kvm-vcpu-0 Not tainted 5.0.0-rc2 #10799
[  614.178446] Call trace:
[  614.178480]  dump_backtrace+0x0/0x148
[  614.178567]  show_stack+0x24/0x30
[  614.178658]  dump_stack+0x90/0xb4
[  614.178710]  panic+0x13c/0x2d8
[  614.178793]  hyp_panic+0xac/0xd8
[  614.178880]  kvm_vcpu_run_vhe+0x9c/0xe0
[  614.178958]  kvm_arch_vcpu_ioctl_run+0x454/0x798
[  614.179038]  kvm_vcpu_ioctl+0x360/0x898
[  614.179087]  do_vfs_ioctl+0xc4/0x858
[  614.179174]  ksys_ioctl+0x84/0xb8
[  614.179261]  __arm64_sys_ioctl+0x28/0x38
[  614.179348]  el0_svc_common+0x94/0x108
[  614.179401]  el0_svc_handler+0x38/0x78
[  614.179487]  el0_svc+0x8/0xc
[  614.179558] SMP: stopping secondary CPUs
[  614.179661] Kernel Offset: disabled
[  614.179695] CPU features: 0x003,2a80aa38
[  614.179758] Memory Limit: none
[  614.179858] ---[ end Kernel panic - not syncing: HYP panic:
[  614.179858] PS:404003c9 PC:ffff0000100d70e0 ESR:f2000004
[  614.179858] FAR:0000000080080000 HPFAR:0000000000800800 PAR:1d00007edbadc0de
[  614.179858] VCPU:00000000f8de32f1 ]---

Annotate the VHE world-switch functions that aren't marked
__hyp_text as __kprobes.

Signed-off-by: James Morse <james.morse@arm.com>
Fixes: 3f5c90b890ac ("KVM: arm64: Introduce VHE-specific kvm_vcpu_run")
---
This has been an issue since the VHE/non-VHE world-switch paths were
split. Before then the code was common, and covered by __hyp_text, which
is always blacklisted by a subsequent patch.

---
 arch/arm64/kvm/hyp/switch.c    | 11 ++++++-----
 arch/arm64/kvm/hyp/sysreg-sr.c |  9 +++++----
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index b0b1478094b4..21c291586832 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -23,6 +23,7 @@
 #include <kvm/arm_psci.h>
 
 #include <asm/cpufeature.h>
+#include <asm/kprobes.h>
 #include <asm/kvm_asm.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_host.h>
@@ -91,7 +92,7 @@ static void __hyp_text __deactivate_traps_common(void)
 	write_sysreg(0, pmuserenr_el0);
 }
 
-static void activate_traps_vhe(struct kvm_vcpu *vcpu)
+static void __kprobes activate_traps_vhe(struct kvm_vcpu *vcpu)
 {
 	u64 val;
 
@@ -139,7 +140,7 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
 		__activate_traps_nvhe(vcpu);
 }
 
-static void deactivate_traps_vhe(void)
+static void __kprobes deactivate_traps_vhe(void)
 {
 	extern char vectors[];	/* kernel exception vectors */
 	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
@@ -460,7 +461,7 @@ static void __hyp_text __set_host_arch_workaround_state(struct kvm_vcpu *vcpu)
 }
 
 /* Switch to the guest for VHE systems running in EL2 */
-int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
+int __kprobes kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *host_ctxt;
 	struct kvm_cpu_context *guest_ctxt;
@@ -606,8 +607,8 @@ static void __hyp_text __hyp_call_panic_nvhe(u64 spsr, u64 elr, u64 par,
 		       read_sysreg(hpfar_el2), par, vcpu);
 }
 
-static void __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par,
-				 struct kvm_cpu_context *host_ctxt)
+static void __kprobes __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par,
+					   struct kvm_cpu_context *host_ctxt)
 {
 	struct kvm_vcpu *vcpu;
 	vcpu = host_ctxt->__hyp_running_vcpu;
diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 68d6f7c3b237..fbb6001ecdf7 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -18,6 +18,7 @@
 #include <linux/compiler.h>
 #include <linux/kvm_host.h>
 
+#include <asm/kprobes.h>
 #include <asm/kvm_asm.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_hyp.h>
@@ -94,12 +95,12 @@ void __hyp_text __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt)
 	__sysreg_save_el2_return_state(ctxt);
 }
 
-void sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt)
+void __kprobes sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt)
 {
 	__sysreg_save_common_state(ctxt);
 }
 
-void sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt)
+void __kprobes sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt)
 {
 	__sysreg_save_common_state(ctxt);
 	__sysreg_save_el2_return_state(ctxt);
@@ -184,12 +185,12 @@ void __hyp_text __sysreg_restore_state_nvhe(struct kvm_cpu_context *ctxt)
 	__sysreg_restore_el2_return_state(ctxt);
 }
 
-void sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt)
+void __kprobes sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt)
 {
 	__sysreg_restore_common_state(ctxt);
 }
 
-void sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt)
+void __kprobes sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt)
 {
 	__sysreg_restore_common_state(ctxt);
 	__sysreg_restore_el2_return_state(ctxt);
-- 
2.20.1

[PATCH 1/4] KVM: arm64: Forbid kprobing of the VHE world-switch code

[James Morse]

On systems with VHE the kernel and KVM's world-switch code run at the
same exception level. Code that is only used on a VHE system does not
need to be annotated as __hyp_text as it can reside anywhere in the
kernel text.

__hyp_text was also used to prevent kprobes from patching breakpoint
instructions into this region, as this code runs at a different
exception level. While this is no longer true with VHE, KVM still
switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
world-switch code will cause a hyp-panic.

echo "p:weasel sysreg_save_guest_state_vhe" > /sys/kernel/debug/tracing/kprobe_events
echo 1 > /sys/kernel/debug/tracing/events/kprobes/weasel/enable
lkvm run -k /boot/Image --console serial -p "console=ttyS0 earlycon=uart,mmio,0x3f8"

  # lkvm run -k /boot/Image -m 384 -c 3 --name guest-1474
  Info: Placing fdt at 0x8fe00000 - 0x8fffffff
  Info: virtio-mmio.devices=0x200@0x10000:36

  Info: virtio-mmio.devices=0x200@0x10200:37

  Info: virtio-mmio.devices=0x200@0x10400:38

[  614.178186] Kernel panic - not syncing: HYP panic:
[  614.178186] PS:404003c9 PC:ffff0000100d70e0 ESR:f2000004
[  614.178186] FAR:0000000080080000 HPFAR:0000000000800800 PAR:1d00007edbadc0de
[  614.178186] VCPU:00000000f8de32f1
[  614.178383] CPU: 2 PID: 1482 Comm: kvm-vcpu-0 Not tainted 5.0.0-rc2 #10799
[  614.178446] Call trace:
[  614.178480]  dump_backtrace+0x0/0x148
[  614.178567]  show_stack+0x24/0x30
[  614.178658]  dump_stack+0x90/0xb4
[  614.178710]  panic+0x13c/0x2d8
[  614.178793]  hyp_panic+0xac/0xd8
[  614.178880]  kvm_vcpu_run_vhe+0x9c/0xe0
[  614.178958]  kvm_arch_vcpu_ioctl_run+0x454/0x798
[  614.179038]  kvm_vcpu_ioctl+0x360/0x898
[  614.179087]  do_vfs_ioctl+0xc4/0x858
[  614.179174]  ksys_ioctl+0x84/0xb8
[  614.179261]  __arm64_sys_ioctl+0x28/0x38
[  614.179348]  el0_svc_common+0x94/0x108
[  614.179401]  el0_svc_handler+0x38/0x78
[  614.179487]  el0_svc+0x8/0xc
[  614.179558] SMP: stopping secondary CPUs
[  614.179661] Kernel Offset: disabled
[  614.179695] CPU features: 0x003,2a80aa38
[  614.179758] Memory Limit: none
[  614.179858] ---[ end Kernel panic - not syncing: HYP panic:
[  614.179858] PS:404003c9 PC:ffff0000100d70e0 ESR:f2000004
[  614.179858] FAR:0000000080080000 HPFAR:0000000000800800 PAR:1d00007edbadc0de
[  614.179858] VCPU:00000000f8de32f1 ]---

Annotate the VHE world-switch functions that aren't marked
__hyp_text as __kprobes.

Signed-off-by: James Morse <james.morse@arm.com>
Fixes: 3f5c90b890ac ("KVM: arm64: Introduce VHE-specific kvm_vcpu_run")
---
This has been an issue since the VHE/non-VHE world-switch paths were
split. Before then the code was common, and covered by __hyp_text, which
is always blacklisted by a subsequent patch.

---
 arch/arm64/kvm/hyp/switch.c    | 11 ++++++-----
 arch/arm64/kvm/hyp/sysreg-sr.c |  9 +++++----
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index b0b1478094b4..21c291586832 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -23,6 +23,7 @@
 #include <kvm/arm_psci.h>
 
 #include <asm/cpufeature.h>
+#include <asm/kprobes.h>
 #include <asm/kvm_asm.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_host.h>
@@ -91,7 +92,7 @@ static void __hyp_text __deactivate_traps_common(void)
 	write_sysreg(0, pmuserenr_el0);
 }
 
-static void activate_traps_vhe(struct kvm_vcpu *vcpu)
+static void __kprobes activate_traps_vhe(struct kvm_vcpu *vcpu)
 {
 	u64 val;
 
@@ -139,7 +140,7 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
 		__activate_traps_nvhe(vcpu);
 }
 
-static void deactivate_traps_vhe(void)
+static void __kprobes deactivate_traps_vhe(void)
 {
 	extern char vectors[];	/* kernel exception vectors */
 	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
@@ -460,7 +461,7 @@ static void __hyp_text __set_host_arch_workaround_state(struct kvm_vcpu *vcpu)
 }
 
 /* Switch to the guest for VHE systems running in EL2 */
-int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
+int __kprobes kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *host_ctxt;
 	struct kvm_cpu_context *guest_ctxt;
@@ -606,8 +607,8 @@ static void __hyp_text __hyp_call_panic_nvhe(u64 spsr, u64 elr, u64 par,
 		       read_sysreg(hpfar_el2), par, vcpu);
 }
 
-static void __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par,
-				 struct kvm_cpu_context *host_ctxt)
+static void __kprobes __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par,
+					   struct kvm_cpu_context *host_ctxt)
 {
 	struct kvm_vcpu *vcpu;
 	vcpu = host_ctxt->__hyp_running_vcpu;
diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 68d6f7c3b237..fbb6001ecdf7 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -18,6 +18,7 @@
 #include <linux/compiler.h>
 #include <linux/kvm_host.h>
 
+#include <asm/kprobes.h>
 #include <asm/kvm_asm.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_hyp.h>
@@ -94,12 +95,12 @@ void __hyp_text __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt)
 	__sysreg_save_el2_return_state(ctxt);
 }
 
-void sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt)
+void __kprobes sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt)
 {
 	__sysreg_save_common_state(ctxt);
 }
 
-void sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt)
+void __kprobes sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt)
 {
 	__sysreg_save_common_state(ctxt);
 	__sysreg_save_el2_return_state(ctxt);
@@ -184,12 +185,12 @@ void __hyp_text __sysreg_restore_state_nvhe(struct kvm_cpu_context *ctxt)
 	__sysreg_restore_el2_return_state(ctxt);
 }
 
-void sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt)
+void __kprobes sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt)
 {
 	__sysreg_restore_common_state(ctxt);
 }
 
-void sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt)
+void __kprobes sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt)
 {
 	__sysreg_restore_common_state(ctxt);
 	__sysreg_restore_el2_return_state(ctxt);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] arm64: kprobe: Always blacklist the KVM world-switch code

[James Morse]

On systems with VHE the kernel and KVM's world-switch code run at the
same exception level. Code that is only used on a VHE system does not
need to be annotated as __hyp_text as it can reside anywhere in the
 kernel text.

__hyp_text was also used to prevent kprobes from patching breakpoint
instructions into this region, as this code runs at a different
exception level. While this is no longer true with VHE, KVM still
switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
world-switch code will cause a hyp-panic.

Move the __hyp_text check in the kprobes blacklist so it applies on
VHE systems too, to cover the common code and guest enter/exit
assembly.

Signed-off-by: James Morse <james.morse@arm.com>
Fixes: 888b3c8720e0 ("arm64: Treat all entry code as non-kprobe-able")
---
 arch/arm64/kernel/probes/kprobes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 2a5b338b2542..f17afb99890c 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -478,13 +478,13 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
 	    addr < (unsigned long)__entry_text_end) ||
 	    (addr >= (unsigned long)__idmap_text_start &&
 	    addr < (unsigned long)__idmap_text_end) ||
+	    (addr >= (unsigned long)__hyp_text_start &&
+	    addr < (unsigned long)__hyp_text_end) ||
 	    !!search_exception_tables(addr))
 		return true;
 
 	if (!is_kernel_in_hyp_mode()) {
-		if ((addr >= (unsigned long)__hyp_text_start &&
-		    addr < (unsigned long)__hyp_text_end) ||
-		    (addr >= (unsigned long)__hyp_idmap_text_start &&
+		if ((addr >= (unsigned long)__hyp_idmap_text_start &&
 		    addr < (unsigned long)__hyp_idmap_text_end))
 			return true;
 	}
-- 
2.20.1

[PATCH 2/4] arm64: kprobe: Always blacklist the KVM world-switch code

[James Morse]

On systems with VHE the kernel and KVM's world-switch code run at the
same exception level. Code that is only used on a VHE system does not
need to be annotated as __hyp_text as it can reside anywhere in the
 kernel text.

__hyp_text was also used to prevent kprobes from patching breakpoint
instructions into this region, as this code runs at a different
exception level. While this is no longer true with VHE, KVM still
switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
world-switch code will cause a hyp-panic.

Move the __hyp_text check in the kprobes blacklist so it applies on
VHE systems too, to cover the common code and guest enter/exit
assembly.

Signed-off-by: James Morse <james.morse@arm.com>
Fixes: 888b3c8720e0 ("arm64: Treat all entry code as non-kprobe-able")
---
 arch/arm64/kernel/probes/kprobes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 2a5b338b2542..f17afb99890c 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -478,13 +478,13 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
 	    addr < (unsigned long)__entry_text_end) ||
 	    (addr >= (unsigned long)__idmap_text_start &&
 	    addr < (unsigned long)__idmap_text_end) ||
+	    (addr >= (unsigned long)__hyp_text_start &&
+	    addr < (unsigned long)__hyp_text_end) ||
 	    !!search_exception_tables(addr))
 		return true;
 
 	if (!is_kernel_in_hyp_mode()) {
-		if ((addr >= (unsigned long)__hyp_text_start &&
-		    addr < (unsigned long)__hyp_text_end) ||
-		    (addr >= (unsigned long)__hyp_idmap_text_start &&
+		if ((addr >= (unsigned long)__hyp_idmap_text_start &&
 		    addr < (unsigned long)__hyp_idmap_text_end))
 			return true;
 	}
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] arm64: hyp-stub: Forbid kprobing of the hyp-stub

[James Morse]

The hyp-stub is loaded by the kernel's early startup code at EL2
during boot, before KVM takes ownership later. The hyp-stub's
text is part of the regular kernel text, meaning it can be kprobed.

A breakpoint in the hyp-stub causes the CPU to spin in el2_sync_invalid.

Add it to the __hyp_text.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: stable@vger.kernel.org
---

This has been a problem since kprobes was merged, it should
probably have been covered in 888b3c8720e0.

I'm not sure __hyp_text is the right place. Its not idmaped,
and as it contains a set of vectors, adding it to the host/hyp
idmap sections could grow them beyond a page... but it does
run with the MMU off, so does need to be cleaned to PoC when
anything wacky, like hibernate happens. With this patch,
hibernate should clean the __hyp_text to PoC too.
---
 arch/arm64/kernel/hyp-stub.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index e1261fbaa374..17f325ba831e 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -28,6 +28,8 @@
 #include <asm/virt.h>
 
 	.text
+	.pushsection	.hyp.text, "ax"
+
 	.align 11
 
 ENTRY(__hyp_stub_vectors)
-- 
2.20.1

[PATCH 3/4] arm64: hyp-stub: Forbid kprobing of the hyp-stub

[James Morse]

The hyp-stub is loaded by the kernel's early startup code at EL2
during boot, before KVM takes ownership later. The hyp-stub's
text is part of the regular kernel text, meaning it can be kprobed.

A breakpoint in the hyp-stub causes the CPU to spin in el2_sync_invalid.

Add it to the __hyp_text.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: stable@vger.kernel.org
---

This has been a problem since kprobes was merged, it should
probably have been covered in 888b3c8720e0.

I'm not sure __hyp_text is the right place. Its not idmaped,
and as it contains a set of vectors, adding it to the host/hyp
idmap sections could grow them beyond a page... but it does
run with the MMU off, so does need to be cleaned to PoC when
anything wacky, like hibernate happens. With this patch,
hibernate should clean the __hyp_text to PoC too.
---
 arch/arm64/kernel/hyp-stub.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index e1261fbaa374..17f325ba831e 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -28,6 +28,8 @@
 #include <asm/virt.h>
 
 	.text
+	.pushsection	.hyp.text, "ax"
+
 	.align 11
 
 ENTRY(__hyp_stub_vectors)
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] arm64: hibernate: Clean the __hyp_text to PoC after resume

[James Morse]

During resume hibernate restores all physical memory. Any memory
that is accessed with the MMU disabled needs to be cleaned to the
PoC.

KVMs __hyp_text was previously ommited as it runs with the MMU
enabled, but now that the hyp-stub is located in this section,
we must clean __hyp_text too.

This ensures secondary CPUs that come online after hibernate
has finished resuming, and load KVM via the freshly written
hyp-stub see the correct instructions.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: stable@vger.kernel.org
---
 arch/arm64/kernel/hibernate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 29cdc99688f3..9859e1178e6b 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -299,8 +299,10 @@ int swsusp_arch_suspend(void)
 		dcache_clean_range(__idmap_text_start, __idmap_text_end);
 
 		/* Clean kvm setup code to PoC? */
-		if (el2_reset_needed())
+		if (el2_reset_needed()) {
 			dcache_clean_range(__hyp_idmap_text_start, __hyp_idmap_text_end);
+			dcache_clean_range(__hyp_text_start, __hyp_text_end);
+		}
 
 		/* make the crash dump kernel image protected again */
 		crash_post_resume();
-- 
2.20.1

[PATCH 4/4] arm64: hibernate: Clean the __hyp_text to PoC after resume

[James Morse]

During resume hibernate restores all physical memory. Any memory
that is accessed with the MMU disabled needs to be cleaned to the
PoC.

KVMs __hyp_text was previously ommited as it runs with the MMU
enabled, but now that the hyp-stub is located in this section,
we must clean __hyp_text too.

This ensures secondary CPUs that come online after hibernate
has finished resuming, and load KVM via the freshly written
hyp-stub see the correct instructions.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: stable@vger.kernel.org
---
 arch/arm64/kernel/hibernate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 29cdc99688f3..9859e1178e6b 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -299,8 +299,10 @@ int swsusp_arch_suspend(void)
 		dcache_clean_range(__idmap_text_start, __idmap_text_end);
 
 		/* Clean kvm setup code to PoC? */
-		if (el2_reset_needed())
+		if (el2_reset_needed()) {
 			dcache_clean_range(__hyp_idmap_text_start, __hyp_idmap_text_end);
+			dcache_clean_range(__hyp_text_start, __hyp_text_end);
+		}
 
 		/* make the crash dump kernel image protected again */
 		crash_post_resume();
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] arm64: kprobe: Always blacklist the KVM world-switch code

[Masami Hiramatsu]

On Mon, 21 Jan 2019 17:04:02 +0000
James Morse <james.morse@arm.com> wrote:

> On systems with VHE the kernel and KVM's world-switch code run at the
> same exception level. Code that is only used on a VHE system does not
> need to be annotated as __hyp_text as it can reside anywhere in the
>  kernel text.
> 
> __hyp_text was also used to prevent kprobes from patching breakpoint
> instructions into this region, as this code runs at a different
> exception level. While this is no longer true with VHE, KVM still
> switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
> world-switch code will cause a hyp-panic.
> 
> Move the __hyp_text check in the kprobes blacklist so it applies on
> VHE systems too, to cover the common code and guest enter/exit
> assembly.
> 

Looks good to me!

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> Signed-off-by: James Morse <james.morse@arm.com>
> Fixes: 888b3c8720e0 ("arm64: Treat all entry code as non-kprobe-able")
> ---
>  arch/arm64/kernel/probes/kprobes.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 2a5b338b2542..f17afb99890c 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -478,13 +478,13 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
>  	    addr < (unsigned long)__entry_text_end) ||
>  	    (addr >= (unsigned long)__idmap_text_start &&
>  	    addr < (unsigned long)__idmap_text_end) ||
> +	    (addr >= (unsigned long)__hyp_text_start &&
> +	    addr < (unsigned long)__hyp_text_end) ||
>  	    !!search_exception_tables(addr))
>  		return true;
>  
>  	if (!is_kernel_in_hyp_mode()) {
> -		if ((addr >= (unsigned long)__hyp_text_start &&
> -		    addr < (unsigned long)__hyp_text_end) ||
> -		    (addr >= (unsigned long)__hyp_idmap_text_start &&
> +		if ((addr >= (unsigned long)__hyp_idmap_text_start &&
>  		    addr < (unsigned long)__hyp_idmap_text_end))
>  			return true;
>  	}
> -- 
> 2.20.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 2/4] arm64: kprobe: Always blacklist the KVM world-switch code

[Masami Hiramatsu]

On Mon, 21 Jan 2019 17:04:02 +0000
James Morse <james.morse@arm.com> wrote:

> On systems with VHE the kernel and KVM's world-switch code run at the
> same exception level. Code that is only used on a VHE system does not
> need to be annotated as __hyp_text as it can reside anywhere in the
>  kernel text.
> 
> __hyp_text was also used to prevent kprobes from patching breakpoint
> instructions into this region, as this code runs at a different
> exception level. While this is no longer true with VHE, KVM still
> switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
> world-switch code will cause a hyp-panic.
> 
> Move the __hyp_text check in the kprobes blacklist so it applies on
> VHE systems too, to cover the common code and guest enter/exit
> assembly.
> 

Looks good to me!

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> Signed-off-by: James Morse <james.morse@arm.com>
> Fixes: 888b3c8720e0 ("arm64: Treat all entry code as non-kprobe-able")
> ---
>  arch/arm64/kernel/probes/kprobes.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 2a5b338b2542..f17afb99890c 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -478,13 +478,13 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
>  	    addr < (unsigned long)__entry_text_end) ||
>  	    (addr >= (unsigned long)__idmap_text_start &&
>  	    addr < (unsigned long)__idmap_text_end) ||
> +	    (addr >= (unsigned long)__hyp_text_start &&
> +	    addr < (unsigned long)__hyp_text_end) ||
>  	    !!search_exception_tables(addr))
>  		return true;
>  
>  	if (!is_kernel_in_hyp_mode()) {
> -		if ((addr >= (unsigned long)__hyp_text_start &&
> -		    addr < (unsigned long)__hyp_text_end) ||
> -		    (addr >= (unsigned long)__hyp_idmap_text_start &&
> +		if ((addr >= (unsigned long)__hyp_idmap_text_start &&
>  		    addr < (unsigned long)__hyp_idmap_text_end))
>  			return true;
>  	}
> -- 
> 2.20.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] KVM: arm64: Forbid kprobing of the VHE world-switch code

[Masami Hiramatsu]

Hi James,

On Mon, 21 Jan 2019 17:04:01 +0000
James Morse <james.morse@arm.com> wrote:

> On systems with VHE the kernel and KVM's world-switch code run at the
> same exception level. Code that is only used on a VHE system does not
> need to be annotated as __hyp_text as it can reside anywhere in the
> kernel text.
> 
> __hyp_text was also used to prevent kprobes from patching breakpoint
> instructions into this region, as this code runs at a different
> exception level. While this is no longer true with VHE, KVM still
> switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
> world-switch code will cause a hyp-panic.
> 
> echo "p:weasel sysreg_save_guest_state_vhe" > /sys/kernel/debug/tracing/kprobe_events
> echo 1 > /sys/kernel/debug/tracing/events/kprobes/weasel/enable
> lkvm run -k /boot/Image --console serial -p "console=ttyS0 earlycon=uart,mmio,0x3f8"
> 
>   # lkvm run -k /boot/Image -m 384 -c 3 --name guest-1474
>   Info: Placing fdt at 0x8fe00000 - 0x8fffffff
>   Info: virtio-mmio.devices=0x200@0x10000:36
> 
>   Info: virtio-mmio.devices=0x200@0x10200:37
> 
>   Info: virtio-mmio.devices=0x200@0x10400:38
> 
> [  614.178186] Kernel panic - not syncing: HYP panic:
> [  614.178186] PS:404003c9 PC:ffff0000100d70e0 ESR:f2000004
> [  614.178186] FAR:0000000080080000 HPFAR:0000000000800800 PAR:1d00007edbadc0de
> [  614.178186] VCPU:00000000f8de32f1
> [  614.178383] CPU: 2 PID: 1482 Comm: kvm-vcpu-0 Not tainted 5.0.0-rc2 #10799
> [  614.178446] Call trace:
> [  614.178480]  dump_backtrace+0x0/0x148
> [  614.178567]  show_stack+0x24/0x30
> [  614.178658]  dump_stack+0x90/0xb4
> [  614.178710]  panic+0x13c/0x2d8
> [  614.178793]  hyp_panic+0xac/0xd8
> [  614.178880]  kvm_vcpu_run_vhe+0x9c/0xe0
> [  614.178958]  kvm_arch_vcpu_ioctl_run+0x454/0x798
> [  614.179038]  kvm_vcpu_ioctl+0x360/0x898
> [  614.179087]  do_vfs_ioctl+0xc4/0x858
> [  614.179174]  ksys_ioctl+0x84/0xb8
> [  614.179261]  __arm64_sys_ioctl+0x28/0x38
> [  614.179348]  el0_svc_common+0x94/0x108
> [  614.179401]  el0_svc_handler+0x38/0x78
> [  614.179487]  el0_svc+0x8/0xc
> [  614.179558] SMP: stopping secondary CPUs
> [  614.179661] Kernel Offset: disabled
> [  614.179695] CPU features: 0x003,2a80aa38
> [  614.179758] Memory Limit: none
> [  614.179858] ---[ end Kernel panic - not syncing: HYP panic:
> [  614.179858] PS:404003c9 PC:ffff0000100d70e0 ESR:f2000004
> [  614.179858] FAR:0000000080080000 HPFAR:0000000000800800 PAR:1d00007edbadc0de
> [  614.179858] VCPU:00000000f8de32f1 ]---
> 
> Annotate the VHE world-switch functions that aren't marked
> __hyp_text as __kprobes.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Fixes: 3f5c90b890ac ("KVM: arm64: Introduce VHE-specific kvm_vcpu_run")
> ---
> This has been an issue since the VHE/non-VHE world-switch paths were
> split. Before then the code was common, and covered by __hyp_text, which
> is always blacklisted by a subsequent patch.

Thank you very much for fixing it!

BTW, would you mind if I ask you using NOKPROBE_SYMBOL() macro instead of
__kprobes attribute? __kprobes moves the function into __kprobe_text
forcibly, OTOH, NOKPROBE_SYMBOL() has no such side-effect.

Thank you,

> 
> ---
>  arch/arm64/kvm/hyp/switch.c    | 11 ++++++-----
>  arch/arm64/kvm/hyp/sysreg-sr.c |  9 +++++----
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index b0b1478094b4..21c291586832 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -23,6 +23,7 @@
>  #include <kvm/arm_psci.h>
>  
>  #include <asm/cpufeature.h>
> +#include <asm/kprobes.h>
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_host.h>
> @@ -91,7 +92,7 @@ static void __hyp_text __deactivate_traps_common(void)
>  	write_sysreg(0, pmuserenr_el0);
>  }
>  
> -static void activate_traps_vhe(struct kvm_vcpu *vcpu)
> +static void __kprobes activate_traps_vhe(struct kvm_vcpu *vcpu)
>  {
>  	u64 val;
>  
> @@ -139,7 +140,7 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
>  		__activate_traps_nvhe(vcpu);
>  }
>  
> -static void deactivate_traps_vhe(void)
> +static void __kprobes deactivate_traps_vhe(void)
>  {
>  	extern char vectors[];	/* kernel exception vectors */
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> @@ -460,7 +461,7 @@ static void __hyp_text __set_host_arch_workaround_state(struct kvm_vcpu *vcpu)
>  }
>  
>  /* Switch to the guest for VHE systems running in EL2 */
> -int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> +int __kprobes kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpu_context *host_ctxt;
>  	struct kvm_cpu_context *guest_ctxt;
> @@ -606,8 +607,8 @@ static void __hyp_text __hyp_call_panic_nvhe(u64 spsr, u64 elr, u64 par,
>  		       read_sysreg(hpfar_el2), par, vcpu);
>  }
>  
> -static void __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par,
> -				 struct kvm_cpu_context *host_ctxt)
> +static void __kprobes __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par,
> +					   struct kvm_cpu_context *host_ctxt)
>  {
>  	struct kvm_vcpu *vcpu;
>  	vcpu = host_ctxt->__hyp_running_vcpu;
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 68d6f7c3b237..fbb6001ecdf7 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -18,6 +18,7 @@
>  #include <linux/compiler.h>
>  #include <linux/kvm_host.h>
>  
> +#include <asm/kprobes.h>
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_hyp.h>
> @@ -94,12 +95,12 @@ void __hyp_text __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt)
>  	__sysreg_save_el2_return_state(ctxt);
>  }
>  
> -void sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt)
> +void __kprobes sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt)
>  {
>  	__sysreg_save_common_state(ctxt);
>  }
>  
> -void sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt)
> +void __kprobes sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt)
>  {
>  	__sysreg_save_common_state(ctxt);
>  	__sysreg_save_el2_return_state(ctxt);
> @@ -184,12 +185,12 @@ void __hyp_text __sysreg_restore_state_nvhe(struct kvm_cpu_context *ctxt)
>  	__sysreg_restore_el2_return_state(ctxt);
>  }
>  
> -void sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt)
> +void __kprobes sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt)
>  {
>  	__sysreg_restore_common_state(ctxt);
>  }
>  
> -void sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt)
> +void __kprobes sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt)
>  {
>  	__sysreg_restore_common_state(ctxt);
>  	__sysreg_restore_el2_return_state(ctxt);
> -- 
> 2.20.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 1/4] KVM: arm64: Forbid kprobing of the VHE world-switch code

[Masami Hiramatsu]

Hi James,

On Mon, 21 Jan 2019 17:04:01 +0000
James Morse <james.morse@arm.com> wrote:

> On systems with VHE the kernel and KVM's world-switch code run at the
> same exception level. Code that is only used on a VHE system does not
> need to be annotated as __hyp_text as it can reside anywhere in the
> kernel text.
> 
> __hyp_text was also used to prevent kprobes from patching breakpoint
> instructions into this region, as this code runs at a different
> exception level. While this is no longer true with VHE, KVM still
> switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
> world-switch code will cause a hyp-panic.
> 
> echo "p:weasel sysreg_save_guest_state_vhe" > /sys/kernel/debug/tracing/kprobe_events
> echo 1 > /sys/kernel/debug/tracing/events/kprobes/weasel/enable
> lkvm run -k /boot/Image --console serial -p "console=ttyS0 earlycon=uart,mmio,0x3f8"
> 
>   # lkvm run -k /boot/Image -m 384 -c 3 --name guest-1474
>   Info: Placing fdt at 0x8fe00000 - 0x8fffffff
>   Info: virtio-mmio.devices=0x200@0x10000:36
> 
>   Info: virtio-mmio.devices=0x200@0x10200:37
> 
>   Info: virtio-mmio.devices=0x200@0x10400:38
> 
> [  614.178186] Kernel panic - not syncing: HYP panic:
> [  614.178186] PS:404003c9 PC:ffff0000100d70e0 ESR:f2000004
> [  614.178186] FAR:0000000080080000 HPFAR:0000000000800800 PAR:1d00007edbadc0de
> [  614.178186] VCPU:00000000f8de32f1
> [  614.178383] CPU: 2 PID: 1482 Comm: kvm-vcpu-0 Not tainted 5.0.0-rc2 #10799
> [  614.178446] Call trace:
> [  614.178480]  dump_backtrace+0x0/0x148
> [  614.178567]  show_stack+0x24/0x30
> [  614.178658]  dump_stack+0x90/0xb4
> [  614.178710]  panic+0x13c/0x2d8
> [  614.178793]  hyp_panic+0xac/0xd8
> [  614.178880]  kvm_vcpu_run_vhe+0x9c/0xe0
> [  614.178958]  kvm_arch_vcpu_ioctl_run+0x454/0x798
> [  614.179038]  kvm_vcpu_ioctl+0x360/0x898
> [  614.179087]  do_vfs_ioctl+0xc4/0x858
> [  614.179174]  ksys_ioctl+0x84/0xb8
> [  614.179261]  __arm64_sys_ioctl+0x28/0x38
> [  614.179348]  el0_svc_common+0x94/0x108
> [  614.179401]  el0_svc_handler+0x38/0x78
> [  614.179487]  el0_svc+0x8/0xc
> [  614.179558] SMP: stopping secondary CPUs
> [  614.179661] Kernel Offset: disabled
> [  614.179695] CPU features: 0x003,2a80aa38
> [  614.179758] Memory Limit: none
> [  614.179858] ---[ end Kernel panic - not syncing: HYP panic:
> [  614.179858] PS:404003c9 PC:ffff0000100d70e0 ESR:f2000004
> [  614.179858] FAR:0000000080080000 HPFAR:0000000000800800 PAR:1d00007edbadc0de
> [  614.179858] VCPU:00000000f8de32f1 ]---
> 
> Annotate the VHE world-switch functions that aren't marked
> __hyp_text as __kprobes.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Fixes: 3f5c90b890ac ("KVM: arm64: Introduce VHE-specific kvm_vcpu_run")
> ---
> This has been an issue since the VHE/non-VHE world-switch paths were
> split. Before then the code was common, and covered by __hyp_text, which
> is always blacklisted by a subsequent patch.

Thank you very much for fixing it!

BTW, would you mind if I ask you using NOKPROBE_SYMBOL() macro instead of
__kprobes attribute? __kprobes moves the function into __kprobe_text
forcibly, OTOH, NOKPROBE_SYMBOL() has no such side-effect.

Thank you,

> 
> ---
>  arch/arm64/kvm/hyp/switch.c    | 11 ++++++-----
>  arch/arm64/kvm/hyp/sysreg-sr.c |  9 +++++----
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index b0b1478094b4..21c291586832 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -23,6 +23,7 @@
>  #include <kvm/arm_psci.h>
>  
>  #include <asm/cpufeature.h>
> +#include <asm/kprobes.h>
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_host.h>
> @@ -91,7 +92,7 @@ static void __hyp_text __deactivate_traps_common(void)
>  	write_sysreg(0, pmuserenr_el0);
>  }
>  
> -static void activate_traps_vhe(struct kvm_vcpu *vcpu)
> +static void __kprobes activate_traps_vhe(struct kvm_vcpu *vcpu)
>  {
>  	u64 val;
>  
> @@ -139,7 +140,7 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
>  		__activate_traps_nvhe(vcpu);
>  }
>  
> -static void deactivate_traps_vhe(void)
> +static void __kprobes deactivate_traps_vhe(void)
>  {
>  	extern char vectors[];	/* kernel exception vectors */
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> @@ -460,7 +461,7 @@ static void __hyp_text __set_host_arch_workaround_state(struct kvm_vcpu *vcpu)
>  }
>  
>  /* Switch to the guest for VHE systems running in EL2 */
> -int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> +int __kprobes kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpu_context *host_ctxt;
>  	struct kvm_cpu_context *guest_ctxt;
> @@ -606,8 +607,8 @@ static void __hyp_text __hyp_call_panic_nvhe(u64 spsr, u64 elr, u64 par,
>  		       read_sysreg(hpfar_el2), par, vcpu);
>  }
>  
> -static void __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par,
> -				 struct kvm_cpu_context *host_ctxt)
> +static void __kprobes __hyp_call_panic_vhe(u64 spsr, u64 elr, u64 par,
> +					   struct kvm_cpu_context *host_ctxt)
>  {
>  	struct kvm_vcpu *vcpu;
>  	vcpu = host_ctxt->__hyp_running_vcpu;
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 68d6f7c3b237..fbb6001ecdf7 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -18,6 +18,7 @@
>  #include <linux/compiler.h>
>  #include <linux/kvm_host.h>
>  
> +#include <asm/kprobes.h>
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_hyp.h>
> @@ -94,12 +95,12 @@ void __hyp_text __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt)
>  	__sysreg_save_el2_return_state(ctxt);
>  }
>  
> -void sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt)
> +void __kprobes sysreg_save_host_state_vhe(struct kvm_cpu_context *ctxt)
>  {
>  	__sysreg_save_common_state(ctxt);
>  }
>  
> -void sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt)
> +void __kprobes sysreg_save_guest_state_vhe(struct kvm_cpu_context *ctxt)
>  {
>  	__sysreg_save_common_state(ctxt);
>  	__sysreg_save_el2_return_state(ctxt);
> @@ -184,12 +185,12 @@ void __hyp_text __sysreg_restore_state_nvhe(struct kvm_cpu_context *ctxt)
>  	__sysreg_restore_el2_return_state(ctxt);
>  }
>  
> -void sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt)
> +void __kprobes sysreg_restore_host_state_vhe(struct kvm_cpu_context *ctxt)
>  {
>  	__sysreg_restore_common_state(ctxt);
>  }
>  
> -void sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt)
> +void __kprobes sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt)
>  {
>  	__sysreg_restore_common_state(ctxt);
>  	__sysreg_restore_el2_return_state(ctxt);
> -- 
> 2.20.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] KVM: arm64: Forbid kprobing of the VHE world-switch code

[James Morse]

Hello,

On 22/01/2019 03:11, Masami Hiramatsu wrote:
> On Mon, 21 Jan 2019 17:04:01 +0000
> James Morse <james.morse@arm.com> wrote:
>> On systems with VHE the kernel and KVM's world-switch code run at the
>> same exception level. Code that is only used on a VHE system does not
>> need to be annotated as __hyp_text as it can reside anywhere in the
>> kernel text.
>>
>> __hyp_text was also used to prevent kprobes from patching breakpoint
>> instructions into this region, as this code runs at a different
>> exception level. While this is no longer true with VHE, KVM still
>> switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
>> world-switch code will cause a hyp-panic.

>> Annotate the VHE world-switch functions that aren't marked
>> __hyp_text as __kprobes.

>> ---
>> This has been an issue since the VHE/non-VHE world-switch paths were
>> split. Before then the code was common, and covered by __hyp_text, which
>> is always blacklisted by a subsequent patch.
> 
> Thank you very much for fixing it!
> 
> BTW, would you mind if I ask you using NOKPROBE_SYMBOL() macro instead of
> __kprobes attribute? __kprobes moves the function into __kprobe_text
> forcibly, OTOH, NOKPROBE_SYMBOL() has no such side-effect.

Aha, yes. __kprobes moves the function to a special section, whereas the macro
spits out the address of the function into the blacklist section, which is
processed via init_kprobes().
I used __kprobes as its in keeping with __hyp_text, but this is clearly better
as it doesn't restrict the layout of the code. (and it solves the
hibernate/kexec problems as those would otherwise need to be in two sections!)

For my own education, when should __kprobes be used? Is it legacy?


Thanks!

James

Re: [PATCH 1/4] KVM: arm64: Forbid kprobing of the VHE world-switch code

[James Morse]

Hello,

On 22/01/2019 03:11, Masami Hiramatsu wrote:
> On Mon, 21 Jan 2019 17:04:01 +0000
> James Morse <james.morse@arm.com> wrote:
>> On systems with VHE the kernel and KVM's world-switch code run at the
>> same exception level. Code that is only used on a VHE system does not
>> need to be annotated as __hyp_text as it can reside anywhere in the
>> kernel text.
>>
>> __hyp_text was also used to prevent kprobes from patching breakpoint
>> instructions into this region, as this code runs at a different
>> exception level. While this is no longer true with VHE, KVM still
>> switches VBAR_EL1, meaning a kprobe's breakpoint executed in the
>> world-switch code will cause a hyp-panic.

>> Annotate the VHE world-switch functions that aren't marked
>> __hyp_text as __kprobes.

>> ---
>> This has been an issue since the VHE/non-VHE world-switch paths were
>> split. Before then the code was common, and covered by __hyp_text, which
>> is always blacklisted by a subsequent patch.
> 
> Thank you very much for fixing it!
> 
> BTW, would you mind if I ask you using NOKPROBE_SYMBOL() macro instead of
> __kprobes attribute? __kprobes moves the function into __kprobe_text
> forcibly, OTOH, NOKPROBE_SYMBOL() has no such side-effect.

Aha, yes. __kprobes moves the function to a special section, whereas the macro
spits out the address of the function into the blacklist section, which is
processed via init_kprobes().
I used __kprobes as its in keeping with __hyp_text, but this is clearly better
as it doesn't restrict the layout of the code. (and it solves the
hibernate/kexec problems as those would otherwise need to be in two sections!)

For my own education, when should __kprobes be used? Is it legacy?


Thanks!

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel