* [PATCH v3 0/9] UBSan Enablement for hyp/nVHE code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu
Respin of George's patch series enabling UBSAN for hyp/nVHE code.
Updates in v3:
* rebased onto v5.11-rc3
* added base commit info
Updates in v2:
* CONFIG_KVM_ARM_DEBUG_BUFFER removed; __kvm_check_ubsan_buffer is
called directly instead of via __kvm_arm_check_debug_buffer.
* Bugfixing commits removed as these are already upstream.
* Some code brought up to date, i.e. moved from entry.S to host.S.
* Merged "Add support for creating and checking a buffer" and
"Add a buffer that can pass UBSan data from hyp/nVHE" into
one commit as these changes don't work without each other.
George Popescu (9):
KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to
kernel
KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
KVM: arm64: Enable UBsan check for unreachable code inside hyp/nVHE
code
KVM: arm64: Enable shift out of bounds undefined behaviour check for
hyp/nVHE
KVM: arm64: __ubsan_handle_load_invalid_value EL2 implementation.
KVM: arm64: Detect type mismatch undefined behaviour from hyp/nVHE
code
KVM: arm64: Detect arithmetic overflow is inside hyp/nVHE.
KVM: arm64: Add UBSan tests for PKVM.
arch/arm64/include/asm/assembler.h | 10 ++
arch/arm64/include/asm/kvm_debug_buffer.h | 34 ++++
arch/arm64/include/asm/kvm_host.h | 8 +-
arch/arm64/include/asm/kvm_ubsan.h | 50 ++++++
arch/arm64/kvm/Makefile | 2 +
arch/arm64/kvm/arm.c | 9 ++
arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 +++++++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 3 +-
arch/arm64/kvm/hyp/nvhe/host.S | 4 +
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
arch/arm64/kvm/hyp/nvhe/ubsan.c | 164 ++++++++++++++++++++
arch/arm64/kvm/kvm_ubsan_buffer.c | 81 ++++++++++
12 files changed, 478 insertions(+), 2 deletions(-)
create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
base-commit: 5ee88057889bbca5f5bb96031b62b3756b33e164
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 0/9] UBSan Enablement for hyp/nVHE code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, Marc Zyngier, open list, linux-arm-kernel, George Popescu
Respin of George's patch series enabling UBSAN for hyp/nVHE code.
Updates in v3:
* rebased onto v5.11-rc3
* added base commit info
Updates in v2:
* CONFIG_KVM_ARM_DEBUG_BUFFER removed; __kvm_check_ubsan_buffer is
called directly instead of via __kvm_arm_check_debug_buffer.
* Bugfixing commits removed as these are already upstream.
* Some code brought up to date, i.e. moved from entry.S to host.S.
* Merged "Add support for creating and checking a buffer" and
"Add a buffer that can pass UBSan data from hyp/nVHE" into
one commit as these changes don't work without each other.
George Popescu (9):
KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to
kernel
KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
KVM: arm64: Enable UBsan check for unreachable code inside hyp/nVHE
code
KVM: arm64: Enable shift out of bounds undefined behaviour check for
hyp/nVHE
KVM: arm64: __ubsan_handle_load_invalid_value EL2 implementation.
KVM: arm64: Detect type mismatch undefined behaviour from hyp/nVHE
code
KVM: arm64: Detect arithmetic overflow is inside hyp/nVHE.
KVM: arm64: Add UBSan tests for PKVM.
arch/arm64/include/asm/assembler.h | 10 ++
arch/arm64/include/asm/kvm_debug_buffer.h | 34 ++++
arch/arm64/include/asm/kvm_host.h | 8 +-
arch/arm64/include/asm/kvm_ubsan.h | 50 ++++++
arch/arm64/kvm/Makefile | 2 +
arch/arm64/kvm/arm.c | 9 ++
arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 +++++++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 3 +-
arch/arm64/kvm/hyp/nvhe/host.S | 4 +
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
arch/arm64/kvm/hyp/nvhe/ubsan.c | 164 ++++++++++++++++++++
arch/arm64/kvm/kvm_ubsan_buffer.c | 81 ++++++++++
12 files changed, 478 insertions(+), 2 deletions(-)
create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
base-commit: 5ee88057889bbca5f5bb96031b62b3756b33e164
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 0/9] UBSan Enablement for hyp/nVHE code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, Suzuki K Poulose, Marc Zyngier, open list,
James Morse, linux-arm-kernel, George Popescu, Julien Thierry
Respin of George's patch series enabling UBSAN for hyp/nVHE code.
Updates in v3:
* rebased onto v5.11-rc3
* added base commit info
Updates in v2:
* CONFIG_KVM_ARM_DEBUG_BUFFER removed; __kvm_check_ubsan_buffer is
called directly instead of via __kvm_arm_check_debug_buffer.
* Bugfixing commits removed as these are already upstream.
* Some code brought up to date, i.e. moved from entry.S to host.S.
* Merged "Add support for creating and checking a buffer" and
"Add a buffer that can pass UBSan data from hyp/nVHE" into
one commit as these changes don't work without each other.
George Popescu (9):
KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to
kernel
KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
KVM: arm64: Enable UBsan check for unreachable code inside hyp/nVHE
code
KVM: arm64: Enable shift out of bounds undefined behaviour check for
hyp/nVHE
KVM: arm64: __ubsan_handle_load_invalid_value EL2 implementation.
KVM: arm64: Detect type mismatch undefined behaviour from hyp/nVHE
code
KVM: arm64: Detect arithmetic overflow is inside hyp/nVHE.
KVM: arm64: Add UBSan tests for PKVM.
arch/arm64/include/asm/assembler.h | 10 ++
arch/arm64/include/asm/kvm_debug_buffer.h | 34 ++++
arch/arm64/include/asm/kvm_host.h | 8 +-
arch/arm64/include/asm/kvm_ubsan.h | 50 ++++++
arch/arm64/kvm/Makefile | 2 +
arch/arm64/kvm/arm.c | 9 ++
arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 +++++++++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 3 +-
arch/arm64/kvm/hyp/nvhe/host.S | 4 +
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
arch/arm64/kvm/hyp/nvhe/ubsan.c | 164 ++++++++++++++++++++
arch/arm64/kvm/kvm_ubsan_buffer.c | 81 ++++++++++
12 files changed, 478 insertions(+), 2 deletions(-)
create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
base-commit: 5ee88057889bbca5f5bb96031b62b3756b33e164
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 1/9] KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George-Aurelian Popescu
From: George-Aurelian Popescu <georgepope@google.com>
Implement UBSan handlers inside nVHe hyp code, as empty functions for the
moment, so the undefined behaviours, that are triggered there, will be
linked to them, not to the ones defined in kernel-proper lib/ubsan.c.
In this way, enabling UBSAN_MISC won't cause a link error.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index 1f1e351c5fe2..2a683e7c6c5b 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -10,6 +10,8 @@ obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
hyp-main.o hyp-smp.o psci-relay.o
obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
../fpsimd.o ../hyp-entry.o ../exception.o
+obj-$(CONFIG_UBSAN) += ubsan.o
+CFLAGS_ubsan.nvhe.o += -I $(srctree)/lib/
##
## Build rules for compiling nVHE hyp code
@@ -61,7 +63,6 @@ KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAG
# cause crashes. Just disable it.
GCOV_PROFILE := n
KASAN_SANITIZE := n
-UBSAN_SANITIZE := n
KCOV_INSTRUMENT := n
# Skip objtool checking for this directory because nVHE code is compiled with
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
new file mode 100644
index 000000000000..a5db6b61ceb2
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+#include <linux/ctype.h>
+#include <linux/types.h>
+#include <ubsan.h>
+
+void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
+
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
+
+void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
+
+void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
+
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_builtin_unreachable(void *_data) {}
+
+void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 1/9] KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George-Aurelian Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George-Aurelian Popescu <georgepope@google.com>
Implement UBSan handlers inside nVHe hyp code, as empty functions for the
moment, so the undefined behaviours, that are triggered there, will be
linked to them, not to the ones defined in kernel-proper lib/ubsan.c.
In this way, enabling UBSAN_MISC won't cause a link error.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index 1f1e351c5fe2..2a683e7c6c5b 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -10,6 +10,8 @@ obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
hyp-main.o hyp-smp.o psci-relay.o
obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
../fpsimd.o ../hyp-entry.o ../exception.o
+obj-$(CONFIG_UBSAN) += ubsan.o
+CFLAGS_ubsan.nvhe.o += -I $(srctree)/lib/
##
## Build rules for compiling nVHE hyp code
@@ -61,7 +63,6 @@ KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAG
# cause crashes. Just disable it.
GCOV_PROFILE := n
KASAN_SANITIZE := n
-UBSAN_SANITIZE := n
KCOV_INSTRUMENT := n
# Skip objtool checking for this directory because nVHE code is compiled with
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
new file mode 100644
index 000000000000..a5db6b61ceb2
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+#include <linux/ctype.h>
+#include <linux/types.h>
+#include <ubsan.h>
+
+void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
+
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
+
+void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
+
+void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
+
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_builtin_unreachable(void *_data) {}
+
+void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 1/9] KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George-Aurelian Popescu, Elena Petrova, Suzuki K Poulose,
Marc Zyngier, open list, James Morse, linux-arm-kernel,
George Popescu, Julien Thierry
From: George-Aurelian Popescu <georgepope@google.com>
Implement UBSan handlers inside nVHe hyp code, as empty functions for the
moment, so the undefined behaviours, that are triggered there, will be
linked to them, not to the ones defined in kernel-proper lib/ubsan.c.
In this way, enabling UBSAN_MISC won't cause a link error.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index 1f1e351c5fe2..2a683e7c6c5b 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -10,6 +10,8 @@ obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
hyp-main.o hyp-smp.o psci-relay.o
obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
../fpsimd.o ../hyp-entry.o ../exception.o
+obj-$(CONFIG_UBSAN) += ubsan.o
+CFLAGS_ubsan.nvhe.o += -I $(srctree)/lib/
##
## Build rules for compiling nVHE hyp code
@@ -61,7 +63,6 @@ KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAG
# cause crashes. Just disable it.
GCOV_PROFILE := n
KASAN_SANITIZE := n
-UBSAN_SANITIZE := n
KCOV_INSTRUMENT := n
# Skip objtool checking for this directory because nVHE code is compiled with
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
new file mode 100644
index 000000000000..a5db6b61ceb2
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+#include <linux/ctype.h>
+#include <linux/types.h>
+#include <ubsan.h>
+
+void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
+
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
+
+void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
+
+void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
+
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
+
+void __ubsan_handle_builtin_unreachable(void *_data) {}
+
+void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 2/9] KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to kernel
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George Popescu
From: George Popescu <georgepope@google.com>
Share a buffer between the kernel and the hyp/nVHE code by using the
macros from kvm_debug_buffer.h.
The hyp/nVHE code requires a write index which counts how many elements
have been writtens inside the buffer and the kernel requires a read
index which counts how many elements have been read from the buffer.
The write index and the buffer are shared with the kernel in read-only.
The kvm_debug_buffer_ind returns the reading and writing points of the
circular buffer and updates the reading index.
Data collected from UBSan handlers inside hyp/nVHE is stored in the
kvm_ubsan_buffer.
This buffer stores only UBSan data because it should not be preoccupied
by other mechanisms data structures and functionalities.
Also, for the moment the buffer is mapped inside .bss, where both the kernel
and the hyp/nVHE code have Read/Write rights, but in the future this will change
and the kernel will not be able to acess hyp/nVHE's .bss. At that point the buffer
will only need to be mapped in order for this patch to work.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/assembler.h | 11 +++++++
arch/arm64/include/asm/kvm_debug_buffer.h | 36 ++++++++++++++++++++
arch/arm64/include/asm/kvm_host.h | 8 ++++-
arch/arm64/include/asm/kvm_ubsan.h | 14 ++++++++
arch/arm64/kvm/Makefile | 2 ++
arch/arm64/kvm/arm.c | 9 +++++
arch/arm64/kvm/hyp/nvhe/host.S | 4 +++
arch/arm64/kvm/hyp/nvhe/ubsan.c | 23 +++++++++++++
arch/arm64/kvm/kvm_ubsan_buffer.c | 40 +++++++++++++++++++++++
9 files changed, 146 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index bf125c591116..ebc18a8a0e1f 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -258,6 +258,17 @@ alternative_endif
ldr \dst, [\dst, \tmp]
.endm
+ /*
+ * @sym: The name of the per-cpu variable
+ * @reg: value to store
+ * @tmp1: scratch register
+ * @tmp2: scratch register
+ */
+ .macro str_this_cpu sym, reg, tmp1, tmp2
+ adr_this_cpu \tmp1, \sym, \tmp2
+ str \reg, [\tmp1]
+ .endm
+
/*
* vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
*/
diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
new file mode 100644
index 000000000000..e5375c2cff1a
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_debug_buffer.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <linux/percpu-defs.h>
+
+
+#define KVM_DEBUG_BUFFER_SIZE 1000
+
+#ifdef __KVM_NVHE_HYPERVISOR__
+#define DEFINE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DEFINE_PER_CPU(type_name, buffer_name)[size]; \
+ DEFINE_PER_CPU(unsigned long, write_ind) = 0;
+
+#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DECLARE_PER_CPU(type_name, buffer_name)[size]; \
+ DECLARE_PER_CPU(unsigned long, write_ind);
+#else
+#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
+ DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
+#endif //__KVM_NVHE_HYPERVISOR__
+
+#ifdef __ASSEMBLY__
+#include <asm/assembler.h>
+
+.macro clear_buffer tmp1, tmp2, tmp3
+ mov \tmp1, 0
+#ifdef CONFIG_UBSAN
+ str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
+#endif //CONFIG_UBSAN
+.endm
+
+#endif
\ No newline at end of file
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 8fcfab0c2567..905ca0d7c52c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -592,6 +592,12 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
void kvm_arm_halt_guest(struct kvm *kvm);
void kvm_arm_resume_guest(struct kvm *kvm);
+
+#ifdef CONFIG_UBSAN
+extern void __kvm_check_ubsan_buffer(void);
+#else
+static inline void __kvm_check_ubsan_buffer(void) {}
+#endif /* CONFIG_UBSAN */
#define kvm_call_hyp_nvhe(f, ...) \
({ \
struct arm_smccc_res res; \
@@ -599,7 +605,7 @@ void kvm_arm_resume_guest(struct kvm *kvm);
arm_smccc_1_1_hvc(KVM_HOST_SMCCC_FUNC(f), \
##__VA_ARGS__, &res); \
WARN_ON(res.a0 != SMCCC_RET_SUCCESS); \
- \
+ __kvm_check_ubsan_buffer(); \
res.a1; \
})
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
new file mode 100644
index 000000000000..fb32c7fd65d4
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <ubsan.h>
+
+#define UBSAN_MAX_TYPE 6
+#define KVM_UBSAN_BUFFER_SIZE 1000
+
+struct kvm_ubsan_info {
+ int type;
+};
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 13b017284bf9..ab3a9c402550 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -4,6 +4,7 @@
#
ccflags-y += -I $(srctree)/$(src)
+CFLAGS_kvm_ubsan_buffer.o += -I $(srctree)/lib/
KVM=../../../virt/kvm
@@ -25,3 +26,4 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
vgic/vgic-its.o vgic/vgic-debug.o
kvm-$(CONFIG_HW_PERF_EVENTS) += pmu-emul.o
+kvm-$(CONFIG_UBSAN) += kvm_ubsan_buffer.o
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 04c44853b103..63f0495550b8 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1784,6 +1784,15 @@ static int init_hyp_mode(void)
goto out_err;
}
}
+#ifdef CONFIG_UBSAN
+ /* required by ubsan to access the handlers structures fields */
+ err = create_hyp_mappings(kvm_ksym_ref(_data),
+ kvm_ksym_ref(__end_once), PAGE_HYP_RO);
+ if (err) {
+ kvm_err("Cannot map data section\n");
+ goto out_err;
+ }
+#endif
/*
* Map Hyp percpu pages
diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
index a820dfdc9c25..0967906265d3 100644
--- a/arch/arm64/kvm/hyp/nvhe/host.S
+++ b/arch/arm64/kvm/hyp/nvhe/host.S
@@ -8,6 +8,7 @@
#include <asm/assembler.h>
#include <asm/kvm_asm.h>
+#include <asm/kvm_debug_buffer.h>
#include <asm/kvm_mmu.h>
.text
@@ -34,6 +35,9 @@ SYM_FUNC_START(__host_exit)
/* Store the host regs x18-x29, lr */
save_callee_saved_regs x0
+ /* when entering the host clear the buffers */
+ clear_buffer x4, x5, x6
+
/* Save the host context pointer in x29 across the function call */
mov x29, x0
bl handle_trap
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index a5db6b61ceb2..8a194fb1f6cf 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -3,10 +3,33 @@
* Copyright 2020 Google LLC
* Author: George Popescu <georgepope@google.com>
*/
+#include <linux/bitops.h>
#include <linux/ctype.h>
#include <linux/types.h>
+#include <linux/percpu-defs.h>
+#include <linux/kvm_host.h>
+#include <asm/kvm_arm.h>
+#include <asm/kvm_asm.h>
+#include <asm/kvm_ubsan.h>
+#include <asm/kvm_debug_buffer.h>
+#include <kvm/arm_pmu.h>
#include <ubsan.h>
+DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
+ kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+
+static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
+{
+ struct kvm_ubsan_info *res = NULL;
+ unsigned long write_ind = __this_cpu_read(kvm_ubsan_buff_wr_ind);
+ if (write_ind < KVM_UBSAN_BUFFER_SIZE) {
+ res = this_cpu_ptr(&kvm_ubsan_buffer[write_ind]);
+ ++write_ind;
+ __this_cpu_write(kvm_ubsan_buff_wr_ind, write_ind);
+ }
+ return res;
+}
+
void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
new file mode 100644
index 000000000000..4a1959ba9f68
--- /dev/null
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <linux/ctype.h>
+#include <linux/types.h>
+#include <asm/kvm_debug_buffer.h>
+#include <asm/kvm_arm.h>
+#include <asm/kvm_asm.h>
+#include <kvm/arm_pmu.h>
+
+#include <ubsan.h>
+#include <asm/kvm_ubsan.h>
+
+DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
+ kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+
+
+void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
+{
+ unsigned long i;
+ struct kvm_ubsan_info *slot;
+
+ slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
+ for (i = left; i < right; ++i) {
+ /* check ubsan data */
+ slot[i].type = 0;
+ }
+}
+
+void __kvm_check_ubsan_buffer(void)
+{
+ unsigned long *write_ind;
+
+ write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
+ iterate_kvm_ubsan_buffer(0, *write_ind);
+}
+
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 2/9] KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to kernel
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George Popescu <georgepope@google.com>
Share a buffer between the kernel and the hyp/nVHE code by using the
macros from kvm_debug_buffer.h.
The hyp/nVHE code requires a write index which counts how many elements
have been writtens inside the buffer and the kernel requires a read
index which counts how many elements have been read from the buffer.
The write index and the buffer are shared with the kernel in read-only.
The kvm_debug_buffer_ind returns the reading and writing points of the
circular buffer and updates the reading index.
Data collected from UBSan handlers inside hyp/nVHE is stored in the
kvm_ubsan_buffer.
This buffer stores only UBSan data because it should not be preoccupied
by other mechanisms data structures and functionalities.
Also, for the moment the buffer is mapped inside .bss, where both the kernel
and the hyp/nVHE code have Read/Write rights, but in the future this will change
and the kernel will not be able to acess hyp/nVHE's .bss. At that point the buffer
will only need to be mapped in order for this patch to work.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/assembler.h | 11 +++++++
arch/arm64/include/asm/kvm_debug_buffer.h | 36 ++++++++++++++++++++
arch/arm64/include/asm/kvm_host.h | 8 ++++-
arch/arm64/include/asm/kvm_ubsan.h | 14 ++++++++
arch/arm64/kvm/Makefile | 2 ++
arch/arm64/kvm/arm.c | 9 +++++
arch/arm64/kvm/hyp/nvhe/host.S | 4 +++
arch/arm64/kvm/hyp/nvhe/ubsan.c | 23 +++++++++++++
arch/arm64/kvm/kvm_ubsan_buffer.c | 40 +++++++++++++++++++++++
9 files changed, 146 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index bf125c591116..ebc18a8a0e1f 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -258,6 +258,17 @@ alternative_endif
ldr \dst, [\dst, \tmp]
.endm
+ /*
+ * @sym: The name of the per-cpu variable
+ * @reg: value to store
+ * @tmp1: scratch register
+ * @tmp2: scratch register
+ */
+ .macro str_this_cpu sym, reg, tmp1, tmp2
+ adr_this_cpu \tmp1, \sym, \tmp2
+ str \reg, [\tmp1]
+ .endm
+
/*
* vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
*/
diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
new file mode 100644
index 000000000000..e5375c2cff1a
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_debug_buffer.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <linux/percpu-defs.h>
+
+
+#define KVM_DEBUG_BUFFER_SIZE 1000
+
+#ifdef __KVM_NVHE_HYPERVISOR__
+#define DEFINE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DEFINE_PER_CPU(type_name, buffer_name)[size]; \
+ DEFINE_PER_CPU(unsigned long, write_ind) = 0;
+
+#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DECLARE_PER_CPU(type_name, buffer_name)[size]; \
+ DECLARE_PER_CPU(unsigned long, write_ind);
+#else
+#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
+ DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
+#endif //__KVM_NVHE_HYPERVISOR__
+
+#ifdef __ASSEMBLY__
+#include <asm/assembler.h>
+
+.macro clear_buffer tmp1, tmp2, tmp3
+ mov \tmp1, 0
+#ifdef CONFIG_UBSAN
+ str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
+#endif //CONFIG_UBSAN
+.endm
+
+#endif
\ No newline at end of file
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 8fcfab0c2567..905ca0d7c52c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -592,6 +592,12 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
void kvm_arm_halt_guest(struct kvm *kvm);
void kvm_arm_resume_guest(struct kvm *kvm);
+
+#ifdef CONFIG_UBSAN
+extern void __kvm_check_ubsan_buffer(void);
+#else
+static inline void __kvm_check_ubsan_buffer(void) {}
+#endif /* CONFIG_UBSAN */
#define kvm_call_hyp_nvhe(f, ...) \
({ \
struct arm_smccc_res res; \
@@ -599,7 +605,7 @@ void kvm_arm_resume_guest(struct kvm *kvm);
arm_smccc_1_1_hvc(KVM_HOST_SMCCC_FUNC(f), \
##__VA_ARGS__, &res); \
WARN_ON(res.a0 != SMCCC_RET_SUCCESS); \
- \
+ __kvm_check_ubsan_buffer(); \
res.a1; \
})
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
new file mode 100644
index 000000000000..fb32c7fd65d4
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <ubsan.h>
+
+#define UBSAN_MAX_TYPE 6
+#define KVM_UBSAN_BUFFER_SIZE 1000
+
+struct kvm_ubsan_info {
+ int type;
+};
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 13b017284bf9..ab3a9c402550 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -4,6 +4,7 @@
#
ccflags-y += -I $(srctree)/$(src)
+CFLAGS_kvm_ubsan_buffer.o += -I $(srctree)/lib/
KVM=../../../virt/kvm
@@ -25,3 +26,4 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
vgic/vgic-its.o vgic/vgic-debug.o
kvm-$(CONFIG_HW_PERF_EVENTS) += pmu-emul.o
+kvm-$(CONFIG_UBSAN) += kvm_ubsan_buffer.o
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 04c44853b103..63f0495550b8 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1784,6 +1784,15 @@ static int init_hyp_mode(void)
goto out_err;
}
}
+#ifdef CONFIG_UBSAN
+ /* required by ubsan to access the handlers structures fields */
+ err = create_hyp_mappings(kvm_ksym_ref(_data),
+ kvm_ksym_ref(__end_once), PAGE_HYP_RO);
+ if (err) {
+ kvm_err("Cannot map data section\n");
+ goto out_err;
+ }
+#endif
/*
* Map Hyp percpu pages
diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
index a820dfdc9c25..0967906265d3 100644
--- a/arch/arm64/kvm/hyp/nvhe/host.S
+++ b/arch/arm64/kvm/hyp/nvhe/host.S
@@ -8,6 +8,7 @@
#include <asm/assembler.h>
#include <asm/kvm_asm.h>
+#include <asm/kvm_debug_buffer.h>
#include <asm/kvm_mmu.h>
.text
@@ -34,6 +35,9 @@ SYM_FUNC_START(__host_exit)
/* Store the host regs x18-x29, lr */
save_callee_saved_regs x0
+ /* when entering the host clear the buffers */
+ clear_buffer x4, x5, x6
+
/* Save the host context pointer in x29 across the function call */
mov x29, x0
bl handle_trap
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index a5db6b61ceb2..8a194fb1f6cf 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -3,10 +3,33 @@
* Copyright 2020 Google LLC
* Author: George Popescu <georgepope@google.com>
*/
+#include <linux/bitops.h>
#include <linux/ctype.h>
#include <linux/types.h>
+#include <linux/percpu-defs.h>
+#include <linux/kvm_host.h>
+#include <asm/kvm_arm.h>
+#include <asm/kvm_asm.h>
+#include <asm/kvm_ubsan.h>
+#include <asm/kvm_debug_buffer.h>
+#include <kvm/arm_pmu.h>
#include <ubsan.h>
+DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
+ kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+
+static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
+{
+ struct kvm_ubsan_info *res = NULL;
+ unsigned long write_ind = __this_cpu_read(kvm_ubsan_buff_wr_ind);
+ if (write_ind < KVM_UBSAN_BUFFER_SIZE) {
+ res = this_cpu_ptr(&kvm_ubsan_buffer[write_ind]);
+ ++write_ind;
+ __this_cpu_write(kvm_ubsan_buff_wr_ind, write_ind);
+ }
+ return res;
+}
+
void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
new file mode 100644
index 000000000000..4a1959ba9f68
--- /dev/null
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <linux/ctype.h>
+#include <linux/types.h>
+#include <asm/kvm_debug_buffer.h>
+#include <asm/kvm_arm.h>
+#include <asm/kvm_asm.h>
+#include <kvm/arm_pmu.h>
+
+#include <ubsan.h>
+#include <asm/kvm_ubsan.h>
+
+DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
+ kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+
+
+void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
+{
+ unsigned long i;
+ struct kvm_ubsan_info *slot;
+
+ slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
+ for (i = left; i < right; ++i) {
+ /* check ubsan data */
+ slot[i].type = 0;
+ }
+}
+
+void __kvm_check_ubsan_buffer(void)
+{
+ unsigned long *write_ind;
+
+ write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
+ iterate_kvm_ubsan_buffer(0, *write_ind);
+}
+
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 2/9] KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to kernel
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Suzuki K Poulose, Marc Zyngier,
open list, James Morse, linux-arm-kernel, George Popescu,
Julien Thierry
From: George Popescu <georgepope@google.com>
Share a buffer between the kernel and the hyp/nVHE code by using the
macros from kvm_debug_buffer.h.
The hyp/nVHE code requires a write index which counts how many elements
have been writtens inside the buffer and the kernel requires a read
index which counts how many elements have been read from the buffer.
The write index and the buffer are shared with the kernel in read-only.
The kvm_debug_buffer_ind returns the reading and writing points of the
circular buffer and updates the reading index.
Data collected from UBSan handlers inside hyp/nVHE is stored in the
kvm_ubsan_buffer.
This buffer stores only UBSan data because it should not be preoccupied
by other mechanisms data structures and functionalities.
Also, for the moment the buffer is mapped inside .bss, where both the kernel
and the hyp/nVHE code have Read/Write rights, but in the future this will change
and the kernel will not be able to acess hyp/nVHE's .bss. At that point the buffer
will only need to be mapped in order for this patch to work.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/assembler.h | 11 +++++++
arch/arm64/include/asm/kvm_debug_buffer.h | 36 ++++++++++++++++++++
arch/arm64/include/asm/kvm_host.h | 8 ++++-
arch/arm64/include/asm/kvm_ubsan.h | 14 ++++++++
arch/arm64/kvm/Makefile | 2 ++
arch/arm64/kvm/arm.c | 9 +++++
arch/arm64/kvm/hyp/nvhe/host.S | 4 +++
arch/arm64/kvm/hyp/nvhe/ubsan.c | 23 +++++++++++++
arch/arm64/kvm/kvm_ubsan_buffer.c | 40 +++++++++++++++++++++++
9 files changed, 146 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index bf125c591116..ebc18a8a0e1f 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -258,6 +258,17 @@ alternative_endif
ldr \dst, [\dst, \tmp]
.endm
+ /*
+ * @sym: The name of the per-cpu variable
+ * @reg: value to store
+ * @tmp1: scratch register
+ * @tmp2: scratch register
+ */
+ .macro str_this_cpu sym, reg, tmp1, tmp2
+ adr_this_cpu \tmp1, \sym, \tmp2
+ str \reg, [\tmp1]
+ .endm
+
/*
* vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
*/
diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
new file mode 100644
index 000000000000..e5375c2cff1a
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_debug_buffer.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <linux/percpu-defs.h>
+
+
+#define KVM_DEBUG_BUFFER_SIZE 1000
+
+#ifdef __KVM_NVHE_HYPERVISOR__
+#define DEFINE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DEFINE_PER_CPU(type_name, buffer_name)[size]; \
+ DEFINE_PER_CPU(unsigned long, write_ind) = 0;
+
+#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DECLARE_PER_CPU(type_name, buffer_name)[size]; \
+ DECLARE_PER_CPU(unsigned long, write_ind);
+#else
+#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
+ DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
+ DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
+#endif //__KVM_NVHE_HYPERVISOR__
+
+#ifdef __ASSEMBLY__
+#include <asm/assembler.h>
+
+.macro clear_buffer tmp1, tmp2, tmp3
+ mov \tmp1, 0
+#ifdef CONFIG_UBSAN
+ str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
+#endif //CONFIG_UBSAN
+.endm
+
+#endif
\ No newline at end of file
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 8fcfab0c2567..905ca0d7c52c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -592,6 +592,12 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
void kvm_arm_halt_guest(struct kvm *kvm);
void kvm_arm_resume_guest(struct kvm *kvm);
+
+#ifdef CONFIG_UBSAN
+extern void __kvm_check_ubsan_buffer(void);
+#else
+static inline void __kvm_check_ubsan_buffer(void) {}
+#endif /* CONFIG_UBSAN */
#define kvm_call_hyp_nvhe(f, ...) \
({ \
struct arm_smccc_res res; \
@@ -599,7 +605,7 @@ void kvm_arm_resume_guest(struct kvm *kvm);
arm_smccc_1_1_hvc(KVM_HOST_SMCCC_FUNC(f), \
##__VA_ARGS__, &res); \
WARN_ON(res.a0 != SMCCC_RET_SUCCESS); \
- \
+ __kvm_check_ubsan_buffer(); \
res.a1; \
})
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
new file mode 100644
index 000000000000..fb32c7fd65d4
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <ubsan.h>
+
+#define UBSAN_MAX_TYPE 6
+#define KVM_UBSAN_BUFFER_SIZE 1000
+
+struct kvm_ubsan_info {
+ int type;
+};
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 13b017284bf9..ab3a9c402550 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -4,6 +4,7 @@
#
ccflags-y += -I $(srctree)/$(src)
+CFLAGS_kvm_ubsan_buffer.o += -I $(srctree)/lib/
KVM=../../../virt/kvm
@@ -25,3 +26,4 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
vgic/vgic-its.o vgic/vgic-debug.o
kvm-$(CONFIG_HW_PERF_EVENTS) += pmu-emul.o
+kvm-$(CONFIG_UBSAN) += kvm_ubsan_buffer.o
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 04c44853b103..63f0495550b8 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1784,6 +1784,15 @@ static int init_hyp_mode(void)
goto out_err;
}
}
+#ifdef CONFIG_UBSAN
+ /* required by ubsan to access the handlers structures fields */
+ err = create_hyp_mappings(kvm_ksym_ref(_data),
+ kvm_ksym_ref(__end_once), PAGE_HYP_RO);
+ if (err) {
+ kvm_err("Cannot map data section\n");
+ goto out_err;
+ }
+#endif
/*
* Map Hyp percpu pages
diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
index a820dfdc9c25..0967906265d3 100644
--- a/arch/arm64/kvm/hyp/nvhe/host.S
+++ b/arch/arm64/kvm/hyp/nvhe/host.S
@@ -8,6 +8,7 @@
#include <asm/assembler.h>
#include <asm/kvm_asm.h>
+#include <asm/kvm_debug_buffer.h>
#include <asm/kvm_mmu.h>
.text
@@ -34,6 +35,9 @@ SYM_FUNC_START(__host_exit)
/* Store the host regs x18-x29, lr */
save_callee_saved_regs x0
+ /* when entering the host clear the buffers */
+ clear_buffer x4, x5, x6
+
/* Save the host context pointer in x29 across the function call */
mov x29, x0
bl handle_trap
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index a5db6b61ceb2..8a194fb1f6cf 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -3,10 +3,33 @@
* Copyright 2020 Google LLC
* Author: George Popescu <georgepope@google.com>
*/
+#include <linux/bitops.h>
#include <linux/ctype.h>
#include <linux/types.h>
+#include <linux/percpu-defs.h>
+#include <linux/kvm_host.h>
+#include <asm/kvm_arm.h>
+#include <asm/kvm_asm.h>
+#include <asm/kvm_ubsan.h>
+#include <asm/kvm_debug_buffer.h>
+#include <kvm/arm_pmu.h>
#include <ubsan.h>
+DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
+ kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+
+static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
+{
+ struct kvm_ubsan_info *res = NULL;
+ unsigned long write_ind = __this_cpu_read(kvm_ubsan_buff_wr_ind);
+ if (write_ind < KVM_UBSAN_BUFFER_SIZE) {
+ res = this_cpu_ptr(&kvm_ubsan_buffer[write_ind]);
+ ++write_ind;
+ __this_cpu_write(kvm_ubsan_buff_wr_ind, write_ind);
+ }
+ return res;
+}
+
void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
new file mode 100644
index 000000000000..4a1959ba9f68
--- /dev/null
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 Google LLC
+ * Author: George Popescu <georgepope@google.com>
+ */
+
+#include <linux/ctype.h>
+#include <linux/types.h>
+#include <asm/kvm_debug_buffer.h>
+#include <asm/kvm_arm.h>
+#include <asm/kvm_asm.h>
+#include <kvm/arm_pmu.h>
+
+#include <ubsan.h>
+#include <asm/kvm_ubsan.h>
+
+DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
+ kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+
+
+void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
+{
+ unsigned long i;
+ struct kvm_ubsan_info *slot;
+
+ slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
+ for (i = left; i < right; ++i) {
+ /* check ubsan data */
+ slot[i].type = 0;
+ }
+}
+
+void __kvm_check_ubsan_buffer(void)
+{
+ unsigned long *write_ind;
+
+ write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
+ iterate_kvm_ubsan_buffer(0, *write_ind);
+}
+
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 3/9] KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George Popescu
From: George Popescu <georgepope@google.com>
If an out of bounds happens inside the hyp/nVHE code, the ubsan_out_of_bounds
handler stores the logging data inside the kvm_ubsan_buffer. The one responsible
for printing is the kernel ubsan_out_of_bounds handler. The process of
decapsulating the data from the buffer is straightforward.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 19 ++++++++++++++++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 ++++++++++++--
arch/arm64/kvm/kvm_ubsan_buffer.c | 10 ++++++++++
3 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index fb32c7fd65d4..4f471acb88b0 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -9,6 +9,23 @@
#define UBSAN_MAX_TYPE 6
#define KVM_UBSAN_BUFFER_SIZE 1000
+
+struct ubsan_values {
+ void *lval;
+ void *rval;
+ char op;
+};
+
struct kvm_ubsan_info {
- int type;
+ enum {
+ UBSAN_OUT_OF_BOUNDS,
+ } type;
+ union {
+ struct out_of_bounds_data out_of_bounds_data;
+ };
+ union {
+ struct ubsan_values u_val;
+ };
};
+
+void __ubsan_handle_out_of_bounds(void *_data, void *index);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 8a194fb1f6cf..55a8f6db8555 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -13,7 +13,6 @@
#include <asm/kvm_ubsan.h>
#include <asm/kvm_debug_buffer.h>
#include <kvm/arm_pmu.h>
-#include <ubsan.h>
DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
@@ -44,7 +43,18 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
-void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
+void __ubsan_handle_out_of_bounds(void *_data, void *index)
+{
+ struct kvm_ubsan_info *slot;
+ struct out_of_bounds_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_OUT_OF_BOUNDS;
+ slot->out_of_bounds_data = *data;
+ slot->u_val.lval = index;
+ }
+}
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 4a1959ba9f68..a1523f86be3c 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -17,6 +17,15 @@
DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
+{
+ switch (slot->type) {
+ case UBSAN_OUT_OF_BOUNDS:
+ __ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
+ slot->u_val.lval);
+ break;
+ }
+}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
{
@@ -26,6 +35,7 @@ void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
for (i = left; i < right; ++i) {
/* check ubsan data */
+ __kvm_check_ubsan_data(slot + i);
slot[i].type = 0;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 3/9] KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George Popescu <georgepope@google.com>
If an out of bounds happens inside the hyp/nVHE code, the ubsan_out_of_bounds
handler stores the logging data inside the kvm_ubsan_buffer. The one responsible
for printing is the kernel ubsan_out_of_bounds handler. The process of
decapsulating the data from the buffer is straightforward.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 19 ++++++++++++++++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 ++++++++++++--
arch/arm64/kvm/kvm_ubsan_buffer.c | 10 ++++++++++
3 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index fb32c7fd65d4..4f471acb88b0 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -9,6 +9,23 @@
#define UBSAN_MAX_TYPE 6
#define KVM_UBSAN_BUFFER_SIZE 1000
+
+struct ubsan_values {
+ void *lval;
+ void *rval;
+ char op;
+};
+
struct kvm_ubsan_info {
- int type;
+ enum {
+ UBSAN_OUT_OF_BOUNDS,
+ } type;
+ union {
+ struct out_of_bounds_data out_of_bounds_data;
+ };
+ union {
+ struct ubsan_values u_val;
+ };
};
+
+void __ubsan_handle_out_of_bounds(void *_data, void *index);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 8a194fb1f6cf..55a8f6db8555 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -13,7 +13,6 @@
#include <asm/kvm_ubsan.h>
#include <asm/kvm_debug_buffer.h>
#include <kvm/arm_pmu.h>
-#include <ubsan.h>
DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
@@ -44,7 +43,18 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
-void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
+void __ubsan_handle_out_of_bounds(void *_data, void *index)
+{
+ struct kvm_ubsan_info *slot;
+ struct out_of_bounds_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_OUT_OF_BOUNDS;
+ slot->out_of_bounds_data = *data;
+ slot->u_val.lval = index;
+ }
+}
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 4a1959ba9f68..a1523f86be3c 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -17,6 +17,15 @@
DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
+{
+ switch (slot->type) {
+ case UBSAN_OUT_OF_BOUNDS:
+ __ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
+ slot->u_val.lval);
+ break;
+ }
+}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
{
@@ -26,6 +35,7 @@ void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
for (i = left; i < right; ++i) {
/* check ubsan data */
+ __kvm_check_ubsan_data(slot + i);
slot[i].type = 0;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 3/9] KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Suzuki K Poulose, Marc Zyngier,
open list, James Morse, linux-arm-kernel, George Popescu,
Julien Thierry
From: George Popescu <georgepope@google.com>
If an out of bounds happens inside the hyp/nVHE code, the ubsan_out_of_bounds
handler stores the logging data inside the kvm_ubsan_buffer. The one responsible
for printing is the kernel ubsan_out_of_bounds handler. The process of
decapsulating the data from the buffer is straightforward.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 19 ++++++++++++++++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 ++++++++++++--
arch/arm64/kvm/kvm_ubsan_buffer.c | 10 ++++++++++
3 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index fb32c7fd65d4..4f471acb88b0 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -9,6 +9,23 @@
#define UBSAN_MAX_TYPE 6
#define KVM_UBSAN_BUFFER_SIZE 1000
+
+struct ubsan_values {
+ void *lval;
+ void *rval;
+ char op;
+};
+
struct kvm_ubsan_info {
- int type;
+ enum {
+ UBSAN_OUT_OF_BOUNDS,
+ } type;
+ union {
+ struct out_of_bounds_data out_of_bounds_data;
+ };
+ union {
+ struct ubsan_values u_val;
+ };
};
+
+void __ubsan_handle_out_of_bounds(void *_data, void *index);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 8a194fb1f6cf..55a8f6db8555 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -13,7 +13,6 @@
#include <asm/kvm_ubsan.h>
#include <asm/kvm_debug_buffer.h>
#include <kvm/arm_pmu.h>
-#include <ubsan.h>
DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
@@ -44,7 +43,18 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
-void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
+void __ubsan_handle_out_of_bounds(void *_data, void *index)
+{
+ struct kvm_ubsan_info *slot;
+ struct out_of_bounds_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_OUT_OF_BOUNDS;
+ slot->out_of_bounds_data = *data;
+ slot->u_val.lval = index;
+ }
+}
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 4a1959ba9f68..a1523f86be3c 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -17,6 +17,15 @@
DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
+void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
+{
+ switch (slot->type) {
+ case UBSAN_OUT_OF_BOUNDS:
+ __ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
+ slot->u_val.lval);
+ break;
+ }
+}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
{
@@ -26,6 +35,7 @@ void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
for (i = left; i < right; ++i) {
/* check ubsan data */
+ __kvm_check_ubsan_data(slot + i);
slot[i].type = 0;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 4/9] KVM: arm64: Enable UBsan check for unreachable code inside hyp/nVHE code
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George Popescu
From: George Popescu <georgepope@google.com>
The data from __ubsan_handle_builtin_unreachable is passed to the buffer
and printed inside the kernel by its simetric handler.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 3 +++
arch/arm64/kvm/hyp/nvhe/ubsan.c | 12 +++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 3 +++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 4f471acb88b0..70c6f2541d07 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -19,9 +19,11 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
+ UBSAN_UNREACHABLE_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
+ struct unreachable_data unreachable_data;
};
union {
struct ubsan_values u_val;
@@ -29,3 +31,4 @@ struct kvm_ubsan_info {
};
void __ubsan_handle_out_of_bounds(void *_data, void *index);
+void __ubsan_handle_builtin_unreachable(void *_data);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 55a8f6db8555..5e55897b2d72 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -58,6 +58,16 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index)
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
-void __ubsan_handle_builtin_unreachable(void *_data) {}
+void __ubsan_handle_builtin_unreachable(void *_data)
+{
+ struct kvm_ubsan_info *slot;
+ struct unreachable_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_UNREACHABLE_DATA;
+ slot->unreachable_data = *data;
+ }
+}
void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index a1523f86be3c..e51949c275aa 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -24,6 +24,9 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
slot->u_val.lval);
break;
+ case UBSAN_UNREACHABLE_DATA:
+ __ubsan_handle_builtin_unreachable(&slot->unreachable_data);
+ break;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 4/9] KVM: arm64: Enable UBsan check for unreachable code inside hyp/nVHE code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George Popescu <georgepope@google.com>
The data from __ubsan_handle_builtin_unreachable is passed to the buffer
and printed inside the kernel by its simetric handler.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 3 +++
arch/arm64/kvm/hyp/nvhe/ubsan.c | 12 +++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 3 +++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 4f471acb88b0..70c6f2541d07 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -19,9 +19,11 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
+ UBSAN_UNREACHABLE_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
+ struct unreachable_data unreachable_data;
};
union {
struct ubsan_values u_val;
@@ -29,3 +31,4 @@ struct kvm_ubsan_info {
};
void __ubsan_handle_out_of_bounds(void *_data, void *index);
+void __ubsan_handle_builtin_unreachable(void *_data);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 55a8f6db8555..5e55897b2d72 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -58,6 +58,16 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index)
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
-void __ubsan_handle_builtin_unreachable(void *_data) {}
+void __ubsan_handle_builtin_unreachable(void *_data)
+{
+ struct kvm_ubsan_info *slot;
+ struct unreachable_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_UNREACHABLE_DATA;
+ slot->unreachable_data = *data;
+ }
+}
void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index a1523f86be3c..e51949c275aa 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -24,6 +24,9 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
slot->u_val.lval);
break;
+ case UBSAN_UNREACHABLE_DATA:
+ __ubsan_handle_builtin_unreachable(&slot->unreachable_data);
+ break;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 4/9] KVM: arm64: Enable UBsan check for unreachable code inside hyp/nVHE code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Suzuki K Poulose, Marc Zyngier,
open list, James Morse, linux-arm-kernel, George Popescu,
Julien Thierry
From: George Popescu <georgepope@google.com>
The data from __ubsan_handle_builtin_unreachable is passed to the buffer
and printed inside the kernel by its simetric handler.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 3 +++
arch/arm64/kvm/hyp/nvhe/ubsan.c | 12 +++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 3 +++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 4f471acb88b0..70c6f2541d07 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -19,9 +19,11 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
+ UBSAN_UNREACHABLE_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
+ struct unreachable_data unreachable_data;
};
union {
struct ubsan_values u_val;
@@ -29,3 +31,4 @@ struct kvm_ubsan_info {
};
void __ubsan_handle_out_of_bounds(void *_data, void *index);
+void __ubsan_handle_builtin_unreachable(void *_data);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 55a8f6db8555..5e55897b2d72 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -58,6 +58,16 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index)
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
-void __ubsan_handle_builtin_unreachable(void *_data) {}
+void __ubsan_handle_builtin_unreachable(void *_data)
+{
+ struct kvm_ubsan_info *slot;
+ struct unreachable_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_UNREACHABLE_DATA;
+ slot->unreachable_data = *data;
+ }
+}
void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index a1523f86be3c..e51949c275aa 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -24,6 +24,9 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
slot->u_val.lval);
break;
+ case UBSAN_UNREACHABLE_DATA:
+ __ubsan_handle_builtin_unreachable(&slot->unreachable_data);
+ break;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 5/9] KVM: arm64: Enable shift out of bounds undefined behaviour check for hyp/nVHE
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George Popescu
From: George Popescu <georgepope@google.com>
__ubsan_handle_shift_out_of_bounds data is passed to the buffer inside
hyp/nVHE. This data is passed to the original handler from kernel.
The 64bit values of the shift expression operands are stored as the lhs
and rhs pointers, so there is no need to dereference them.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 5 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 +++++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 4 ++++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 70c6f2541d07..0eef0e11a93b 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -19,11 +19,13 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
- UBSAN_UNREACHABLE_DATA
+ UBSAN_UNREACHABLE_DATA,
+ UBSAN_SHIFT_OUT_OF_BOUNDS
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
+ struct shift_out_of_bounds_data shift_out_of_bounds_data;
};
union {
struct ubsan_values u_val;
@@ -32,3 +34,4 @@ struct kvm_ubsan_info {
void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 5e55897b2d72..1069ed5036d5 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -56,7 +56,19 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index)
}
}
-void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs)
+{
+ struct kvm_ubsan_info *slot;
+ struct shift_out_of_bounds_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_SHIFT_OUT_OF_BOUNDS;
+ slot->shift_out_of_bounds_data = *data;
+ slot->u_val.lval = lhs;
+ slot->u_val.rval = rhs;
+ }
+}
void __ubsan_handle_builtin_unreachable(void *_data)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index e51949c275aa..b80045883047 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -27,6 +27,10 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
case UBSAN_UNREACHABLE_DATA:
__ubsan_handle_builtin_unreachable(&slot->unreachable_data);
break;
+ case UBSAN_SHIFT_OUT_OF_BOUNDS:
+ __ubsan_handle_shift_out_of_bounds(&slot->shift_out_of_bounds_data,
+ slot->u_val.lval, slot->u_val.rval);
+ break;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 5/9] KVM: arm64: Enable shift out of bounds undefined behaviour check for hyp/nVHE
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George Popescu <georgepope@google.com>
__ubsan_handle_shift_out_of_bounds data is passed to the buffer inside
hyp/nVHE. This data is passed to the original handler from kernel.
The 64bit values of the shift expression operands are stored as the lhs
and rhs pointers, so there is no need to dereference them.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 5 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 +++++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 4 ++++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 70c6f2541d07..0eef0e11a93b 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -19,11 +19,13 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
- UBSAN_UNREACHABLE_DATA
+ UBSAN_UNREACHABLE_DATA,
+ UBSAN_SHIFT_OUT_OF_BOUNDS
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
+ struct shift_out_of_bounds_data shift_out_of_bounds_data;
};
union {
struct ubsan_values u_val;
@@ -32,3 +34,4 @@ struct kvm_ubsan_info {
void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 5e55897b2d72..1069ed5036d5 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -56,7 +56,19 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index)
}
}
-void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs)
+{
+ struct kvm_ubsan_info *slot;
+ struct shift_out_of_bounds_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_SHIFT_OUT_OF_BOUNDS;
+ slot->shift_out_of_bounds_data = *data;
+ slot->u_val.lval = lhs;
+ slot->u_val.rval = rhs;
+ }
+}
void __ubsan_handle_builtin_unreachable(void *_data)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index e51949c275aa..b80045883047 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -27,6 +27,10 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
case UBSAN_UNREACHABLE_DATA:
__ubsan_handle_builtin_unreachable(&slot->unreachable_data);
break;
+ case UBSAN_SHIFT_OUT_OF_BOUNDS:
+ __ubsan_handle_shift_out_of_bounds(&slot->shift_out_of_bounds_data,
+ slot->u_val.lval, slot->u_val.rval);
+ break;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 5/9] KVM: arm64: Enable shift out of bounds undefined behaviour check for hyp/nVHE
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Suzuki K Poulose, Marc Zyngier,
open list, James Morse, linux-arm-kernel, George Popescu,
Julien Thierry
From: George Popescu <georgepope@google.com>
__ubsan_handle_shift_out_of_bounds data is passed to the buffer inside
hyp/nVHE. This data is passed to the original handler from kernel.
The 64bit values of the shift expression operands are stored as the lhs
and rhs pointers, so there is no need to dereference them.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 5 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 +++++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 4 ++++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 70c6f2541d07..0eef0e11a93b 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -19,11 +19,13 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
- UBSAN_UNREACHABLE_DATA
+ UBSAN_UNREACHABLE_DATA,
+ UBSAN_SHIFT_OUT_OF_BOUNDS
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
+ struct shift_out_of_bounds_data shift_out_of_bounds_data;
};
union {
struct ubsan_values u_val;
@@ -32,3 +34,4 @@ struct kvm_ubsan_info {
void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 5e55897b2d72..1069ed5036d5 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -56,7 +56,19 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index)
}
}
-void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs)
+{
+ struct kvm_ubsan_info *slot;
+ struct shift_out_of_bounds_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_SHIFT_OUT_OF_BOUNDS;
+ slot->shift_out_of_bounds_data = *data;
+ slot->u_val.lval = lhs;
+ slot->u_val.rval = rhs;
+ }
+}
void __ubsan_handle_builtin_unreachable(void *_data)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index e51949c275aa..b80045883047 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -27,6 +27,10 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
case UBSAN_UNREACHABLE_DATA:
__ubsan_handle_builtin_unreachable(&slot->unreachable_data);
break;
+ case UBSAN_SHIFT_OUT_OF_BOUNDS:
+ __ubsan_handle_shift_out_of_bounds(&slot->shift_out_of_bounds_data,
+ slot->u_val.lval, slot->u_val.rval);
+ break;
}
}
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 6/9] KVM: arm64: __ubsan_handle_load_invalid_value EL2 implementation.
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George Popescu
From: George Popescu <georgepope@google.com>
The handler for the load invalid value undefined behaviour is
implemented at EL2. The EL2 handler's parameters are stored inside the buffer.
They are used by the symetric handler from EL1.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 5 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 +++++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 6 +++++-
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 0eef0e11a93b..95ac6728ffd1 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -20,12 +20,14 @@ struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
- UBSAN_SHIFT_OUT_OF_BOUNDS
+ UBSAN_SHIFT_OUT_OF_BOUNDS,
+ UBSAN_INVALID_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
struct shift_out_of_bounds_data shift_out_of_bounds_data;
+ struct invalid_value_data invalid_value_data;
};
union {
struct ubsan_values u_val;
@@ -35,3 +37,4 @@ struct kvm_ubsan_info {
void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
+void __ubsan_handle_load_invalid_value(void *_data, void *val);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 1069ed5036d5..3143f7722be2 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -82,4 +82,16 @@ void __ubsan_handle_builtin_unreachable(void *_data)
}
}
-void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
+void __ubsan_handle_load_invalid_value(void *_data, void *val)
+{
+ struct kvm_ubsan_info *slot;
+ struct invalid_value_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_INVALID_DATA;
+ slot->invalid_value_data = *data;
+ slot->u_val.lval = val;
+ }
+
+}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index b80045883047..5439f7a91636 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -31,7 +31,11 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_shift_out_of_bounds(&slot->shift_out_of_bounds_data,
slot->u_val.lval, slot->u_val.rval);
break;
- }
+ case UBSAN_INVALID_DATA:
+ __ubsan_handle_load_invalid_value(&slot->invalid_value_data,
+ slot->u_val.lval);
+ break;
+ }
}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 6/9] KVM: arm64: __ubsan_handle_load_invalid_value EL2 implementation.
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George Popescu <georgepope@google.com>
The handler for the load invalid value undefined behaviour is
implemented at EL2. The EL2 handler's parameters are stored inside the buffer.
They are used by the symetric handler from EL1.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 5 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 +++++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 6 +++++-
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 0eef0e11a93b..95ac6728ffd1 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -20,12 +20,14 @@ struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
- UBSAN_SHIFT_OUT_OF_BOUNDS
+ UBSAN_SHIFT_OUT_OF_BOUNDS,
+ UBSAN_INVALID_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
struct shift_out_of_bounds_data shift_out_of_bounds_data;
+ struct invalid_value_data invalid_value_data;
};
union {
struct ubsan_values u_val;
@@ -35,3 +37,4 @@ struct kvm_ubsan_info {
void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
+void __ubsan_handle_load_invalid_value(void *_data, void *val);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 1069ed5036d5..3143f7722be2 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -82,4 +82,16 @@ void __ubsan_handle_builtin_unreachable(void *_data)
}
}
-void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
+void __ubsan_handle_load_invalid_value(void *_data, void *val)
+{
+ struct kvm_ubsan_info *slot;
+ struct invalid_value_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_INVALID_DATA;
+ slot->invalid_value_data = *data;
+ slot->u_val.lval = val;
+ }
+
+}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index b80045883047..5439f7a91636 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -31,7 +31,11 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_shift_out_of_bounds(&slot->shift_out_of_bounds_data,
slot->u_val.lval, slot->u_val.rval);
break;
- }
+ case UBSAN_INVALID_DATA:
+ __ubsan_handle_load_invalid_value(&slot->invalid_value_data,
+ slot->u_val.lval);
+ break;
+ }
}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 6/9] KVM: arm64: __ubsan_handle_load_invalid_value EL2 implementation.
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Suzuki K Poulose, Marc Zyngier,
open list, James Morse, linux-arm-kernel, George Popescu,
Julien Thierry
From: George Popescu <georgepope@google.com>
The handler for the load invalid value undefined behaviour is
implemented at EL2. The EL2 handler's parameters are stored inside the buffer.
They are used by the symetric handler from EL1.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 5 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 +++++++++++++-
arch/arm64/kvm/kvm_ubsan_buffer.c | 6 +++++-
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 0eef0e11a93b..95ac6728ffd1 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -20,12 +20,14 @@ struct kvm_ubsan_info {
enum {
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
- UBSAN_SHIFT_OUT_OF_BOUNDS
+ UBSAN_SHIFT_OUT_OF_BOUNDS,
+ UBSAN_INVALID_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
struct shift_out_of_bounds_data shift_out_of_bounds_data;
+ struct invalid_value_data invalid_value_data;
};
union {
struct ubsan_values u_val;
@@ -35,3 +37,4 @@ struct kvm_ubsan_info {
void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
+void __ubsan_handle_load_invalid_value(void *_data, void *val);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 1069ed5036d5..3143f7722be2 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -82,4 +82,16 @@ void __ubsan_handle_builtin_unreachable(void *_data)
}
}
-void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
+void __ubsan_handle_load_invalid_value(void *_data, void *val)
+{
+ struct kvm_ubsan_info *slot;
+ struct invalid_value_data *data = _data;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_INVALID_DATA;
+ slot->invalid_value_data = *data;
+ slot->u_val.lval = val;
+ }
+
+}
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index b80045883047..5439f7a91636 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -31,7 +31,11 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_shift_out_of_bounds(&slot->shift_out_of_bounds_data,
slot->u_val.lval, slot->u_val.rval);
break;
- }
+ case UBSAN_INVALID_DATA:
+ __ubsan_handle_load_invalid_value(&slot->invalid_value_data,
+ slot->u_val.lval);
+ break;
+ }
}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 7/9] KVM: arm64: Detect type mismatch undefined behaviour from hyp/nVHE code
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George Popescu
From: George Popescu <georgepope@google.com>
Type mismatch undefiend behaviour handler provides two handlers with two
data structures type_mismatch_data and type_mismatch_data_v1. Both can be
stored inside a common data structure: type_mismatch_data_common, which
differs of type_mismatch_data only by keeping a pointer to a
struct source_location.
In this way, the buffer keeps the data encapsulated inside of a struct
type_mismatch_data, because pointers from nVHE can not be passed to the
kernel.
Inside the kernel call the __ubsan_handle_type_mismatch_data with the
data from the buffer.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 6 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 41 ++++++++++++++++++++++++++++--
arch/arm64/kvm/kvm_ubsan_buffer.c | 5 +++-
3 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 95ac6728ffd1..93c1b695097a 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -21,13 +21,15 @@ struct kvm_ubsan_info {
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
- UBSAN_INVALID_DATA
+ UBSAN_INVALID_DATA,
+ UBSAN_TYPE_MISMATCH
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
struct shift_out_of_bounds_data shift_out_of_bounds_data;
struct invalid_value_data invalid_value_data;
+ struct type_mismatch_data type_mismatch_data;
};
union {
struct ubsan_values u_val;
@@ -38,3 +40,5 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
void __ubsan_handle_load_invalid_value(void *_data, void *val);
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *_data, void *ptr);
+
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 3143f7722be2..a9f72d4bcab7 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -29,6 +29,24 @@ static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
return res;
}
+static void write_type_mismatch_data(struct type_mismatch_data_common *data, void *lval)
+{
+ struct kvm_ubsan_info *slot;
+ struct type_mismatch_data *aux_cont;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_TYPE_MISMATCH;
+ aux_cont = &slot->type_mismatch_data;
+ aux_cont->location.file_name = data->location->file_name;
+ aux_cont->location.reported = data->location->reported;
+ aux_cont->type = data->type;
+ aux_cont->alignment = data->alignment;
+ aux_cont->type_check_kind = data->type_check_kind;
+ slot->u_val.lval = lval;
+ }
+}
+
void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
@@ -39,9 +57,28 @@ void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
-void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr)
+{
+ struct type_mismatch_data_common common_data = {
+ .location = &data->location,
+ .type = data->type,
+ .alignment = data->alignment,
+ .type_check_kind = data->type_check_kind
+ };
+ write_type_mismatch_data(&common_data, ptr);
+}
-void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
+void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr)
+{
+ struct type_mismatch_data_v1 *data = _data;
+ struct type_mismatch_data_common common_data = {
+ .location = &data->location,
+ .type = data->type,
+ .alignment = 1UL << data->log_alignment,
+ .type_check_kind = data->type_check_kind
+ };
+ write_type_mismatch_data(&common_data, ptr);
+}
void __ubsan_handle_out_of_bounds(void *_data, void *index)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 5439f7a91636..b7823dedf8b1 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -35,6 +35,10 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_load_invalid_value(&slot->invalid_value_data,
slot->u_val.lval);
break;
+ case UBSAN_TYPE_MISMATCH:
+ __ubsan_handle_type_mismatch(&slot->type_mismatch_data,
+ slot->u_val.lval);
+ break;
}
}
@@ -58,4 +62,3 @@ void __kvm_check_ubsan_buffer(void)
write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
iterate_kvm_ubsan_buffer(0, *write_ind);
}
-
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 7/9] KVM: arm64: Detect type mismatch undefined behaviour from hyp/nVHE code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George Popescu <georgepope@google.com>
Type mismatch undefiend behaviour handler provides two handlers with two
data structures type_mismatch_data and type_mismatch_data_v1. Both can be
stored inside a common data structure: type_mismatch_data_common, which
differs of type_mismatch_data only by keeping a pointer to a
struct source_location.
In this way, the buffer keeps the data encapsulated inside of a struct
type_mismatch_data, because pointers from nVHE can not be passed to the
kernel.
Inside the kernel call the __ubsan_handle_type_mismatch_data with the
data from the buffer.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 6 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 41 ++++++++++++++++++++++++++++--
arch/arm64/kvm/kvm_ubsan_buffer.c | 5 +++-
3 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 95ac6728ffd1..93c1b695097a 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -21,13 +21,15 @@ struct kvm_ubsan_info {
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
- UBSAN_INVALID_DATA
+ UBSAN_INVALID_DATA,
+ UBSAN_TYPE_MISMATCH
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
struct shift_out_of_bounds_data shift_out_of_bounds_data;
struct invalid_value_data invalid_value_data;
+ struct type_mismatch_data type_mismatch_data;
};
union {
struct ubsan_values u_val;
@@ -38,3 +40,5 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
void __ubsan_handle_load_invalid_value(void *_data, void *val);
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *_data, void *ptr);
+
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 3143f7722be2..a9f72d4bcab7 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -29,6 +29,24 @@ static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
return res;
}
+static void write_type_mismatch_data(struct type_mismatch_data_common *data, void *lval)
+{
+ struct kvm_ubsan_info *slot;
+ struct type_mismatch_data *aux_cont;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_TYPE_MISMATCH;
+ aux_cont = &slot->type_mismatch_data;
+ aux_cont->location.file_name = data->location->file_name;
+ aux_cont->location.reported = data->location->reported;
+ aux_cont->type = data->type;
+ aux_cont->alignment = data->alignment;
+ aux_cont->type_check_kind = data->type_check_kind;
+ slot->u_val.lval = lval;
+ }
+}
+
void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
@@ -39,9 +57,28 @@ void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
-void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr)
+{
+ struct type_mismatch_data_common common_data = {
+ .location = &data->location,
+ .type = data->type,
+ .alignment = data->alignment,
+ .type_check_kind = data->type_check_kind
+ };
+ write_type_mismatch_data(&common_data, ptr);
+}
-void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
+void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr)
+{
+ struct type_mismatch_data_v1 *data = _data;
+ struct type_mismatch_data_common common_data = {
+ .location = &data->location,
+ .type = data->type,
+ .alignment = 1UL << data->log_alignment,
+ .type_check_kind = data->type_check_kind
+ };
+ write_type_mismatch_data(&common_data, ptr);
+}
void __ubsan_handle_out_of_bounds(void *_data, void *index)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 5439f7a91636..b7823dedf8b1 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -35,6 +35,10 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_load_invalid_value(&slot->invalid_value_data,
slot->u_val.lval);
break;
+ case UBSAN_TYPE_MISMATCH:
+ __ubsan_handle_type_mismatch(&slot->type_mismatch_data,
+ slot->u_val.lval);
+ break;
}
}
@@ -58,4 +62,3 @@ void __kvm_check_ubsan_buffer(void)
write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
iterate_kvm_ubsan_buffer(0, *write_ind);
}
-
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 7/9] KVM: arm64: Detect type mismatch undefined behaviour from hyp/nVHE code
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Suzuki K Poulose, Marc Zyngier,
open list, James Morse, linux-arm-kernel, George Popescu,
Julien Thierry
From: George Popescu <georgepope@google.com>
Type mismatch undefiend behaviour handler provides two handlers with two
data structures type_mismatch_data and type_mismatch_data_v1. Both can be
stored inside a common data structure: type_mismatch_data_common, which
differs of type_mismatch_data only by keeping a pointer to a
struct source_location.
In this way, the buffer keeps the data encapsulated inside of a struct
type_mismatch_data, because pointers from nVHE can not be passed to the
kernel.
Inside the kernel call the __ubsan_handle_type_mismatch_data with the
data from the buffer.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 6 ++++-
arch/arm64/kvm/hyp/nvhe/ubsan.c | 41 ++++++++++++++++++++++++++++--
arch/arm64/kvm/kvm_ubsan_buffer.c | 5 +++-
3 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 95ac6728ffd1..93c1b695097a 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -21,13 +21,15 @@ struct kvm_ubsan_info {
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
- UBSAN_INVALID_DATA
+ UBSAN_INVALID_DATA,
+ UBSAN_TYPE_MISMATCH
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
struct unreachable_data unreachable_data;
struct shift_out_of_bounds_data shift_out_of_bounds_data;
struct invalid_value_data invalid_value_data;
+ struct type_mismatch_data type_mismatch_data;
};
union {
struct ubsan_values u_val;
@@ -38,3 +40,5 @@ void __ubsan_handle_out_of_bounds(void *_data, void *index);
void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
void __ubsan_handle_load_invalid_value(void *_data, void *val);
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *_data, void *ptr);
+
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index 3143f7722be2..a9f72d4bcab7 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -29,6 +29,24 @@ static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
return res;
}
+static void write_type_mismatch_data(struct type_mismatch_data_common *data, void *lval)
+{
+ struct kvm_ubsan_info *slot;
+ struct type_mismatch_data *aux_cont;
+
+ slot = kvm_ubsan_buffer_next_slot();
+ if (slot) {
+ slot->type = UBSAN_TYPE_MISMATCH;
+ aux_cont = &slot->type_mismatch_data;
+ aux_cont->location.file_name = data->location->file_name;
+ aux_cont->location.reported = data->location->reported;
+ aux_cont->type = data->type;
+ aux_cont->alignment = data->alignment;
+ aux_cont->type_check_kind = data->type_check_kind;
+ slot->u_val.lval = lval;
+ }
+}
+
void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
@@ -39,9 +57,28 @@ void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
-void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr)
+{
+ struct type_mismatch_data_common common_data = {
+ .location = &data->location,
+ .type = data->type,
+ .alignment = data->alignment,
+ .type_check_kind = data->type_check_kind
+ };
+ write_type_mismatch_data(&common_data, ptr);
+}
-void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
+void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr)
+{
+ struct type_mismatch_data_v1 *data = _data;
+ struct type_mismatch_data_common common_data = {
+ .location = &data->location,
+ .type = data->type,
+ .alignment = 1UL << data->log_alignment,
+ .type_check_kind = data->type_check_kind
+ };
+ write_type_mismatch_data(&common_data, ptr);
+}
void __ubsan_handle_out_of_bounds(void *_data, void *index)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 5439f7a91636..b7823dedf8b1 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -35,6 +35,10 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_load_invalid_value(&slot->invalid_value_data,
slot->u_val.lval);
break;
+ case UBSAN_TYPE_MISMATCH:
+ __ubsan_handle_type_mismatch(&slot->type_mismatch_data,
+ slot->u_val.lval);
+ break;
}
}
@@ -58,4 +62,3 @@ void __kvm_check_ubsan_buffer(void)
write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
iterate_kvm_ubsan_buffer(0, *write_ind);
}
-
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 8/9] KVM: arm64: Detect arithmetic overflow is inside hyp/nVHE.
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George Popescu
From: George Popescu <georgepope@google.com>
Whenever an arithmetic overflow: addition, substraction, multiplication,
division or negating happens inside the hyp/nVHE code,
an __ubsan_handle_*_overflow is called.
All the overflow handlers are sharing the same structure called
overflow_data.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 10 ++++++--
arch/arm64/kvm/hyp/nvhe/ubsan.c | 40 ++++++++++++++++++++++++++----
arch/arm64/kvm/kvm_ubsan_buffer.c | 20 ++++++++++++++-
3 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 93c1b695097a..da4a3b4e28e0 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -22,7 +22,8 @@ struct kvm_ubsan_info {
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
UBSAN_INVALID_DATA,
- UBSAN_TYPE_MISMATCH
+ UBSAN_TYPE_MISMATCH,
+ UBSAN_OVERFLOW_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
@@ -30,6 +31,7 @@ struct kvm_ubsan_info {
struct shift_out_of_bounds_data shift_out_of_bounds_data;
struct invalid_value_data invalid_value_data;
struct type_mismatch_data type_mismatch_data;
+ struct overflow_data overflow_data;
};
union {
struct ubsan_values u_val;
@@ -41,4 +43,8 @@ void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
void __ubsan_handle_load_invalid_value(void *_data, void *val);
void __ubsan_handle_type_mismatch(struct type_mismatch_data *_data, void *ptr);
-
+void __ubsan_handle_add_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_sub_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_mul_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_negate_overflow(void *_data, void *old_val);
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index a9f72d4bcab7..f16842ff7316 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -47,15 +47,45 @@ static void write_type_mismatch_data(struct type_mismatch_data_common *data, voi
}
}
-void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
+static void write_overflow_data(struct overflow_data *data, void *lval, void *rval, char op)
+{
+ struct kvm_ubsan_info *slot = kvm_ubsan_buffer_next_slot();
+
+ if (slot) {
+ slot->type = UBSAN_OVERFLOW_DATA;
+ slot->overflow_data = *data;
+ slot->u_val.op = op;
+ slot->u_val.lval = lval;
+ if (op != '!')
+ slot->u_val.rval = rval;
+ }
+}
+
+void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '+');
+}
-void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '-');
+}
-void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '*');
+}
-void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
+void __ubsan_handle_negate_overflow(void *_data, void *old_val)
+{
+ write_overflow_data(_data, old_val, NULL, '!');
+}
+
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '/');
+}
-void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index b7823dedf8b1..2c7060cbb48b 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -39,7 +39,25 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_type_mismatch(&slot->type_mismatch_data,
slot->u_val.lval);
break;
- }
+ case UBSAN_OVERFLOW_DATA:
+ if (slot->u_val.op == '/') {
+ __ubsan_handle_divrem_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '!') {
+ __ubsan_handle_negate_overflow(&slot->overflow_data,
+ slot->u_val.lval);
+ } else if (slot->u_val.op == '+') {
+ __ubsan_handle_add_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '-') {
+ __ubsan_handle_sub_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '*') {
+ __ubsan_handle_mul_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ }
+ break;
+ }
}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 8/9] KVM: arm64: Detect arithmetic overflow is inside hyp/nVHE.
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George Popescu <georgepope@google.com>
Whenever an arithmetic overflow: addition, substraction, multiplication,
division or negating happens inside the hyp/nVHE code,
an __ubsan_handle_*_overflow is called.
All the overflow handlers are sharing the same structure called
overflow_data.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 10 ++++++--
arch/arm64/kvm/hyp/nvhe/ubsan.c | 40 ++++++++++++++++++++++++++----
arch/arm64/kvm/kvm_ubsan_buffer.c | 20 ++++++++++++++-
3 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 93c1b695097a..da4a3b4e28e0 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -22,7 +22,8 @@ struct kvm_ubsan_info {
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
UBSAN_INVALID_DATA,
- UBSAN_TYPE_MISMATCH
+ UBSAN_TYPE_MISMATCH,
+ UBSAN_OVERFLOW_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
@@ -30,6 +31,7 @@ struct kvm_ubsan_info {
struct shift_out_of_bounds_data shift_out_of_bounds_data;
struct invalid_value_data invalid_value_data;
struct type_mismatch_data type_mismatch_data;
+ struct overflow_data overflow_data;
};
union {
struct ubsan_values u_val;
@@ -41,4 +43,8 @@ void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
void __ubsan_handle_load_invalid_value(void *_data, void *val);
void __ubsan_handle_type_mismatch(struct type_mismatch_data *_data, void *ptr);
-
+void __ubsan_handle_add_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_sub_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_mul_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_negate_overflow(void *_data, void *old_val);
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index a9f72d4bcab7..f16842ff7316 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -47,15 +47,45 @@ static void write_type_mismatch_data(struct type_mismatch_data_common *data, voi
}
}
-void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
+static void write_overflow_data(struct overflow_data *data, void *lval, void *rval, char op)
+{
+ struct kvm_ubsan_info *slot = kvm_ubsan_buffer_next_slot();
+
+ if (slot) {
+ slot->type = UBSAN_OVERFLOW_DATA;
+ slot->overflow_data = *data;
+ slot->u_val.op = op;
+ slot->u_val.lval = lval;
+ if (op != '!')
+ slot->u_val.rval = rval;
+ }
+}
+
+void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '+');
+}
-void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '-');
+}
-void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '*');
+}
-void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
+void __ubsan_handle_negate_overflow(void *_data, void *old_val)
+{
+ write_overflow_data(_data, old_val, NULL, '!');
+}
+
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '/');
+}
-void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index b7823dedf8b1..2c7060cbb48b 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -39,7 +39,25 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_type_mismatch(&slot->type_mismatch_data,
slot->u_val.lval);
break;
- }
+ case UBSAN_OVERFLOW_DATA:
+ if (slot->u_val.op == '/') {
+ __ubsan_handle_divrem_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '!') {
+ __ubsan_handle_negate_overflow(&slot->overflow_data,
+ slot->u_val.lval);
+ } else if (slot->u_val.op == '+') {
+ __ubsan_handle_add_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '-') {
+ __ubsan_handle_sub_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '*') {
+ __ubsan_handle_mul_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ }
+ break;
+ }
}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 8/9] KVM: arm64: Detect arithmetic overflow is inside hyp/nVHE.
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George Popescu, Elena Petrova, Suzuki K Poulose, Marc Zyngier,
open list, James Morse, linux-arm-kernel, George Popescu,
Julien Thierry
From: George Popescu <georgepope@google.com>
Whenever an arithmetic overflow: addition, substraction, multiplication,
division or negating happens inside the hyp/nVHE code,
an __ubsan_handle_*_overflow is called.
All the overflow handlers are sharing the same structure called
overflow_data.
Signed-off-by: George Popescu <georgepope@google.com>
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/kvm_ubsan.h | 10 ++++++--
arch/arm64/kvm/hyp/nvhe/ubsan.c | 40 ++++++++++++++++++++++++++----
arch/arm64/kvm/kvm_ubsan_buffer.c | 20 ++++++++++++++-
3 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index 93c1b695097a..da4a3b4e28e0 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -22,7 +22,8 @@ struct kvm_ubsan_info {
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
UBSAN_INVALID_DATA,
- UBSAN_TYPE_MISMATCH
+ UBSAN_TYPE_MISMATCH,
+ UBSAN_OVERFLOW_DATA
} type;
union {
struct out_of_bounds_data out_of_bounds_data;
@@ -30,6 +31,7 @@ struct kvm_ubsan_info {
struct shift_out_of_bounds_data shift_out_of_bounds_data;
struct invalid_value_data invalid_value_data;
struct type_mismatch_data type_mismatch_data;
+ struct overflow_data overflow_data;
};
union {
struct ubsan_values u_val;
@@ -41,4 +43,8 @@ void __ubsan_handle_builtin_unreachable(void *_data);
void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
void __ubsan_handle_load_invalid_value(void *_data, void *val);
void __ubsan_handle_type_mismatch(struct type_mismatch_data *_data, void *ptr);
-
+void __ubsan_handle_add_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_sub_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_mul_overflow(void *data, void *lhs, void *rhs);
+void __ubsan_handle_negate_overflow(void *_data, void *old_val);
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs);
diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
index a9f72d4bcab7..f16842ff7316 100644
--- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
+++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
@@ -47,15 +47,45 @@ static void write_type_mismatch_data(struct type_mismatch_data_common *data, voi
}
}
-void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
+static void write_overflow_data(struct overflow_data *data, void *lval, void *rval, char op)
+{
+ struct kvm_ubsan_info *slot = kvm_ubsan_buffer_next_slot();
+
+ if (slot) {
+ slot->type = UBSAN_OVERFLOW_DATA;
+ slot->overflow_data = *data;
+ slot->u_val.op = op;
+ slot->u_val.lval = lval;
+ if (op != '!')
+ slot->u_val.rval = rval;
+ }
+}
+
+void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '+');
+}
-void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '-');
+}
-void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
+void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '*');
+}
-void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
+void __ubsan_handle_negate_overflow(void *_data, void *old_val)
+{
+ write_overflow_data(_data, old_val, NULL, '!');
+}
+
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs)
+{
+ write_overflow_data(_data, lhs, rhs, '/');
+}
-void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr)
{
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index b7823dedf8b1..2c7060cbb48b 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -39,7 +39,25 @@ void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
__ubsan_handle_type_mismatch(&slot->type_mismatch_data,
slot->u_val.lval);
break;
- }
+ case UBSAN_OVERFLOW_DATA:
+ if (slot->u_val.op == '/') {
+ __ubsan_handle_divrem_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '!') {
+ __ubsan_handle_negate_overflow(&slot->overflow_data,
+ slot->u_val.lval);
+ } else if (slot->u_val.op == '+') {
+ __ubsan_handle_add_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '-') {
+ __ubsan_handle_sub_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ } else if (slot->u_val.op == '*') {
+ __ubsan_handle_mul_overflow(&slot->overflow_data,
+ slot->u_val.lval, slot->u_val.rval);
+ }
+ break;
+ }
}
void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 9/9] KVM: arm64: Add UBSan tests for PKVM.
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-15 17:18 ` Elena Petrova
-1 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: Elena Petrova, linux-arm-kernel, open list, Marc Zyngier,
James Morse, Julien Thierry, Suzuki K Poulose, George Popescu,
George-Aurelian Popescu
From: George-Aurelian Popescu <georgepope@google.com>
Test the UBsan functionality inside hyp/nVHE.
Because modules are not supported inside of hyp/nVHE code, the default
testing module for UBSan can not be used.
New functions have to be defined inside of hyp/nVHE.
They are called in kvm_get_mdcr_el2, to test UBSAN whenever a VM starts.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/assembler.h | 17 ++-
arch/arm64/include/asm/kvm_debug_buffer.h | 10 +-
arch/arm64/include/asm/kvm_ubsan.h | 2 +-
arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 ++++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
arch/arm64/kvm/kvm_ubsan_buffer.c | 1 -
6 files changed, 128 insertions(+), 17 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index ebc18a8a0e1f..8422b0d925e8 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -259,16 +259,15 @@ alternative_endif
.endm
/*
- * @sym: The name of the per-cpu variable
- * @reg: value to store
- * @tmp1: scratch register
- * @tmp2: scratch register
- */
- .macro str_this_cpu sym, reg, tmp1, tmp2
- adr_this_cpu \tmp1, \sym, \tmp2
+ * @sym: The name of the per-cpu variable
+ * @reg: value to store
+ * @tmp1: scratch register
+ * @tmp2: scratch register
+ */
+ .macro str_this_cpu sym, reg, tmp1, tmp2
+ adr_this_cpu \tmp1, \sym, \tmp2
str \reg, [\tmp1]
- .endm
-
+ .endm
/*
* vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
*/
diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
index e5375c2cff1a..361b473bb004 100644
--- a/arch/arm64/include/asm/kvm_debug_buffer.h
+++ b/arch/arm64/include/asm/kvm_debug_buffer.h
@@ -3,10 +3,8 @@
* Copyright 2020 Google LLC
* Author: George Popescu <georgepope@google.com>
*/
-
#include <linux/percpu-defs.h>
-
#define KVM_DEBUG_BUFFER_SIZE 1000
#ifdef __KVM_NVHE_HYPERVISOR__
@@ -20,17 +18,17 @@
#else
#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
- DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
+ DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
#endif //__KVM_NVHE_HYPERVISOR__
#ifdef __ASSEMBLY__
#include <asm/assembler.h>
.macro clear_buffer tmp1, tmp2, tmp3
- mov \tmp1, 0
+ mov \tmp1, 0
#ifdef CONFIG_UBSAN
- str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
+ str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
#endif //CONFIG_UBSAN
.endm
-#endif
\ No newline at end of file
+#endif
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index da4a3b4e28e0..0b8bed08d48e 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -9,7 +9,6 @@
#define UBSAN_MAX_TYPE 6
#define KVM_UBSAN_BUFFER_SIZE 1000
-
struct ubsan_values {
void *lval;
void *rval;
@@ -18,6 +17,7 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
+ UBSAN_NONE,
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
diff --git a/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
new file mode 100644
index 000000000000..07759c0d1e0e
--- /dev/null
+++ b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <linux/ctype.h>
+
+typedef void(*test_ubsan_fp)(void);
+
+static void test_ubsan_add_overflow(void)
+{
+ volatile int val = INT_MAX;
+
+ val += 2;
+}
+
+static void test_ubsan_sub_overflow(void)
+{
+ volatile int val = INT_MIN;
+ volatile int val2 = 2;
+
+ val -= val2;
+}
+
+static void test_ubsan_mul_overflow(void)
+{
+ volatile int val = INT_MAX / 2;
+
+ val *= 3;
+}
+
+static void test_ubsan_negate_overflow(void)
+{
+ volatile int val = INT_MIN;
+
+ val = -val;
+}
+
+static void test_ubsan_divrem_overflow(void)
+{
+ volatile int val = 16;
+ volatile int val2 = 0;
+
+ val /= val2;
+}
+
+static void test_ubsan_shift_out_of_bounds(void)
+{
+ volatile int val = -1;
+ int val2 = 10;
+
+ val2 <<= val;
+}
+
+static void test_ubsan_out_of_bounds(void)
+{
+ volatile int i = 4, j = 5;
+ volatile int arr[4];
+
+ arr[j] = i;
+}
+
+static void test_ubsan_load_invalid_value(void)
+{
+ volatile char *dst, *src;
+ bool val, val2, *ptr;
+ char c = 4;
+
+ dst = (char *)&val;
+ src = &c;
+ *dst = *src;
+
+ ptr = &val2;
+ val2 = val;
+}
+
+static void test_ubsan_misaligned_access(void)
+{
+ volatile char arr[5] __aligned(4) = {1, 2, 3, 4, 5};
+ volatile int *ptr, val = 6;
+
+ ptr = (int *)(arr + 1);
+ *ptr = val;
+}
+
+static void test_ubsan_object_size_mismatch(void)
+{
+ /* "((aligned(8)))" helps this not into be misaligned for ptr-access. */
+ volatile int val __aligned(8) = 4;
+ volatile long long *ptr, val2;
+
+ ptr = (long long *)&val;
+ val2 = *ptr;
+}
+
+static const test_ubsan_fp test_ubsan_array[] = {
+ test_ubsan_out_of_bounds,
+ test_ubsan_add_overflow,
+ test_ubsan_sub_overflow,
+ test_ubsan_mul_overflow,
+ test_ubsan_negate_overflow,
+ test_ubsan_divrem_overflow,
+ test_ubsan_shift_out_of_bounds,
+ test_ubsan_load_invalid_value,
+ test_ubsan_misaligned_access,
+ test_ubsan_object_size_mismatch,
+};
+
+static void test_ubsan(void)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(test_ubsan_array); i++)
+ test_ubsan_array[i]();
+}
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index a906f9e2ff34..939600e9fdd6 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -13,6 +13,7 @@
#include <asm/kvm_mmu.h>
#include <nvhe/trap_handler.h>
+#include <hyp/test_ubsan.h>
DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
@@ -90,6 +91,8 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
static void handle___kvm_get_mdcr_el2(struct kvm_cpu_context *host_ctxt)
{
cpu_reg(host_ctxt, 1) = __kvm_get_mdcr_el2();
+ if (IS_ENABLED(CONFIG_TEST_UBSAN))
+ test_ubsan();
}
static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 2c7060cbb48b..49bedc9de139 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -11,7 +11,6 @@
#include <asm/kvm_asm.h>
#include <kvm/arm_pmu.h>
-#include <ubsan.h>
#include <asm/kvm_ubsan.h>
DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
--
2.30.0.296.g2bfb1c46d8-goog
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 9/9] KVM: arm64: Add UBSan tests for PKVM.
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George-Aurelian Popescu, Elena Petrova, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
From: George-Aurelian Popescu <georgepope@google.com>
Test the UBsan functionality inside hyp/nVHE.
Because modules are not supported inside of hyp/nVHE code, the default
testing module for UBSan can not be used.
New functions have to be defined inside of hyp/nVHE.
They are called in kvm_get_mdcr_el2, to test UBSAN whenever a VM starts.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/assembler.h | 17 ++-
arch/arm64/include/asm/kvm_debug_buffer.h | 10 +-
arch/arm64/include/asm/kvm_ubsan.h | 2 +-
arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 ++++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
arch/arm64/kvm/kvm_ubsan_buffer.c | 1 -
6 files changed, 128 insertions(+), 17 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index ebc18a8a0e1f..8422b0d925e8 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -259,16 +259,15 @@ alternative_endif
.endm
/*
- * @sym: The name of the per-cpu variable
- * @reg: value to store
- * @tmp1: scratch register
- * @tmp2: scratch register
- */
- .macro str_this_cpu sym, reg, tmp1, tmp2
- adr_this_cpu \tmp1, \sym, \tmp2
+ * @sym: The name of the per-cpu variable
+ * @reg: value to store
+ * @tmp1: scratch register
+ * @tmp2: scratch register
+ */
+ .macro str_this_cpu sym, reg, tmp1, tmp2
+ adr_this_cpu \tmp1, \sym, \tmp2
str \reg, [\tmp1]
- .endm
-
+ .endm
/*
* vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
*/
diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
index e5375c2cff1a..361b473bb004 100644
--- a/arch/arm64/include/asm/kvm_debug_buffer.h
+++ b/arch/arm64/include/asm/kvm_debug_buffer.h
@@ -3,10 +3,8 @@
* Copyright 2020 Google LLC
* Author: George Popescu <georgepope@google.com>
*/
-
#include <linux/percpu-defs.h>
-
#define KVM_DEBUG_BUFFER_SIZE 1000
#ifdef __KVM_NVHE_HYPERVISOR__
@@ -20,17 +18,17 @@
#else
#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
- DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
+ DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
#endif //__KVM_NVHE_HYPERVISOR__
#ifdef __ASSEMBLY__
#include <asm/assembler.h>
.macro clear_buffer tmp1, tmp2, tmp3
- mov \tmp1, 0
+ mov \tmp1, 0
#ifdef CONFIG_UBSAN
- str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
+ str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
#endif //CONFIG_UBSAN
.endm
-#endif
\ No newline at end of file
+#endif
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index da4a3b4e28e0..0b8bed08d48e 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -9,7 +9,6 @@
#define UBSAN_MAX_TYPE 6
#define KVM_UBSAN_BUFFER_SIZE 1000
-
struct ubsan_values {
void *lval;
void *rval;
@@ -18,6 +17,7 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
+ UBSAN_NONE,
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
diff --git a/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
new file mode 100644
index 000000000000..07759c0d1e0e
--- /dev/null
+++ b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <linux/ctype.h>
+
+typedef void(*test_ubsan_fp)(void);
+
+static void test_ubsan_add_overflow(void)
+{
+ volatile int val = INT_MAX;
+
+ val += 2;
+}
+
+static void test_ubsan_sub_overflow(void)
+{
+ volatile int val = INT_MIN;
+ volatile int val2 = 2;
+
+ val -= val2;
+}
+
+static void test_ubsan_mul_overflow(void)
+{
+ volatile int val = INT_MAX / 2;
+
+ val *= 3;
+}
+
+static void test_ubsan_negate_overflow(void)
+{
+ volatile int val = INT_MIN;
+
+ val = -val;
+}
+
+static void test_ubsan_divrem_overflow(void)
+{
+ volatile int val = 16;
+ volatile int val2 = 0;
+
+ val /= val2;
+}
+
+static void test_ubsan_shift_out_of_bounds(void)
+{
+ volatile int val = -1;
+ int val2 = 10;
+
+ val2 <<= val;
+}
+
+static void test_ubsan_out_of_bounds(void)
+{
+ volatile int i = 4, j = 5;
+ volatile int arr[4];
+
+ arr[j] = i;
+}
+
+static void test_ubsan_load_invalid_value(void)
+{
+ volatile char *dst, *src;
+ bool val, val2, *ptr;
+ char c = 4;
+
+ dst = (char *)&val;
+ src = &c;
+ *dst = *src;
+
+ ptr = &val2;
+ val2 = val;
+}
+
+static void test_ubsan_misaligned_access(void)
+{
+ volatile char arr[5] __aligned(4) = {1, 2, 3, 4, 5};
+ volatile int *ptr, val = 6;
+
+ ptr = (int *)(arr + 1);
+ *ptr = val;
+}
+
+static void test_ubsan_object_size_mismatch(void)
+{
+ /* "((aligned(8)))" helps this not into be misaligned for ptr-access. */
+ volatile int val __aligned(8) = 4;
+ volatile long long *ptr, val2;
+
+ ptr = (long long *)&val;
+ val2 = *ptr;
+}
+
+static const test_ubsan_fp test_ubsan_array[] = {
+ test_ubsan_out_of_bounds,
+ test_ubsan_add_overflow,
+ test_ubsan_sub_overflow,
+ test_ubsan_mul_overflow,
+ test_ubsan_negate_overflow,
+ test_ubsan_divrem_overflow,
+ test_ubsan_shift_out_of_bounds,
+ test_ubsan_load_invalid_value,
+ test_ubsan_misaligned_access,
+ test_ubsan_object_size_mismatch,
+};
+
+static void test_ubsan(void)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(test_ubsan_array); i++)
+ test_ubsan_array[i]();
+}
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index a906f9e2ff34..939600e9fdd6 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -13,6 +13,7 @@
#include <asm/kvm_mmu.h>
#include <nvhe/trap_handler.h>
+#include <hyp/test_ubsan.h>
DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
@@ -90,6 +91,8 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
static void handle___kvm_get_mdcr_el2(struct kvm_cpu_context *host_ctxt)
{
cpu_reg(host_ctxt, 1) = __kvm_get_mdcr_el2();
+ if (IS_ENABLED(CONFIG_TEST_UBSAN))
+ test_ubsan();
}
static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 2c7060cbb48b..49bedc9de139 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -11,7 +11,6 @@
#include <asm/kvm_asm.h>
#include <kvm/arm_pmu.h>
-#include <ubsan.h>
#include <asm/kvm_ubsan.h>
DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 9/9] KVM: arm64: Add UBSan tests for PKVM.
@ 2021-01-15 17:18 ` Elena Petrova
0 siblings, 0 replies; 42+ messages in thread
From: Elena Petrova @ 2021-01-15 17:18 UTC (permalink / raw)
To: kvmarm
Cc: George-Aurelian Popescu, Elena Petrova, Suzuki K Poulose,
Marc Zyngier, open list, James Morse, linux-arm-kernel,
George Popescu, Julien Thierry
From: George-Aurelian Popescu <georgepope@google.com>
Test the UBsan functionality inside hyp/nVHE.
Because modules are not supported inside of hyp/nVHE code, the default
testing module for UBSan can not be used.
New functions have to be defined inside of hyp/nVHE.
They are called in kvm_get_mdcr_el2, to test UBSAN whenever a VM starts.
Signed-off-by: Elena Petrova <lenaptr@google.com>
---
arch/arm64/include/asm/assembler.h | 17 ++-
arch/arm64/include/asm/kvm_debug_buffer.h | 10 +-
arch/arm64/include/asm/kvm_ubsan.h | 2 +-
arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 ++++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
arch/arm64/kvm/kvm_ubsan_buffer.c | 1 -
6 files changed, 128 insertions(+), 17 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index ebc18a8a0e1f..8422b0d925e8 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -259,16 +259,15 @@ alternative_endif
.endm
/*
- * @sym: The name of the per-cpu variable
- * @reg: value to store
- * @tmp1: scratch register
- * @tmp2: scratch register
- */
- .macro str_this_cpu sym, reg, tmp1, tmp2
- adr_this_cpu \tmp1, \sym, \tmp2
+ * @sym: The name of the per-cpu variable
+ * @reg: value to store
+ * @tmp1: scratch register
+ * @tmp2: scratch register
+ */
+ .macro str_this_cpu sym, reg, tmp1, tmp2
+ adr_this_cpu \tmp1, \sym, \tmp2
str \reg, [\tmp1]
- .endm
-
+ .endm
/*
* vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
*/
diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
index e5375c2cff1a..361b473bb004 100644
--- a/arch/arm64/include/asm/kvm_debug_buffer.h
+++ b/arch/arm64/include/asm/kvm_debug_buffer.h
@@ -3,10 +3,8 @@
* Copyright 2020 Google LLC
* Author: George Popescu <georgepope@google.com>
*/
-
#include <linux/percpu-defs.h>
-
#define KVM_DEBUG_BUFFER_SIZE 1000
#ifdef __KVM_NVHE_HYPERVISOR__
@@ -20,17 +18,17 @@
#else
#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
- DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
+ DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
#endif //__KVM_NVHE_HYPERVISOR__
#ifdef __ASSEMBLY__
#include <asm/assembler.h>
.macro clear_buffer tmp1, tmp2, tmp3
- mov \tmp1, 0
+ mov \tmp1, 0
#ifdef CONFIG_UBSAN
- str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
+ str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
#endif //CONFIG_UBSAN
.endm
-#endif
\ No newline at end of file
+#endif
diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
index da4a3b4e28e0..0b8bed08d48e 100644
--- a/arch/arm64/include/asm/kvm_ubsan.h
+++ b/arch/arm64/include/asm/kvm_ubsan.h
@@ -9,7 +9,6 @@
#define UBSAN_MAX_TYPE 6
#define KVM_UBSAN_BUFFER_SIZE 1000
-
struct ubsan_values {
void *lval;
void *rval;
@@ -18,6 +17,7 @@ struct ubsan_values {
struct kvm_ubsan_info {
enum {
+ UBSAN_NONE,
UBSAN_OUT_OF_BOUNDS,
UBSAN_UNREACHABLE_DATA,
UBSAN_SHIFT_OUT_OF_BOUNDS,
diff --git a/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
new file mode 100644
index 000000000000..07759c0d1e0e
--- /dev/null
+++ b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <linux/ctype.h>
+
+typedef void(*test_ubsan_fp)(void);
+
+static void test_ubsan_add_overflow(void)
+{
+ volatile int val = INT_MAX;
+
+ val += 2;
+}
+
+static void test_ubsan_sub_overflow(void)
+{
+ volatile int val = INT_MIN;
+ volatile int val2 = 2;
+
+ val -= val2;
+}
+
+static void test_ubsan_mul_overflow(void)
+{
+ volatile int val = INT_MAX / 2;
+
+ val *= 3;
+}
+
+static void test_ubsan_negate_overflow(void)
+{
+ volatile int val = INT_MIN;
+
+ val = -val;
+}
+
+static void test_ubsan_divrem_overflow(void)
+{
+ volatile int val = 16;
+ volatile int val2 = 0;
+
+ val /= val2;
+}
+
+static void test_ubsan_shift_out_of_bounds(void)
+{
+ volatile int val = -1;
+ int val2 = 10;
+
+ val2 <<= val;
+}
+
+static void test_ubsan_out_of_bounds(void)
+{
+ volatile int i = 4, j = 5;
+ volatile int arr[4];
+
+ arr[j] = i;
+}
+
+static void test_ubsan_load_invalid_value(void)
+{
+ volatile char *dst, *src;
+ bool val, val2, *ptr;
+ char c = 4;
+
+ dst = (char *)&val;
+ src = &c;
+ *dst = *src;
+
+ ptr = &val2;
+ val2 = val;
+}
+
+static void test_ubsan_misaligned_access(void)
+{
+ volatile char arr[5] __aligned(4) = {1, 2, 3, 4, 5};
+ volatile int *ptr, val = 6;
+
+ ptr = (int *)(arr + 1);
+ *ptr = val;
+}
+
+static void test_ubsan_object_size_mismatch(void)
+{
+ /* "((aligned(8)))" helps this not into be misaligned for ptr-access. */
+ volatile int val __aligned(8) = 4;
+ volatile long long *ptr, val2;
+
+ ptr = (long long *)&val;
+ val2 = *ptr;
+}
+
+static const test_ubsan_fp test_ubsan_array[] = {
+ test_ubsan_out_of_bounds,
+ test_ubsan_add_overflow,
+ test_ubsan_sub_overflow,
+ test_ubsan_mul_overflow,
+ test_ubsan_negate_overflow,
+ test_ubsan_divrem_overflow,
+ test_ubsan_shift_out_of_bounds,
+ test_ubsan_load_invalid_value,
+ test_ubsan_misaligned_access,
+ test_ubsan_object_size_mismatch,
+};
+
+static void test_ubsan(void)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(test_ubsan_array); i++)
+ test_ubsan_array[i]();
+}
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index a906f9e2ff34..939600e9fdd6 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -13,6 +13,7 @@
#include <asm/kvm_mmu.h>
#include <nvhe/trap_handler.h>
+#include <hyp/test_ubsan.h>
DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
@@ -90,6 +91,8 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
static void handle___kvm_get_mdcr_el2(struct kvm_cpu_context *host_ctxt)
{
cpu_reg(host_ctxt, 1) = __kvm_get_mdcr_el2();
+ if (IS_ENABLED(CONFIG_TEST_UBSAN))
+ test_ubsan();
}
static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
index 2c7060cbb48b..49bedc9de139 100644
--- a/arch/arm64/kvm/kvm_ubsan_buffer.c
+++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
@@ -11,7 +11,6 @@
#include <asm/kvm_asm.h>
#include <kvm/arm_pmu.h>
-#include <ubsan.h>
#include <asm/kvm_ubsan.h>
DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
--
2.30.0.296.g2bfb1c46d8-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/9] KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-18 9:53 ` David Brazdil
-1 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 9:53 UTC (permalink / raw)
To: Elena Petrova
Cc: kvmarm, George-Aurelian Popescu, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
On Fri, Jan 15, 2021 at 05:18:22PM +0000, Elena Petrova wrote:
> From: George-Aurelian Popescu <georgepope@google.com>
>
> Implement UBSan handlers inside nVHe hyp code, as empty functions for the
> moment, so the undefined behaviours, that are triggered there, will be
> linked to them, not to the ones defined in kernel-proper lib/ubsan.c.
>
> In this way, enabling UBSAN_MISC won't cause a link error.
The commit message needs to be updated - UBSAN_MISC does not exist any longer.
Also "nVHe" is a typo.
Other than that:
Acked-by: David Brazdil <dbrazdil@google.com>
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++-
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index 1f1e351c5fe2..2a683e7c6c5b 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -10,6 +10,8 @@ obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
> hyp-main.o hyp-smp.o psci-relay.o
> obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
> ../fpsimd.o ../hyp-entry.o ../exception.o
> +obj-$(CONFIG_UBSAN) += ubsan.o
> +CFLAGS_ubsan.nvhe.o += -I $(srctree)/lib/
>
> ##
> ## Build rules for compiling nVHE hyp code
> @@ -61,7 +63,6 @@ KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAG
> # cause crashes. Just disable it.
> GCOV_PROFILE := n
> KASAN_SANITIZE := n
> -UBSAN_SANITIZE := n
> KCOV_INSTRUMENT := n
>
> # Skip objtool checking for this directory because nVHE code is compiled with
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> new file mode 100644
> index 000000000000..a5db6b61ceb2
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +#include <linux/ctype.h>
> +#include <linux/types.h>
> +#include <ubsan.h>
> +
> +void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
> +
> +void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
> +
> +void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
> +
> +void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
> +
> +void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_builtin_unreachable(void *_data) {}
> +
> +void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/9] KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
@ 2021-01-18 9:53 ` David Brazdil
0 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 9:53 UTC (permalink / raw)
To: Elena Petrova
Cc: George-Aurelian Popescu, Marc Zyngier, open list, kvmarm,
George Popescu, linux-arm-kernel
On Fri, Jan 15, 2021 at 05:18:22PM +0000, Elena Petrova wrote:
> From: George-Aurelian Popescu <georgepope@google.com>
>
> Implement UBSan handlers inside nVHe hyp code, as empty functions for the
> moment, so the undefined behaviours, that are triggered there, will be
> linked to them, not to the ones defined in kernel-proper lib/ubsan.c.
>
> In this way, enabling UBSAN_MISC won't cause a link error.
The commit message needs to be updated - UBSAN_MISC does not exist any longer.
Also "nVHe" is a typo.
Other than that:
Acked-by: David Brazdil <dbrazdil@google.com>
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++-
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index 1f1e351c5fe2..2a683e7c6c5b 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -10,6 +10,8 @@ obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
> hyp-main.o hyp-smp.o psci-relay.o
> obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
> ../fpsimd.o ../hyp-entry.o ../exception.o
> +obj-$(CONFIG_UBSAN) += ubsan.o
> +CFLAGS_ubsan.nvhe.o += -I $(srctree)/lib/
>
> ##
> ## Build rules for compiling nVHE hyp code
> @@ -61,7 +63,6 @@ KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAG
> # cause crashes. Just disable it.
> GCOV_PROFILE := n
> KASAN_SANITIZE := n
> -UBSAN_SANITIZE := n
> KCOV_INSTRUMENT := n
>
> # Skip objtool checking for this directory because nVHE code is compiled with
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> new file mode 100644
> index 000000000000..a5db6b61ceb2
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +#include <linux/ctype.h>
> +#include <linux/types.h>
> +#include <ubsan.h>
> +
> +void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
> +
> +void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
> +
> +void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
> +
> +void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
> +
> +void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_builtin_unreachable(void *_data) {}
> +
> +void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/9] KVM: arm64: Enable UBSan instrumentation in nVHE hyp code
@ 2021-01-18 9:53 ` David Brazdil
0 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 9:53 UTC (permalink / raw)
To: Elena Petrova
Cc: George-Aurelian Popescu, Marc Zyngier, open list, kvmarm,
George Popescu, linux-arm-kernel
On Fri, Jan 15, 2021 at 05:18:22PM +0000, Elena Petrova wrote:
> From: George-Aurelian Popescu <georgepope@google.com>
>
> Implement UBSan handlers inside nVHe hyp code, as empty functions for the
> moment, so the undefined behaviours, that are triggered there, will be
> linked to them, not to the ones defined in kernel-proper lib/ubsan.c.
>
> In this way, enabling UBSAN_MISC won't cause a link error.
The commit message needs to be updated - UBSAN_MISC does not exist any longer.
Also "nVHe" is a typo.
Other than that:
Acked-by: David Brazdil <dbrazdil@google.com>
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/kvm/hyp/nvhe/Makefile | 3 ++-
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/kvm/hyp/nvhe/ubsan.c
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index 1f1e351c5fe2..2a683e7c6c5b 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -10,6 +10,8 @@ obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
> hyp-main.o hyp-smp.o psci-relay.o
> obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
> ../fpsimd.o ../hyp-entry.o ../exception.o
> +obj-$(CONFIG_UBSAN) += ubsan.o
> +CFLAGS_ubsan.nvhe.o += -I $(srctree)/lib/
>
> ##
> ## Build rules for compiling nVHE hyp code
> @@ -61,7 +63,6 @@ KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAG
> # cause crashes. Just disable it.
> GCOV_PROFILE := n
> KASAN_SANITIZE := n
> -UBSAN_SANITIZE := n
> KCOV_INSTRUMENT := n
>
> # Skip objtool checking for this directory because nVHE code is compiled with
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> new file mode 100644
> index 000000000000..a5db6b61ceb2
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +#include <linux/ctype.h>
> +#include <linux/types.h>
> +#include <ubsan.h>
> +
> +void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_mul_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_negate_overflow(void *_data, void *old_val) {}
> +
> +void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
> +
> +void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
> +
> +void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
> +
> +void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
> +
> +void __ubsan_handle_builtin_unreachable(void *_data) {}
> +
> +void __ubsan_handle_load_invalid_value(void *_data, void *val) {}
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/9] KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to kernel
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-18 10:37 ` David Brazdil
-1 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 10:37 UTC (permalink / raw)
To: Elena Petrova
Cc: kvmarm, George Popescu, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
On Fri, Jan 15, 2021 at 05:18:23PM +0000, Elena Petrova wrote:
> From: George Popescu <georgepope@google.com>
>
> Share a buffer between the kernel and the hyp/nVHE code by using the
> macros from kvm_debug_buffer.h.
> The hyp/nVHE code requires a write index which counts how many elements
> have been writtens inside the buffer and the kernel requires a read
> index which counts how many elements have been read from the buffer.
> The write index and the buffer are shared with the kernel in read-only.
>
> The kvm_debug_buffer_ind returns the reading and writing points of the
> circular buffer and updates the reading index.
>
> Data collected from UBSan handlers inside hyp/nVHE is stored in the
> kvm_ubsan_buffer.
> This buffer stores only UBSan data because it should not be preoccupied
> by other mechanisms data structures and functionalities.
>
> Also, for the moment the buffer is mapped inside .bss, where both the kernel
> and the hyp/nVHE code have Read/Write rights, but in the future this will change
> and the kernel will not be able to acess hyp/nVHE's .bss. At that point the buffer
> will only need to be mapped in order for this patch to work.
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/assembler.h | 11 +++++++
> arch/arm64/include/asm/kvm_debug_buffer.h | 36 ++++++++++++++++++++
> arch/arm64/include/asm/kvm_host.h | 8 ++++-
> arch/arm64/include/asm/kvm_ubsan.h | 14 ++++++++
> arch/arm64/kvm/Makefile | 2 ++
> arch/arm64/kvm/arm.c | 9 +++++
> arch/arm64/kvm/hyp/nvhe/host.S | 4 +++
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 23 +++++++++++++
> arch/arm64/kvm/kvm_ubsan_buffer.c | 40 +++++++++++++++++++++++
> 9 files changed, 146 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
> create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
> create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index bf125c591116..ebc18a8a0e1f 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -258,6 +258,17 @@ alternative_endif
> ldr \dst, [\dst, \tmp]
> .endm
>
> + /*
> + * @sym: The name of the per-cpu variable
> + * @reg: value to store
> + * @tmp1: scratch register
> + * @tmp2: scratch register
> + */
> + .macro str_this_cpu sym, reg, tmp1, tmp2
> + adr_this_cpu \tmp1, \sym, \tmp2
> + str \reg, [\tmp1]
> + .endm
> +
> /*
> * vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
> */
> diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
> new file mode 100644
> index 000000000000..e5375c2cff1a
> --- /dev/null
> +++ b/arch/arm64/include/asm/kvm_debug_buffer.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <linux/percpu-defs.h>
> +
> +
> +#define KVM_DEBUG_BUFFER_SIZE 1000
nit: unused?
> +
> +#ifdef __KVM_NVHE_HYPERVISOR__
> +#define DEFINE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DEFINE_PER_CPU(type_name, buffer_name)[size]; \
> + DEFINE_PER_CPU(unsigned long, write_ind) = 0;
This is subjective but I would:
* stop calling the second variable a "write index" and instead call it
"cursor" - I think this is a leftover from George's early two-cursor
implementation
* wrap these two variables in a struct; that is a bit tricky given that each
instantiation defines its own size (the macros would have to generate that
struct), so bar that I would at least generate the name of the cursor
variable as buffer_name##suffix.
> +
> +#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DECLARE_PER_CPU(type_name, buffer_name)[size]; \
> + DECLARE_PER_CPU(unsigned long, write_ind);
> +#else
> +#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
> + DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> +#endif //__KVM_NVHE_HYPERVISOR__
> +
> +#ifdef __ASSEMBLY__
> +#include <asm/assembler.h>
> +
> +.macro clear_buffer tmp1, tmp2, tmp3
> + mov \tmp1, 0
> +#ifdef CONFIG_UBSAN
> + str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> +#endif //CONFIG_UBSAN
> +.endm
I find this macro odd. Everything else in this file is about a generic buffer
data structure, yet this macro enumerates all instantiations of it (currently
just one). Please keep this generic and pass the name of the buffer cursor
as a parameter.
> +
> +#endif
> \ No newline at end of file
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 8fcfab0c2567..905ca0d7c52c 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -592,6 +592,12 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
> void kvm_arm_halt_guest(struct kvm *kvm);
> void kvm_arm_resume_guest(struct kvm *kvm);
>
> +
nit: Two empty lines
> +#ifdef CONFIG_UBSAN
> +extern void __kvm_check_ubsan_buffer(void);
> +#else
> +static inline void __kvm_check_ubsan_buffer(void) {}
> +#endif /* CONFIG_UBSAN */
nit: An empty line here would be nice.
> #define kvm_call_hyp_nvhe(f, ...) \
> ({ \
> struct arm_smccc_res res; \
> @@ -599,7 +605,7 @@ void kvm_arm_resume_guest(struct kvm *kvm);
> arm_smccc_1_1_hvc(KVM_HOST_SMCCC_FUNC(f), \
> ##__VA_ARGS__, &res); \
> WARN_ON(res.a0 != SMCCC_RET_SUCCESS); \
> - \
> + __kvm_check_ubsan_buffer(); \
> res.a1; \
> })
>
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> new file mode 100644
> index 000000000000..fb32c7fd65d4
> --- /dev/null
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <ubsan.h>
> +
> +#define UBSAN_MAX_TYPE 6
nit: How about you make this the last entry in the `type` enum in
`kvm_ubsan_info`.
> +#define KVM_UBSAN_BUFFER_SIZE 1000
> +
> +struct kvm_ubsan_info {
> + int type;
> +};
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 13b017284bf9..ab3a9c402550 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -4,6 +4,7 @@
> #
>
> ccflags-y += -I $(srctree)/$(src)
> +CFLAGS_kvm_ubsan_buffer.o += -I $(srctree)/lib/
>
> KVM=../../../virt/kvm
>
> @@ -25,3 +26,4 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
> vgic/vgic-its.o vgic/vgic-debug.o
>
> kvm-$(CONFIG_HW_PERF_EVENTS) += pmu-emul.o
> +kvm-$(CONFIG_UBSAN) += kvm_ubsan_buffer.o
nit: Can we just name this kvm_ubsan.c?
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 04c44853b103..63f0495550b8 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1784,6 +1784,15 @@ static int init_hyp_mode(void)
> goto out_err;
> }
> }
> +#ifdef CONFIG_UBSAN
> + /* required by ubsan to access the handlers structures fields */
nit: capital R, grammar "handlers structures fields"
> + err = create_hyp_mappings(kvm_ksym_ref(_data),
> + kvm_ksym_ref(__end_once), PAGE_HYP_RO);
> + if (err) {
> + kvm_err("Cannot map data section\n");
> + goto out_err;
> + }
> +#endif
The hypervisor currently doesn't use the .data section, so this approach works
fine at the moment. But this will stop working as soon as somebody starts using
.data for other purposes. It would be nice to do this properly now rather than
fix it later as part of some unrelated series. What you'd need to do:
* add .data to arch/arm64/kvm/hyp/nvhe/hyp.lds.S; that will rename
hypervisor's .data to .hyp.data
* add .hyp.data to arch/arm64/kernel/vmlinux.lds.S
* add new entries to arch/arm64/include/asm/sections.h
* map .hyp.data here
Related, this currently relies on the fact that all pointers in .data are the
kernel VAs. That means later in this series it is safe to just copy the event
description to the buffer without any conversion. my series "Relocate absolute
hyp VAs" will convert all pointers in .hyp.* sections to the hypervisor VAs.
So once we do introduce .hyp.data, the hypervisor's ubsan.c will also have to
convert all pointers.
>
> /*
> * Map Hyp percpu pages
> diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
> index a820dfdc9c25..0967906265d3 100644
> --- a/arch/arm64/kvm/hyp/nvhe/host.S
> +++ b/arch/arm64/kvm/hyp/nvhe/host.S
> @@ -8,6 +8,7 @@
>
> #include <asm/assembler.h>
> #include <asm/kvm_asm.h>
> +#include <asm/kvm_debug_buffer.h>
> #include <asm/kvm_mmu.h>
>
> .text
> @@ -34,6 +35,9 @@ SYM_FUNC_START(__host_exit)
> /* Store the host regs x18-x29, lr */
> save_callee_saved_regs x0
>
> + /* when entering the host clear the buffers */
nit: capital W
> + clear_buffer x4, x5, x6
> +
> /* Save the host context pointer in x29 across the function call */
> mov x29, x0
> bl handle_trap
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> index a5db6b61ceb2..8a194fb1f6cf 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -3,10 +3,33 @@
> * Copyright 2020 Google LLC
> * Author: George Popescu <georgepope@google.com>
> */
> +#include <linux/bitops.h>
> #include <linux/ctype.h>
> #include <linux/types.h>
> +#include <linux/percpu-defs.h>
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_arm.h>
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_ubsan.h>
> +#include <asm/kvm_debug_buffer.h>
> +#include <kvm/arm_pmu.h>
> #include <ubsan.h>
>
> +DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> + kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> +
> +static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
> +{
> + struct kvm_ubsan_info *res = NULL;
> + unsigned long write_ind = __this_cpu_read(kvm_ubsan_buff_wr_ind);
nit: checkpatch will want an empty line between declarations and code
> + if (write_ind < KVM_UBSAN_BUFFER_SIZE) {
> + res = this_cpu_ptr(&kvm_ubsan_buffer[write_ind]);
> + ++write_ind;
> + __this_cpu_write(kvm_ubsan_buff_wr_ind, write_ind);
nit: Could save a line by just writing write_ind+1
> + }
> + return res;
> +}
> +
> void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
>
> void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> new file mode 100644
> index 000000000000..4a1959ba9f68
> --- /dev/null
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <linux/ctype.h>
> +#include <linux/types.h>
> +#include <asm/kvm_debug_buffer.h>
> +#include <asm/kvm_arm.h>
> +#include <asm/kvm_asm.h>
> +#include <kvm/arm_pmu.h>
> +
> +#include <ubsan.h>
> +#include <asm/kvm_ubsan.h>
> +
> +DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> + kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> +
> +
> +void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> +{
> + unsigned long i;
> + struct kvm_ubsan_info *slot;
> +
> + slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
nit: no whitespace after bracket?
> + for (i = left; i < right; ++i) {
> + /* check ubsan data */
> + slot[i].type = 0;
> + }
> +}
> +
> +void __kvm_check_ubsan_buffer(void)
> +{
> + unsigned long *write_ind;
> +
> + write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
> + iterate_kvm_ubsan_buffer(0, *write_ind);
I think this is another remnant of George's previous implementation. The first
argument will always be zero, so I don't see the point in splitting this into
two functions any more.
> +}
> +
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/9] KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to kernel
@ 2021-01-18 10:37 ` David Brazdil
0 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 10:37 UTC (permalink / raw)
To: Elena Petrova
Cc: George Popescu, Marc Zyngier, open list, kvmarm, George Popescu,
linux-arm-kernel
On Fri, Jan 15, 2021 at 05:18:23PM +0000, Elena Petrova wrote:
> From: George Popescu <georgepope@google.com>
>
> Share a buffer between the kernel and the hyp/nVHE code by using the
> macros from kvm_debug_buffer.h.
> The hyp/nVHE code requires a write index which counts how many elements
> have been writtens inside the buffer and the kernel requires a read
> index which counts how many elements have been read from the buffer.
> The write index and the buffer are shared with the kernel in read-only.
>
> The kvm_debug_buffer_ind returns the reading and writing points of the
> circular buffer and updates the reading index.
>
> Data collected from UBSan handlers inside hyp/nVHE is stored in the
> kvm_ubsan_buffer.
> This buffer stores only UBSan data because it should not be preoccupied
> by other mechanisms data structures and functionalities.
>
> Also, for the moment the buffer is mapped inside .bss, where both the kernel
> and the hyp/nVHE code have Read/Write rights, but in the future this will change
> and the kernel will not be able to acess hyp/nVHE's .bss. At that point the buffer
> will only need to be mapped in order for this patch to work.
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/assembler.h | 11 +++++++
> arch/arm64/include/asm/kvm_debug_buffer.h | 36 ++++++++++++++++++++
> arch/arm64/include/asm/kvm_host.h | 8 ++++-
> arch/arm64/include/asm/kvm_ubsan.h | 14 ++++++++
> arch/arm64/kvm/Makefile | 2 ++
> arch/arm64/kvm/arm.c | 9 +++++
> arch/arm64/kvm/hyp/nvhe/host.S | 4 +++
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 23 +++++++++++++
> arch/arm64/kvm/kvm_ubsan_buffer.c | 40 +++++++++++++++++++++++
> 9 files changed, 146 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
> create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
> create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index bf125c591116..ebc18a8a0e1f 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -258,6 +258,17 @@ alternative_endif
> ldr \dst, [\dst, \tmp]
> .endm
>
> + /*
> + * @sym: The name of the per-cpu variable
> + * @reg: value to store
> + * @tmp1: scratch register
> + * @tmp2: scratch register
> + */
> + .macro str_this_cpu sym, reg, tmp1, tmp2
> + adr_this_cpu \tmp1, \sym, \tmp2
> + str \reg, [\tmp1]
> + .endm
> +
> /*
> * vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
> */
> diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
> new file mode 100644
> index 000000000000..e5375c2cff1a
> --- /dev/null
> +++ b/arch/arm64/include/asm/kvm_debug_buffer.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <linux/percpu-defs.h>
> +
> +
> +#define KVM_DEBUG_BUFFER_SIZE 1000
nit: unused?
> +
> +#ifdef __KVM_NVHE_HYPERVISOR__
> +#define DEFINE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DEFINE_PER_CPU(type_name, buffer_name)[size]; \
> + DEFINE_PER_CPU(unsigned long, write_ind) = 0;
This is subjective but I would:
* stop calling the second variable a "write index" and instead call it
"cursor" - I think this is a leftover from George's early two-cursor
implementation
* wrap these two variables in a struct; that is a bit tricky given that each
instantiation defines its own size (the macros would have to generate that
struct), so bar that I would at least generate the name of the cursor
variable as buffer_name##suffix.
> +
> +#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DECLARE_PER_CPU(type_name, buffer_name)[size]; \
> + DECLARE_PER_CPU(unsigned long, write_ind);
> +#else
> +#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
> + DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> +#endif //__KVM_NVHE_HYPERVISOR__
> +
> +#ifdef __ASSEMBLY__
> +#include <asm/assembler.h>
> +
> +.macro clear_buffer tmp1, tmp2, tmp3
> + mov \tmp1, 0
> +#ifdef CONFIG_UBSAN
> + str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> +#endif //CONFIG_UBSAN
> +.endm
I find this macro odd. Everything else in this file is about a generic buffer
data structure, yet this macro enumerates all instantiations of it (currently
just one). Please keep this generic and pass the name of the buffer cursor
as a parameter.
> +
> +#endif
> \ No newline at end of file
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 8fcfab0c2567..905ca0d7c52c 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -592,6 +592,12 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
> void kvm_arm_halt_guest(struct kvm *kvm);
> void kvm_arm_resume_guest(struct kvm *kvm);
>
> +
nit: Two empty lines
> +#ifdef CONFIG_UBSAN
> +extern void __kvm_check_ubsan_buffer(void);
> +#else
> +static inline void __kvm_check_ubsan_buffer(void) {}
> +#endif /* CONFIG_UBSAN */
nit: An empty line here would be nice.
> #define kvm_call_hyp_nvhe(f, ...) \
> ({ \
> struct arm_smccc_res res; \
> @@ -599,7 +605,7 @@ void kvm_arm_resume_guest(struct kvm *kvm);
> arm_smccc_1_1_hvc(KVM_HOST_SMCCC_FUNC(f), \
> ##__VA_ARGS__, &res); \
> WARN_ON(res.a0 != SMCCC_RET_SUCCESS); \
> - \
> + __kvm_check_ubsan_buffer(); \
> res.a1; \
> })
>
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> new file mode 100644
> index 000000000000..fb32c7fd65d4
> --- /dev/null
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <ubsan.h>
> +
> +#define UBSAN_MAX_TYPE 6
nit: How about you make this the last entry in the `type` enum in
`kvm_ubsan_info`.
> +#define KVM_UBSAN_BUFFER_SIZE 1000
> +
> +struct kvm_ubsan_info {
> + int type;
> +};
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 13b017284bf9..ab3a9c402550 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -4,6 +4,7 @@
> #
>
> ccflags-y += -I $(srctree)/$(src)
> +CFLAGS_kvm_ubsan_buffer.o += -I $(srctree)/lib/
>
> KVM=../../../virt/kvm
>
> @@ -25,3 +26,4 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
> vgic/vgic-its.o vgic/vgic-debug.o
>
> kvm-$(CONFIG_HW_PERF_EVENTS) += pmu-emul.o
> +kvm-$(CONFIG_UBSAN) += kvm_ubsan_buffer.o
nit: Can we just name this kvm_ubsan.c?
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 04c44853b103..63f0495550b8 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1784,6 +1784,15 @@ static int init_hyp_mode(void)
> goto out_err;
> }
> }
> +#ifdef CONFIG_UBSAN
> + /* required by ubsan to access the handlers structures fields */
nit: capital R, grammar "handlers structures fields"
> + err = create_hyp_mappings(kvm_ksym_ref(_data),
> + kvm_ksym_ref(__end_once), PAGE_HYP_RO);
> + if (err) {
> + kvm_err("Cannot map data section\n");
> + goto out_err;
> + }
> +#endif
The hypervisor currently doesn't use the .data section, so this approach works
fine at the moment. But this will stop working as soon as somebody starts using
.data for other purposes. It would be nice to do this properly now rather than
fix it later as part of some unrelated series. What you'd need to do:
* add .data to arch/arm64/kvm/hyp/nvhe/hyp.lds.S; that will rename
hypervisor's .data to .hyp.data
* add .hyp.data to arch/arm64/kernel/vmlinux.lds.S
* add new entries to arch/arm64/include/asm/sections.h
* map .hyp.data here
Related, this currently relies on the fact that all pointers in .data are the
kernel VAs. That means later in this series it is safe to just copy the event
description to the buffer without any conversion. my series "Relocate absolute
hyp VAs" will convert all pointers in .hyp.* sections to the hypervisor VAs.
So once we do introduce .hyp.data, the hypervisor's ubsan.c will also have to
convert all pointers.
>
> /*
> * Map Hyp percpu pages
> diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
> index a820dfdc9c25..0967906265d3 100644
> --- a/arch/arm64/kvm/hyp/nvhe/host.S
> +++ b/arch/arm64/kvm/hyp/nvhe/host.S
> @@ -8,6 +8,7 @@
>
> #include <asm/assembler.h>
> #include <asm/kvm_asm.h>
> +#include <asm/kvm_debug_buffer.h>
> #include <asm/kvm_mmu.h>
>
> .text
> @@ -34,6 +35,9 @@ SYM_FUNC_START(__host_exit)
> /* Store the host regs x18-x29, lr */
> save_callee_saved_regs x0
>
> + /* when entering the host clear the buffers */
nit: capital W
> + clear_buffer x4, x5, x6
> +
> /* Save the host context pointer in x29 across the function call */
> mov x29, x0
> bl handle_trap
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> index a5db6b61ceb2..8a194fb1f6cf 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -3,10 +3,33 @@
> * Copyright 2020 Google LLC
> * Author: George Popescu <georgepope@google.com>
> */
> +#include <linux/bitops.h>
> #include <linux/ctype.h>
> #include <linux/types.h>
> +#include <linux/percpu-defs.h>
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_arm.h>
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_ubsan.h>
> +#include <asm/kvm_debug_buffer.h>
> +#include <kvm/arm_pmu.h>
> #include <ubsan.h>
>
> +DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> + kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> +
> +static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
> +{
> + struct kvm_ubsan_info *res = NULL;
> + unsigned long write_ind = __this_cpu_read(kvm_ubsan_buff_wr_ind);
nit: checkpatch will want an empty line between declarations and code
> + if (write_ind < KVM_UBSAN_BUFFER_SIZE) {
> + res = this_cpu_ptr(&kvm_ubsan_buffer[write_ind]);
> + ++write_ind;
> + __this_cpu_write(kvm_ubsan_buff_wr_ind, write_ind);
nit: Could save a line by just writing write_ind+1
> + }
> + return res;
> +}
> +
> void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
>
> void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> new file mode 100644
> index 000000000000..4a1959ba9f68
> --- /dev/null
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <linux/ctype.h>
> +#include <linux/types.h>
> +#include <asm/kvm_debug_buffer.h>
> +#include <asm/kvm_arm.h>
> +#include <asm/kvm_asm.h>
> +#include <kvm/arm_pmu.h>
> +
> +#include <ubsan.h>
> +#include <asm/kvm_ubsan.h>
> +
> +DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> + kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> +
> +
> +void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> +{
> + unsigned long i;
> + struct kvm_ubsan_info *slot;
> +
> + slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
nit: no whitespace after bracket?
> + for (i = left; i < right; ++i) {
> + /* check ubsan data */
> + slot[i].type = 0;
> + }
> +}
> +
> +void __kvm_check_ubsan_buffer(void)
> +{
> + unsigned long *write_ind;
> +
> + write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
> + iterate_kvm_ubsan_buffer(0, *write_ind);
I think this is another remnant of George's previous implementation. The first
argument will always be zero, so I don't see the point in splitting this into
two functions any more.
> +}
> +
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/9] KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to kernel
@ 2021-01-18 10:37 ` David Brazdil
0 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 10:37 UTC (permalink / raw)
To: Elena Petrova
Cc: George Popescu, Marc Zyngier, open list, kvmarm, George Popescu,
linux-arm-kernel
On Fri, Jan 15, 2021 at 05:18:23PM +0000, Elena Petrova wrote:
> From: George Popescu <georgepope@google.com>
>
> Share a buffer between the kernel and the hyp/nVHE code by using the
> macros from kvm_debug_buffer.h.
> The hyp/nVHE code requires a write index which counts how many elements
> have been writtens inside the buffer and the kernel requires a read
> index which counts how many elements have been read from the buffer.
> The write index and the buffer are shared with the kernel in read-only.
>
> The kvm_debug_buffer_ind returns the reading and writing points of the
> circular buffer and updates the reading index.
>
> Data collected from UBSan handlers inside hyp/nVHE is stored in the
> kvm_ubsan_buffer.
> This buffer stores only UBSan data because it should not be preoccupied
> by other mechanisms data structures and functionalities.
>
> Also, for the moment the buffer is mapped inside .bss, where both the kernel
> and the hyp/nVHE code have Read/Write rights, but in the future this will change
> and the kernel will not be able to acess hyp/nVHE's .bss. At that point the buffer
> will only need to be mapped in order for this patch to work.
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/assembler.h | 11 +++++++
> arch/arm64/include/asm/kvm_debug_buffer.h | 36 ++++++++++++++++++++
> arch/arm64/include/asm/kvm_host.h | 8 ++++-
> arch/arm64/include/asm/kvm_ubsan.h | 14 ++++++++
> arch/arm64/kvm/Makefile | 2 ++
> arch/arm64/kvm/arm.c | 9 +++++
> arch/arm64/kvm/hyp/nvhe/host.S | 4 +++
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 23 +++++++++++++
> arch/arm64/kvm/kvm_ubsan_buffer.c | 40 +++++++++++++++++++++++
> 9 files changed, 146 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/kvm_debug_buffer.h
> create mode 100644 arch/arm64/include/asm/kvm_ubsan.h
> create mode 100644 arch/arm64/kvm/kvm_ubsan_buffer.c
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index bf125c591116..ebc18a8a0e1f 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -258,6 +258,17 @@ alternative_endif
> ldr \dst, [\dst, \tmp]
> .endm
>
> + /*
> + * @sym: The name of the per-cpu variable
> + * @reg: value to store
> + * @tmp1: scratch register
> + * @tmp2: scratch register
> + */
> + .macro str_this_cpu sym, reg, tmp1, tmp2
> + adr_this_cpu \tmp1, \sym, \tmp2
> + str \reg, [\tmp1]
> + .endm
> +
> /*
> * vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
> */
> diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
> new file mode 100644
> index 000000000000..e5375c2cff1a
> --- /dev/null
> +++ b/arch/arm64/include/asm/kvm_debug_buffer.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <linux/percpu-defs.h>
> +
> +
> +#define KVM_DEBUG_BUFFER_SIZE 1000
nit: unused?
> +
> +#ifdef __KVM_NVHE_HYPERVISOR__
> +#define DEFINE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DEFINE_PER_CPU(type_name, buffer_name)[size]; \
> + DEFINE_PER_CPU(unsigned long, write_ind) = 0;
This is subjective but I would:
* stop calling the second variable a "write index" and instead call it
"cursor" - I think this is a leftover from George's early two-cursor
implementation
* wrap these two variables in a struct; that is a bit tricky given that each
instantiation defines its own size (the macros would have to generate that
struct), so bar that I would at least generate the name of the cursor
variable as buffer_name##suffix.
> +
> +#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DECLARE_PER_CPU(type_name, buffer_name)[size]; \
> + DECLARE_PER_CPU(unsigned long, write_ind);
> +#else
> +#define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> + DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
> + DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> +#endif //__KVM_NVHE_HYPERVISOR__
> +
> +#ifdef __ASSEMBLY__
> +#include <asm/assembler.h>
> +
> +.macro clear_buffer tmp1, tmp2, tmp3
> + mov \tmp1, 0
> +#ifdef CONFIG_UBSAN
> + str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> +#endif //CONFIG_UBSAN
> +.endm
I find this macro odd. Everything else in this file is about a generic buffer
data structure, yet this macro enumerates all instantiations of it (currently
just one). Please keep this generic and pass the name of the buffer cursor
as a parameter.
> +
> +#endif
> \ No newline at end of file
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 8fcfab0c2567..905ca0d7c52c 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -592,6 +592,12 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
> void kvm_arm_halt_guest(struct kvm *kvm);
> void kvm_arm_resume_guest(struct kvm *kvm);
>
> +
nit: Two empty lines
> +#ifdef CONFIG_UBSAN
> +extern void __kvm_check_ubsan_buffer(void);
> +#else
> +static inline void __kvm_check_ubsan_buffer(void) {}
> +#endif /* CONFIG_UBSAN */
nit: An empty line here would be nice.
> #define kvm_call_hyp_nvhe(f, ...) \
> ({ \
> struct arm_smccc_res res; \
> @@ -599,7 +605,7 @@ void kvm_arm_resume_guest(struct kvm *kvm);
> arm_smccc_1_1_hvc(KVM_HOST_SMCCC_FUNC(f), \
> ##__VA_ARGS__, &res); \
> WARN_ON(res.a0 != SMCCC_RET_SUCCESS); \
> - \
> + __kvm_check_ubsan_buffer(); \
> res.a1; \
> })
>
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> new file mode 100644
> index 000000000000..fb32c7fd65d4
> --- /dev/null
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <ubsan.h>
> +
> +#define UBSAN_MAX_TYPE 6
nit: How about you make this the last entry in the `type` enum in
`kvm_ubsan_info`.
> +#define KVM_UBSAN_BUFFER_SIZE 1000
> +
> +struct kvm_ubsan_info {
> + int type;
> +};
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 13b017284bf9..ab3a9c402550 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -4,6 +4,7 @@
> #
>
> ccflags-y += -I $(srctree)/$(src)
> +CFLAGS_kvm_ubsan_buffer.o += -I $(srctree)/lib/
>
> KVM=../../../virt/kvm
>
> @@ -25,3 +26,4 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
> vgic/vgic-its.o vgic/vgic-debug.o
>
> kvm-$(CONFIG_HW_PERF_EVENTS) += pmu-emul.o
> +kvm-$(CONFIG_UBSAN) += kvm_ubsan_buffer.o
nit: Can we just name this kvm_ubsan.c?
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 04c44853b103..63f0495550b8 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1784,6 +1784,15 @@ static int init_hyp_mode(void)
> goto out_err;
> }
> }
> +#ifdef CONFIG_UBSAN
> + /* required by ubsan to access the handlers structures fields */
nit: capital R, grammar "handlers structures fields"
> + err = create_hyp_mappings(kvm_ksym_ref(_data),
> + kvm_ksym_ref(__end_once), PAGE_HYP_RO);
> + if (err) {
> + kvm_err("Cannot map data section\n");
> + goto out_err;
> + }
> +#endif
The hypervisor currently doesn't use the .data section, so this approach works
fine at the moment. But this will stop working as soon as somebody starts using
.data for other purposes. It would be nice to do this properly now rather than
fix it later as part of some unrelated series. What you'd need to do:
* add .data to arch/arm64/kvm/hyp/nvhe/hyp.lds.S; that will rename
hypervisor's .data to .hyp.data
* add .hyp.data to arch/arm64/kernel/vmlinux.lds.S
* add new entries to arch/arm64/include/asm/sections.h
* map .hyp.data here
Related, this currently relies on the fact that all pointers in .data are the
kernel VAs. That means later in this series it is safe to just copy the event
description to the buffer without any conversion. my series "Relocate absolute
hyp VAs" will convert all pointers in .hyp.* sections to the hypervisor VAs.
So once we do introduce .hyp.data, the hypervisor's ubsan.c will also have to
convert all pointers.
>
> /*
> * Map Hyp percpu pages
> diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S
> index a820dfdc9c25..0967906265d3 100644
> --- a/arch/arm64/kvm/hyp/nvhe/host.S
> +++ b/arch/arm64/kvm/hyp/nvhe/host.S
> @@ -8,6 +8,7 @@
>
> #include <asm/assembler.h>
> #include <asm/kvm_asm.h>
> +#include <asm/kvm_debug_buffer.h>
> #include <asm/kvm_mmu.h>
>
> .text
> @@ -34,6 +35,9 @@ SYM_FUNC_START(__host_exit)
> /* Store the host regs x18-x29, lr */
> save_callee_saved_regs x0
>
> + /* when entering the host clear the buffers */
nit: capital W
> + clear_buffer x4, x5, x6
> +
> /* Save the host context pointer in x29 across the function call */
> mov x29, x0
> bl handle_trap
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> index a5db6b61ceb2..8a194fb1f6cf 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -3,10 +3,33 @@
> * Copyright 2020 Google LLC
> * Author: George Popescu <georgepope@google.com>
> */
> +#include <linux/bitops.h>
> #include <linux/ctype.h>
> #include <linux/types.h>
> +#include <linux/percpu-defs.h>
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_arm.h>
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_ubsan.h>
> +#include <asm/kvm_debug_buffer.h>
> +#include <kvm/arm_pmu.h>
> #include <ubsan.h>
>
> +DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> + kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> +
> +static inline struct kvm_ubsan_info *kvm_ubsan_buffer_next_slot(void)
> +{
> + struct kvm_ubsan_info *res = NULL;
> + unsigned long write_ind = __this_cpu_read(kvm_ubsan_buff_wr_ind);
nit: checkpatch will want an empty line between declarations and code
> + if (write_ind < KVM_UBSAN_BUFFER_SIZE) {
> + res = this_cpu_ptr(&kvm_ubsan_buffer[write_ind]);
> + ++write_ind;
> + __this_cpu_write(kvm_ubsan_buff_wr_ind, write_ind);
nit: Could save a line by just writing write_ind+1
> + }
> + return res;
> +}
> +
> void __ubsan_handle_add_overflow(void *_data, void *lhs, void *rhs) {}
>
> void __ubsan_handle_sub_overflow(void *_data, void *lhs, void *rhs) {}
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> new file mode 100644
> index 000000000000..4a1959ba9f68
> --- /dev/null
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2020 Google LLC
> + * Author: George Popescu <georgepope@google.com>
> + */
> +
> +#include <linux/ctype.h>
> +#include <linux/types.h>
> +#include <asm/kvm_debug_buffer.h>
> +#include <asm/kvm_arm.h>
> +#include <asm/kvm_asm.h>
> +#include <kvm/arm_pmu.h>
> +
> +#include <ubsan.h>
> +#include <asm/kvm_ubsan.h>
> +
> +DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> + kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> +
> +
> +void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> +{
> + unsigned long i;
> + struct kvm_ubsan_info *slot;
> +
> + slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
nit: no whitespace after bracket?
> + for (i = left; i < right; ++i) {
> + /* check ubsan data */
> + slot[i].type = 0;
> + }
> +}
> +
> +void __kvm_check_ubsan_buffer(void)
> +{
> + unsigned long *write_ind;
> +
> + write_ind = (unsigned long *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buff_wr_ind);
> + iterate_kvm_ubsan_buffer(0, *write_ind);
I think this is another remnant of George's previous implementation. The first
argument will always be zero, so I don't see the point in splitting this into
two functions any more.
> +}
> +
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 9/9] KVM: arm64: Add UBSan tests for PKVM.
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-18 10:44 ` David Brazdil
-1 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 10:44 UTC (permalink / raw)
To: Elena Petrova
Cc: kvmarm, George-Aurelian Popescu, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
On Fri, Jan 15, 2021 at 05:18:30PM +0000, Elena Petrova wrote:
> From: George-Aurelian Popescu <georgepope@google.com>
>
> Test the UBsan functionality inside hyp/nVHE.
> Because modules are not supported inside of hyp/nVHE code, the default
> testing module for UBSan can not be used.
> New functions have to be defined inside of hyp/nVHE.
> They are called in kvm_get_mdcr_el2, to test UBSAN whenever a VM starts.
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/assembler.h | 17 ++-
> arch/arm64/include/asm/kvm_debug_buffer.h | 10 +-
> arch/arm64/include/asm/kvm_ubsan.h | 2 +-
> arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 ++++++++++++++++++++
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
> arch/arm64/kvm/kvm_ubsan_buffer.c | 1 -
> 6 files changed, 128 insertions(+), 17 deletions(-)
> create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index ebc18a8a0e1f..8422b0d925e8 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -259,16 +259,15 @@ alternative_endif
> .endm
>
> /*
> - * @sym: The name of the per-cpu variable
> - * @reg: value to store
> - * @tmp1: scratch register
> - * @tmp2: scratch register
> - */
> - .macro str_this_cpu sym, reg, tmp1, tmp2
> - adr_this_cpu \tmp1, \sym, \tmp2
> + * @sym: The name of the per-cpu variable
> + * @reg: value to store
> + * @tmp1: scratch register
> + * @tmp2: scratch register
> + */
> + .macro str_this_cpu sym, reg, tmp1, tmp2
> + adr_this_cpu \tmp1, \sym, \tmp2
> str \reg, [\tmp1]
> - .endm
> -
> + .endm
> /*
> * vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
> */
> diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
> index e5375c2cff1a..361b473bb004 100644
> --- a/arch/arm64/include/asm/kvm_debug_buffer.h
> +++ b/arch/arm64/include/asm/kvm_debug_buffer.h
> @@ -3,10 +3,8 @@
> * Copyright 2020 Google LLC
> * Author: George Popescu <georgepope@google.com>
> */
> -
> #include <linux/percpu-defs.h>
>
> -
> #define KVM_DEBUG_BUFFER_SIZE 1000
>
> #ifdef __KVM_NVHE_HYPERVISOR__
> @@ -20,17 +18,17 @@
> #else
> #define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
> - DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> + DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> #endif //__KVM_NVHE_HYPERVISOR__
>
> #ifdef __ASSEMBLY__
> #include <asm/assembler.h>
>
> .macro clear_buffer tmp1, tmp2, tmp3
> - mov \tmp1, 0
> + mov \tmp1, 0
> #ifdef CONFIG_UBSAN
> - str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> + str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> #endif //CONFIG_UBSAN
> .endm
Are these fixing formatting? If so, move it to the patch that introduced this.
>
> -#endif
> \ No newline at end of file
> +#endif
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> index da4a3b4e28e0..0b8bed08d48e 100644
> --- a/arch/arm64/include/asm/kvm_ubsan.h
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -9,7 +9,6 @@
> #define UBSAN_MAX_TYPE 6
> #define KVM_UBSAN_BUFFER_SIZE 1000
>
> -
ditto
> struct ubsan_values {
> void *lval;
> void *rval;
> @@ -18,6 +17,7 @@ struct ubsan_values {
>
> struct kvm_ubsan_info {
> enum {
> + UBSAN_NONE,
This also looks like it should have been in a previous patch. The code assumes
that 'type == 0' means 'empty slot'. So presumably this is fixing a bug?
> UBSAN_OUT_OF_BOUNDS,
> UBSAN_UNREACHABLE_DATA,
> UBSAN_SHIFT_OUT_OF_BOUNDS,
> diff --git a/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
> new file mode 100644
> index 000000000000..07759c0d1e0e
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
> @@ -0,0 +1,112 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <linux/ctype.h>
> +
> +typedef void(*test_ubsan_fp)(void);
> +
> +static void test_ubsan_add_overflow(void)
> +{
> + volatile int val = INT_MAX;
> +
> + val += 2;
> +}
> +
> +static void test_ubsan_sub_overflow(void)
> +{
> + volatile int val = INT_MIN;
> + volatile int val2 = 2;
> +
> + val -= val2;
> +}
> +
> +static void test_ubsan_mul_overflow(void)
> +{
> + volatile int val = INT_MAX / 2;
> +
> + val *= 3;
> +}
> +
> +static void test_ubsan_negate_overflow(void)
> +{
> + volatile int val = INT_MIN;
> +
> + val = -val;
> +}
> +
> +static void test_ubsan_divrem_overflow(void)
> +{
> + volatile int val = 16;
> + volatile int val2 = 0;
> +
> + val /= val2;
> +}
> +
> +static void test_ubsan_shift_out_of_bounds(void)
> +{
> + volatile int val = -1;
> + int val2 = 10;
> +
> + val2 <<= val;
> +}
> +
> +static void test_ubsan_out_of_bounds(void)
> +{
> + volatile int i = 4, j = 5;
> + volatile int arr[4];
> +
> + arr[j] = i;
> +}
> +
> +static void test_ubsan_load_invalid_value(void)
> +{
> + volatile char *dst, *src;
> + bool val, val2, *ptr;
> + char c = 4;
> +
> + dst = (char *)&val;
> + src = &c;
> + *dst = *src;
> +
> + ptr = &val2;
> + val2 = val;
> +}
> +
> +static void test_ubsan_misaligned_access(void)
> +{
> + volatile char arr[5] __aligned(4) = {1, 2, 3, 4, 5};
> + volatile int *ptr, val = 6;
> +
> + ptr = (int *)(arr + 1);
> + *ptr = val;
> +}
> +
> +static void test_ubsan_object_size_mismatch(void)
> +{
> + /* "((aligned(8)))" helps this not into be misaligned for ptr-access. */
> + volatile int val __aligned(8) = 4;
> + volatile long long *ptr, val2;
> +
> + ptr = (long long *)&val;
> + val2 = *ptr;
> +}
> +
> +static const test_ubsan_fp test_ubsan_array[] = {
> + test_ubsan_out_of_bounds,
> + test_ubsan_add_overflow,
> + test_ubsan_sub_overflow,
> + test_ubsan_mul_overflow,
> + test_ubsan_negate_overflow,
> + test_ubsan_divrem_overflow,
> + test_ubsan_shift_out_of_bounds,
> + test_ubsan_load_invalid_value,
> + test_ubsan_misaligned_access,
> + test_ubsan_object_size_mismatch,
> +};
> +
> +static void test_ubsan(void)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(test_ubsan_array); i++)
> + test_ubsan_array[i]();
> +}
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index a906f9e2ff34..939600e9fdd6 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -13,6 +13,7 @@
> #include <asm/kvm_mmu.h>
>
> #include <nvhe/trap_handler.h>
> +#include <hyp/test_ubsan.h>
>
> DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
>
> @@ -90,6 +91,8 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
> static void handle___kvm_get_mdcr_el2(struct kvm_cpu_context *host_ctxt)
> {
> cpu_reg(host_ctxt, 1) = __kvm_get_mdcr_el2();
> + if (IS_ENABLED(CONFIG_TEST_UBSAN))
> + test_ubsan();
> }
We cannot keep this in here. It's useful to exercise your code in development
but not something you should upstream. I would either remove this completely or
implement it as a separate hypercall.
>
> static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> index 2c7060cbb48b..49bedc9de139 100644
> --- a/arch/arm64/kvm/kvm_ubsan_buffer.c
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -11,7 +11,6 @@
> #include <asm/kvm_asm.h>
> #include <kvm/arm_pmu.h>
>
> -#include <ubsan.h>
ditto
> #include <asm/kvm_ubsan.h>
>
> DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 9/9] KVM: arm64: Add UBSan tests for PKVM.
@ 2021-01-18 10:44 ` David Brazdil
0 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 10:44 UTC (permalink / raw)
To: Elena Petrova
Cc: George-Aurelian Popescu, Marc Zyngier, open list, kvmarm,
George Popescu, linux-arm-kernel
On Fri, Jan 15, 2021 at 05:18:30PM +0000, Elena Petrova wrote:
> From: George-Aurelian Popescu <georgepope@google.com>
>
> Test the UBsan functionality inside hyp/nVHE.
> Because modules are not supported inside of hyp/nVHE code, the default
> testing module for UBSan can not be used.
> New functions have to be defined inside of hyp/nVHE.
> They are called in kvm_get_mdcr_el2, to test UBSAN whenever a VM starts.
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/assembler.h | 17 ++-
> arch/arm64/include/asm/kvm_debug_buffer.h | 10 +-
> arch/arm64/include/asm/kvm_ubsan.h | 2 +-
> arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 ++++++++++++++++++++
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
> arch/arm64/kvm/kvm_ubsan_buffer.c | 1 -
> 6 files changed, 128 insertions(+), 17 deletions(-)
> create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index ebc18a8a0e1f..8422b0d925e8 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -259,16 +259,15 @@ alternative_endif
> .endm
>
> /*
> - * @sym: The name of the per-cpu variable
> - * @reg: value to store
> - * @tmp1: scratch register
> - * @tmp2: scratch register
> - */
> - .macro str_this_cpu sym, reg, tmp1, tmp2
> - adr_this_cpu \tmp1, \sym, \tmp2
> + * @sym: The name of the per-cpu variable
> + * @reg: value to store
> + * @tmp1: scratch register
> + * @tmp2: scratch register
> + */
> + .macro str_this_cpu sym, reg, tmp1, tmp2
> + adr_this_cpu \tmp1, \sym, \tmp2
> str \reg, [\tmp1]
> - .endm
> -
> + .endm
> /*
> * vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
> */
> diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
> index e5375c2cff1a..361b473bb004 100644
> --- a/arch/arm64/include/asm/kvm_debug_buffer.h
> +++ b/arch/arm64/include/asm/kvm_debug_buffer.h
> @@ -3,10 +3,8 @@
> * Copyright 2020 Google LLC
> * Author: George Popescu <georgepope@google.com>
> */
> -
> #include <linux/percpu-defs.h>
>
> -
> #define KVM_DEBUG_BUFFER_SIZE 1000
>
> #ifdef __KVM_NVHE_HYPERVISOR__
> @@ -20,17 +18,17 @@
> #else
> #define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
> - DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> + DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> #endif //__KVM_NVHE_HYPERVISOR__
>
> #ifdef __ASSEMBLY__
> #include <asm/assembler.h>
>
> .macro clear_buffer tmp1, tmp2, tmp3
> - mov \tmp1, 0
> + mov \tmp1, 0
> #ifdef CONFIG_UBSAN
> - str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> + str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> #endif //CONFIG_UBSAN
> .endm
Are these fixing formatting? If so, move it to the patch that introduced this.
>
> -#endif
> \ No newline at end of file
> +#endif
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> index da4a3b4e28e0..0b8bed08d48e 100644
> --- a/arch/arm64/include/asm/kvm_ubsan.h
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -9,7 +9,6 @@
> #define UBSAN_MAX_TYPE 6
> #define KVM_UBSAN_BUFFER_SIZE 1000
>
> -
ditto
> struct ubsan_values {
> void *lval;
> void *rval;
> @@ -18,6 +17,7 @@ struct ubsan_values {
>
> struct kvm_ubsan_info {
> enum {
> + UBSAN_NONE,
This also looks like it should have been in a previous patch. The code assumes
that 'type == 0' means 'empty slot'. So presumably this is fixing a bug?
> UBSAN_OUT_OF_BOUNDS,
> UBSAN_UNREACHABLE_DATA,
> UBSAN_SHIFT_OUT_OF_BOUNDS,
> diff --git a/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
> new file mode 100644
> index 000000000000..07759c0d1e0e
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
> @@ -0,0 +1,112 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <linux/ctype.h>
> +
> +typedef void(*test_ubsan_fp)(void);
> +
> +static void test_ubsan_add_overflow(void)
> +{
> + volatile int val = INT_MAX;
> +
> + val += 2;
> +}
> +
> +static void test_ubsan_sub_overflow(void)
> +{
> + volatile int val = INT_MIN;
> + volatile int val2 = 2;
> +
> + val -= val2;
> +}
> +
> +static void test_ubsan_mul_overflow(void)
> +{
> + volatile int val = INT_MAX / 2;
> +
> + val *= 3;
> +}
> +
> +static void test_ubsan_negate_overflow(void)
> +{
> + volatile int val = INT_MIN;
> +
> + val = -val;
> +}
> +
> +static void test_ubsan_divrem_overflow(void)
> +{
> + volatile int val = 16;
> + volatile int val2 = 0;
> +
> + val /= val2;
> +}
> +
> +static void test_ubsan_shift_out_of_bounds(void)
> +{
> + volatile int val = -1;
> + int val2 = 10;
> +
> + val2 <<= val;
> +}
> +
> +static void test_ubsan_out_of_bounds(void)
> +{
> + volatile int i = 4, j = 5;
> + volatile int arr[4];
> +
> + arr[j] = i;
> +}
> +
> +static void test_ubsan_load_invalid_value(void)
> +{
> + volatile char *dst, *src;
> + bool val, val2, *ptr;
> + char c = 4;
> +
> + dst = (char *)&val;
> + src = &c;
> + *dst = *src;
> +
> + ptr = &val2;
> + val2 = val;
> +}
> +
> +static void test_ubsan_misaligned_access(void)
> +{
> + volatile char arr[5] __aligned(4) = {1, 2, 3, 4, 5};
> + volatile int *ptr, val = 6;
> +
> + ptr = (int *)(arr + 1);
> + *ptr = val;
> +}
> +
> +static void test_ubsan_object_size_mismatch(void)
> +{
> + /* "((aligned(8)))" helps this not into be misaligned for ptr-access. */
> + volatile int val __aligned(8) = 4;
> + volatile long long *ptr, val2;
> +
> + ptr = (long long *)&val;
> + val2 = *ptr;
> +}
> +
> +static const test_ubsan_fp test_ubsan_array[] = {
> + test_ubsan_out_of_bounds,
> + test_ubsan_add_overflow,
> + test_ubsan_sub_overflow,
> + test_ubsan_mul_overflow,
> + test_ubsan_negate_overflow,
> + test_ubsan_divrem_overflow,
> + test_ubsan_shift_out_of_bounds,
> + test_ubsan_load_invalid_value,
> + test_ubsan_misaligned_access,
> + test_ubsan_object_size_mismatch,
> +};
> +
> +static void test_ubsan(void)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(test_ubsan_array); i++)
> + test_ubsan_array[i]();
> +}
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index a906f9e2ff34..939600e9fdd6 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -13,6 +13,7 @@
> #include <asm/kvm_mmu.h>
>
> #include <nvhe/trap_handler.h>
> +#include <hyp/test_ubsan.h>
>
> DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
>
> @@ -90,6 +91,8 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
> static void handle___kvm_get_mdcr_el2(struct kvm_cpu_context *host_ctxt)
> {
> cpu_reg(host_ctxt, 1) = __kvm_get_mdcr_el2();
> + if (IS_ENABLED(CONFIG_TEST_UBSAN))
> + test_ubsan();
> }
We cannot keep this in here. It's useful to exercise your code in development
but not something you should upstream. I would either remove this completely or
implement it as a separate hypercall.
>
> static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> index 2c7060cbb48b..49bedc9de139 100644
> --- a/arch/arm64/kvm/kvm_ubsan_buffer.c
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -11,7 +11,6 @@
> #include <asm/kvm_asm.h>
> #include <kvm/arm_pmu.h>
>
> -#include <ubsan.h>
ditto
> #include <asm/kvm_ubsan.h>
>
> DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 9/9] KVM: arm64: Add UBSan tests for PKVM.
@ 2021-01-18 10:44 ` David Brazdil
0 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 10:44 UTC (permalink / raw)
To: Elena Petrova
Cc: George-Aurelian Popescu, Marc Zyngier, open list, kvmarm,
George Popescu, linux-arm-kernel
On Fri, Jan 15, 2021 at 05:18:30PM +0000, Elena Petrova wrote:
> From: George-Aurelian Popescu <georgepope@google.com>
>
> Test the UBsan functionality inside hyp/nVHE.
> Because modules are not supported inside of hyp/nVHE code, the default
> testing module for UBSan can not be used.
> New functions have to be defined inside of hyp/nVHE.
> They are called in kvm_get_mdcr_el2, to test UBSAN whenever a VM starts.
>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/assembler.h | 17 ++-
> arch/arm64/include/asm/kvm_debug_buffer.h | 10 +-
> arch/arm64/include/asm/kvm_ubsan.h | 2 +-
> arch/arm64/kvm/hyp/include/hyp/test_ubsan.h | 112 ++++++++++++++++++++
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
> arch/arm64/kvm/kvm_ubsan_buffer.c | 1 -
> 6 files changed, 128 insertions(+), 17 deletions(-)
> create mode 100644 arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index ebc18a8a0e1f..8422b0d925e8 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -259,16 +259,15 @@ alternative_endif
> .endm
>
> /*
> - * @sym: The name of the per-cpu variable
> - * @reg: value to store
> - * @tmp1: scratch register
> - * @tmp2: scratch register
> - */
> - .macro str_this_cpu sym, reg, tmp1, tmp2
> - adr_this_cpu \tmp1, \sym, \tmp2
> + * @sym: The name of the per-cpu variable
> + * @reg: value to store
> + * @tmp1: scratch register
> + * @tmp2: scratch register
> + */
> + .macro str_this_cpu sym, reg, tmp1, tmp2
> + adr_this_cpu \tmp1, \sym, \tmp2
> str \reg, [\tmp1]
> - .endm
> -
> + .endm
> /*
> * vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
> */
> diff --git a/arch/arm64/include/asm/kvm_debug_buffer.h b/arch/arm64/include/asm/kvm_debug_buffer.h
> index e5375c2cff1a..361b473bb004 100644
> --- a/arch/arm64/include/asm/kvm_debug_buffer.h
> +++ b/arch/arm64/include/asm/kvm_debug_buffer.h
> @@ -3,10 +3,8 @@
> * Copyright 2020 Google LLC
> * Author: George Popescu <georgepope@google.com>
> */
> -
> #include <linux/percpu-defs.h>
>
> -
> #define KVM_DEBUG_BUFFER_SIZE 1000
>
> #ifdef __KVM_NVHE_HYPERVISOR__
> @@ -20,17 +18,17 @@
> #else
> #define DECLARE_KVM_DEBUG_BUFFER(type_name, buffer_name, write_ind, size)\
> DECLARE_KVM_NVHE_PER_CPU(type_name, buffer_name)[size]; \
> - DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> + DECLARE_KVM_NVHE_PER_CPU(unsigned long, write_ind);
> #endif //__KVM_NVHE_HYPERVISOR__
>
> #ifdef __ASSEMBLY__
> #include <asm/assembler.h>
>
> .macro clear_buffer tmp1, tmp2, tmp3
> - mov \tmp1, 0
> + mov \tmp1, 0
> #ifdef CONFIG_UBSAN
> - str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> + str_this_cpu kvm_ubsan_buff_wr_ind, \tmp1, \tmp2, \tmp3
> #endif //CONFIG_UBSAN
> .endm
Are these fixing formatting? If so, move it to the patch that introduced this.
>
> -#endif
> \ No newline at end of file
> +#endif
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> index da4a3b4e28e0..0b8bed08d48e 100644
> --- a/arch/arm64/include/asm/kvm_ubsan.h
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -9,7 +9,6 @@
> #define UBSAN_MAX_TYPE 6
> #define KVM_UBSAN_BUFFER_SIZE 1000
>
> -
ditto
> struct ubsan_values {
> void *lval;
> void *rval;
> @@ -18,6 +17,7 @@ struct ubsan_values {
>
> struct kvm_ubsan_info {
> enum {
> + UBSAN_NONE,
This also looks like it should have been in a previous patch. The code assumes
that 'type == 0' means 'empty slot'. So presumably this is fixing a bug?
> UBSAN_OUT_OF_BOUNDS,
> UBSAN_UNREACHABLE_DATA,
> UBSAN_SHIFT_OUT_OF_BOUNDS,
> diff --git a/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
> new file mode 100644
> index 000000000000..07759c0d1e0e
> --- /dev/null
> +++ b/arch/arm64/kvm/hyp/include/hyp/test_ubsan.h
> @@ -0,0 +1,112 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <linux/ctype.h>
> +
> +typedef void(*test_ubsan_fp)(void);
> +
> +static void test_ubsan_add_overflow(void)
> +{
> + volatile int val = INT_MAX;
> +
> + val += 2;
> +}
> +
> +static void test_ubsan_sub_overflow(void)
> +{
> + volatile int val = INT_MIN;
> + volatile int val2 = 2;
> +
> + val -= val2;
> +}
> +
> +static void test_ubsan_mul_overflow(void)
> +{
> + volatile int val = INT_MAX / 2;
> +
> + val *= 3;
> +}
> +
> +static void test_ubsan_negate_overflow(void)
> +{
> + volatile int val = INT_MIN;
> +
> + val = -val;
> +}
> +
> +static void test_ubsan_divrem_overflow(void)
> +{
> + volatile int val = 16;
> + volatile int val2 = 0;
> +
> + val /= val2;
> +}
> +
> +static void test_ubsan_shift_out_of_bounds(void)
> +{
> + volatile int val = -1;
> + int val2 = 10;
> +
> + val2 <<= val;
> +}
> +
> +static void test_ubsan_out_of_bounds(void)
> +{
> + volatile int i = 4, j = 5;
> + volatile int arr[4];
> +
> + arr[j] = i;
> +}
> +
> +static void test_ubsan_load_invalid_value(void)
> +{
> + volatile char *dst, *src;
> + bool val, val2, *ptr;
> + char c = 4;
> +
> + dst = (char *)&val;
> + src = &c;
> + *dst = *src;
> +
> + ptr = &val2;
> + val2 = val;
> +}
> +
> +static void test_ubsan_misaligned_access(void)
> +{
> + volatile char arr[5] __aligned(4) = {1, 2, 3, 4, 5};
> + volatile int *ptr, val = 6;
> +
> + ptr = (int *)(arr + 1);
> + *ptr = val;
> +}
> +
> +static void test_ubsan_object_size_mismatch(void)
> +{
> + /* "((aligned(8)))" helps this not into be misaligned for ptr-access. */
> + volatile int val __aligned(8) = 4;
> + volatile long long *ptr, val2;
> +
> + ptr = (long long *)&val;
> + val2 = *ptr;
> +}
> +
> +static const test_ubsan_fp test_ubsan_array[] = {
> + test_ubsan_out_of_bounds,
> + test_ubsan_add_overflow,
> + test_ubsan_sub_overflow,
> + test_ubsan_mul_overflow,
> + test_ubsan_negate_overflow,
> + test_ubsan_divrem_overflow,
> + test_ubsan_shift_out_of_bounds,
> + test_ubsan_load_invalid_value,
> + test_ubsan_misaligned_access,
> + test_ubsan_object_size_mismatch,
> +};
> +
> +static void test_ubsan(void)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(test_ubsan_array); i++)
> + test_ubsan_array[i]();
> +}
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index a906f9e2ff34..939600e9fdd6 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -13,6 +13,7 @@
> #include <asm/kvm_mmu.h>
>
> #include <nvhe/trap_handler.h>
> +#include <hyp/test_ubsan.h>
>
> DEFINE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
>
> @@ -90,6 +91,8 @@ static void handle___vgic_v3_init_lrs(struct kvm_cpu_context *host_ctxt)
> static void handle___kvm_get_mdcr_el2(struct kvm_cpu_context *host_ctxt)
> {
> cpu_reg(host_ctxt, 1) = __kvm_get_mdcr_el2();
> + if (IS_ENABLED(CONFIG_TEST_UBSAN))
> + test_ubsan();
> }
We cannot keep this in here. It's useful to exercise your code in development
but not something you should upstream. I would either remove this completely or
implement it as a separate hypercall.
>
> static void handle___vgic_v3_save_aprs(struct kvm_cpu_context *host_ctxt)
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> index 2c7060cbb48b..49bedc9de139 100644
> --- a/arch/arm64/kvm/kvm_ubsan_buffer.c
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -11,7 +11,6 @@
> #include <asm/kvm_asm.h>
> #include <kvm/arm_pmu.h>
>
> -#include <ubsan.h>
ditto
> #include <asm/kvm_ubsan.h>
>
> DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 3/9] KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
2021-01-15 17:18 ` Elena Petrova
(?)
@ 2021-01-18 11:06 ` David Brazdil
-1 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 11:06 UTC (permalink / raw)
To: Elena Petrova
Cc: kvmarm, George Popescu, Marc Zyngier, open list,
linux-arm-kernel, George Popescu
On Fri, Jan 15, 2021 at 05:18:24PM +0000, Elena Petrova wrote:
> From: George Popescu <georgepope@google.com>
>
> If an out of bounds happens inside the hyp/nVHE code, the ubsan_out_of_bounds
> handler stores the logging data inside the kvm_ubsan_buffer. The one responsible
> for printing is the kernel ubsan_out_of_bounds handler. The process of
> decapsulating the data from the buffer is straightforward.
>
> Signed-off-by: George Popescu <georgepope@google.com>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/kvm_ubsan.h | 19 ++++++++++++++++++-
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 ++++++++++++--
> arch/arm64/kvm/kvm_ubsan_buffer.c | 10 ++++++++++
> 3 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> index fb32c7fd65d4..4f471acb88b0 100644
> --- a/arch/arm64/include/asm/kvm_ubsan.h
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -9,6 +9,23 @@
> #define UBSAN_MAX_TYPE 6
> #define KVM_UBSAN_BUFFER_SIZE 1000
>
> +
> +struct ubsan_values {
> + void *lval;
> + void *rval;
> + char op;
> +};
> +
> struct kvm_ubsan_info {
> - int type;
> + enum {
> + UBSAN_OUT_OF_BOUNDS,
> + } type;
> + union {
> + struct out_of_bounds_data out_of_bounds_data;
> + };
> + union {
> + struct ubsan_values u_val;
> + };
I don't see this second union having more members later in the series.
Remove it? Even the 'struct ubsan_values' seems redundant and we could just
have those three fields directly here.
> };
> +
> +void __ubsan_handle_out_of_bounds(void *_data, void *index);
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> index 8a194fb1f6cf..55a8f6db8555 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -13,7 +13,6 @@
> #include <asm/kvm_ubsan.h>
> #include <asm/kvm_debug_buffer.h>
> #include <kvm/arm_pmu.h>
> -#include <ubsan.h>
>
> DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> @@ -44,7 +43,18 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
>
> void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
>
> -void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
> +void __ubsan_handle_out_of_bounds(void *_data, void *index)
> +{
> + struct kvm_ubsan_info *slot;
> + struct out_of_bounds_data *data = _data;
> +
> + slot = kvm_ubsan_buffer_next_slot();
> + if (slot) {
> + slot->type = UBSAN_OUT_OF_BOUNDS;
> + slot->out_of_bounds_data = *data;
> + slot->u_val.lval = index;
> + }
> +}
>
> void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
>
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> index 4a1959ba9f68..a1523f86be3c 100644
> --- a/arch/arm64/kvm/kvm_ubsan_buffer.c
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -17,6 +17,15 @@
> DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
>
> +void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
> +{
> + switch (slot->type) {
> + case UBSAN_OUT_OF_BOUNDS:
> + __ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
> + slot->u_val.lval);
> + break;
> + }
> +}
>
> void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> {
> @@ -26,6 +35,7 @@ void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
> for (i = left; i < right; ++i) {
> /* check ubsan data */
> + __kvm_check_ubsan_data(slot + i);
Not sure why this is breaking out into another function. The code will not
be shared with any other user.
> slot[i].type = 0;
This invalidation is redundant. The buffer's cursor will be reset on next
hypercall, which will implicitly invalidate all entries.
> }
> }
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 3/9] KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
@ 2021-01-18 11:06 ` David Brazdil
0 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 11:06 UTC (permalink / raw)
To: Elena Petrova
Cc: George Popescu, Marc Zyngier, open list, kvmarm, George Popescu,
linux-arm-kernel
On Fri, Jan 15, 2021 at 05:18:24PM +0000, Elena Petrova wrote:
> From: George Popescu <georgepope@google.com>
>
> If an out of bounds happens inside the hyp/nVHE code, the ubsan_out_of_bounds
> handler stores the logging data inside the kvm_ubsan_buffer. The one responsible
> for printing is the kernel ubsan_out_of_bounds handler. The process of
> decapsulating the data from the buffer is straightforward.
>
> Signed-off-by: George Popescu <georgepope@google.com>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/kvm_ubsan.h | 19 ++++++++++++++++++-
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 ++++++++++++--
> arch/arm64/kvm/kvm_ubsan_buffer.c | 10 ++++++++++
> 3 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> index fb32c7fd65d4..4f471acb88b0 100644
> --- a/arch/arm64/include/asm/kvm_ubsan.h
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -9,6 +9,23 @@
> #define UBSAN_MAX_TYPE 6
> #define KVM_UBSAN_BUFFER_SIZE 1000
>
> +
> +struct ubsan_values {
> + void *lval;
> + void *rval;
> + char op;
> +};
> +
> struct kvm_ubsan_info {
> - int type;
> + enum {
> + UBSAN_OUT_OF_BOUNDS,
> + } type;
> + union {
> + struct out_of_bounds_data out_of_bounds_data;
> + };
> + union {
> + struct ubsan_values u_val;
> + };
I don't see this second union having more members later in the series.
Remove it? Even the 'struct ubsan_values' seems redundant and we could just
have those three fields directly here.
> };
> +
> +void __ubsan_handle_out_of_bounds(void *_data, void *index);
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> index 8a194fb1f6cf..55a8f6db8555 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -13,7 +13,6 @@
> #include <asm/kvm_ubsan.h>
> #include <asm/kvm_debug_buffer.h>
> #include <kvm/arm_pmu.h>
> -#include <ubsan.h>
>
> DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> @@ -44,7 +43,18 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
>
> void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
>
> -void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
> +void __ubsan_handle_out_of_bounds(void *_data, void *index)
> +{
> + struct kvm_ubsan_info *slot;
> + struct out_of_bounds_data *data = _data;
> +
> + slot = kvm_ubsan_buffer_next_slot();
> + if (slot) {
> + slot->type = UBSAN_OUT_OF_BOUNDS;
> + slot->out_of_bounds_data = *data;
> + slot->u_val.lval = index;
> + }
> +}
>
> void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
>
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> index 4a1959ba9f68..a1523f86be3c 100644
> --- a/arch/arm64/kvm/kvm_ubsan_buffer.c
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -17,6 +17,15 @@
> DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
>
> +void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
> +{
> + switch (slot->type) {
> + case UBSAN_OUT_OF_BOUNDS:
> + __ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
> + slot->u_val.lval);
> + break;
> + }
> +}
>
> void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> {
> @@ -26,6 +35,7 @@ void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
> for (i = left; i < right; ++i) {
> /* check ubsan data */
> + __kvm_check_ubsan_data(slot + i);
Not sure why this is breaking out into another function. The code will not
be shared with any other user.
> slot[i].type = 0;
This invalidation is redundant. The buffer's cursor will be reset on next
hypercall, which will implicitly invalidate all entries.
> }
> }
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 3/9] KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE
@ 2021-01-18 11:06 ` David Brazdil
0 siblings, 0 replies; 42+ messages in thread
From: David Brazdil @ 2021-01-18 11:06 UTC (permalink / raw)
To: Elena Petrova
Cc: George Popescu, Marc Zyngier, open list, kvmarm, George Popescu,
linux-arm-kernel
On Fri, Jan 15, 2021 at 05:18:24PM +0000, Elena Petrova wrote:
> From: George Popescu <georgepope@google.com>
>
> If an out of bounds happens inside the hyp/nVHE code, the ubsan_out_of_bounds
> handler stores the logging data inside the kvm_ubsan_buffer. The one responsible
> for printing is the kernel ubsan_out_of_bounds handler. The process of
> decapsulating the data from the buffer is straightforward.
>
> Signed-off-by: George Popescu <georgepope@google.com>
> Signed-off-by: Elena Petrova <lenaptr@google.com>
> ---
> arch/arm64/include/asm/kvm_ubsan.h | 19 ++++++++++++++++++-
> arch/arm64/kvm/hyp/nvhe/ubsan.c | 14 ++++++++++++--
> arch/arm64/kvm/kvm_ubsan_buffer.c | 10 ++++++++++
> 3 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_ubsan.h b/arch/arm64/include/asm/kvm_ubsan.h
> index fb32c7fd65d4..4f471acb88b0 100644
> --- a/arch/arm64/include/asm/kvm_ubsan.h
> +++ b/arch/arm64/include/asm/kvm_ubsan.h
> @@ -9,6 +9,23 @@
> #define UBSAN_MAX_TYPE 6
> #define KVM_UBSAN_BUFFER_SIZE 1000
>
> +
> +struct ubsan_values {
> + void *lval;
> + void *rval;
> + char op;
> +};
> +
> struct kvm_ubsan_info {
> - int type;
> + enum {
> + UBSAN_OUT_OF_BOUNDS,
> + } type;
> + union {
> + struct out_of_bounds_data out_of_bounds_data;
> + };
> + union {
> + struct ubsan_values u_val;
> + };
I don't see this second union having more members later in the series.
Remove it? Even the 'struct ubsan_values' seems redundant and we could just
have those three fields directly here.
> };
> +
> +void __ubsan_handle_out_of_bounds(void *_data, void *index);
> diff --git a/arch/arm64/kvm/hyp/nvhe/ubsan.c b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> index 8a194fb1f6cf..55a8f6db8555 100644
> --- a/arch/arm64/kvm/hyp/nvhe/ubsan.c
> +++ b/arch/arm64/kvm/hyp/nvhe/ubsan.c
> @@ -13,7 +13,6 @@
> #include <asm/kvm_ubsan.h>
> #include <asm/kvm_debug_buffer.h>
> #include <kvm/arm_pmu.h>
> -#include <ubsan.h>
>
> DEFINE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
> @@ -44,7 +43,18 @@ void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr) {}
>
> void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr) {}
>
> -void __ubsan_handle_out_of_bounds(void *_data, void *index) {}
> +void __ubsan_handle_out_of_bounds(void *_data, void *index)
> +{
> + struct kvm_ubsan_info *slot;
> + struct out_of_bounds_data *data = _data;
> +
> + slot = kvm_ubsan_buffer_next_slot();
> + if (slot) {
> + slot->type = UBSAN_OUT_OF_BOUNDS;
> + slot->out_of_bounds_data = *data;
> + slot->u_val.lval = index;
> + }
> +}
>
> void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs) {}
>
> diff --git a/arch/arm64/kvm/kvm_ubsan_buffer.c b/arch/arm64/kvm/kvm_ubsan_buffer.c
> index 4a1959ba9f68..a1523f86be3c 100644
> --- a/arch/arm64/kvm/kvm_ubsan_buffer.c
> +++ b/arch/arm64/kvm/kvm_ubsan_buffer.c
> @@ -17,6 +17,15 @@
> DECLARE_KVM_DEBUG_BUFFER(struct kvm_ubsan_info, kvm_ubsan_buffer,
> kvm_ubsan_buff_wr_ind, KVM_UBSAN_BUFFER_SIZE);
>
> +void __kvm_check_ubsan_data(struct kvm_ubsan_info *slot)
> +{
> + switch (slot->type) {
> + case UBSAN_OUT_OF_BOUNDS:
> + __ubsan_handle_out_of_bounds(&slot->out_of_bounds_data,
> + slot->u_val.lval);
> + break;
> + }
> +}
>
> void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> {
> @@ -26,6 +35,7 @@ void iterate_kvm_ubsan_buffer(unsigned long left, unsigned long right)
> slot = (struct kvm_ubsan_info *) this_cpu_ptr_nvhe_sym(kvm_ubsan_buffer);
> for (i = left; i < right; ++i) {
> /* check ubsan data */
> + __kvm_check_ubsan_data(slot + i);
Not sure why this is breaking out into another function. The code will not
be shared with any other user.
> slot[i].type = 0;
This invalidation is redundant. The buffer's cursor will be reset on next
hypercall, which will implicitly invalidate all entries.
> }
> }
> --
> 2.30.0.296.g2bfb1c46d8-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2021-01-18 20:32 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-15 17:18 [PATCH v3 0/9] UBSan Enablement for hyp/nVHE code Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` [PATCH v3 1/9] KVM: arm64: Enable UBSan instrumentation in nVHE hyp code Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-18 9:53 ` David Brazdil
2021-01-18 9:53 ` David Brazdil
2021-01-18 9:53 ` David Brazdil
2021-01-15 17:18 ` [PATCH v3 2/9] KVM: arm64: Add a buffer that can pass UBSan data from hyp/nVHE to kernel Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-18 10:37 ` David Brazdil
2021-01-18 10:37 ` David Brazdil
2021-01-18 10:37 ` David Brazdil
2021-01-15 17:18 ` [PATCH v3 3/9] KVM: arm64: Enable UBSAN_BOUNDS for the both the kernel and hyp/nVHE Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-18 11:06 ` David Brazdil
2021-01-18 11:06 ` David Brazdil
2021-01-18 11:06 ` David Brazdil
2021-01-15 17:18 ` [PATCH v3 4/9] KVM: arm64: Enable UBsan check for unreachable code inside hyp/nVHE code Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` [PATCH v3 5/9] KVM: arm64: Enable shift out of bounds undefined behaviour check for hyp/nVHE Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` [PATCH v3 6/9] KVM: arm64: __ubsan_handle_load_invalid_value EL2 implementation Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` [PATCH v3 7/9] KVM: arm64: Detect type mismatch undefined behaviour from hyp/nVHE code Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` [PATCH v3 8/9] KVM: arm64: Detect arithmetic overflow is inside hyp/nVHE Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` [PATCH v3 9/9] KVM: arm64: Add UBSan tests for PKVM Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-15 17:18 ` Elena Petrova
2021-01-18 10:44 ` David Brazdil
2021-01-18 10:44 ` David Brazdil
2021-01-18 10:44 ` David Brazdil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.