On Wed, Apr 16, 2025 at 06:04:33PM +0000, Mostafa Saleh wrote:
> Add a new Kconfig CONFIG_UBSAN_KVM_EL2 for KVM which enables
> UBSAN for EL2 code (in protected/nvhe/hvhe) modes.
> This will re-use the same checks enabled for the kernel for
> the hypervisor. The only difference is that for EL2 it always
> emits a "brk" instead of implementing hooks as the hypervisor
> can't print reports.
>
> The KVM code will re-use the same code for the kernel
> "report_ubsan_failure()" so #ifdefs are changed to also have this
> code for CONFIG_UBSAN_KVM_EL2
>
> Signed-off-by: Mostafa Saleh <smost...@google.com>
> ---
> arch/arm64/kvm/hyp/nvhe/Makefile | 6 ++++++
> include/linux/ubsan.h | 2 +-
> lib/Kconfig.ubsan | 9 +++++++++
> lib/ubsan.c | 6 ++++--
> scripts/Makefile.ubsan | 5 ++++-
> 5 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile
> b/arch/arm64/kvm/hyp/nvhe/Makefile
> index b43426a493df..cbe7e12752bc 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -99,3 +99,9 @@ KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE)
> $(CC_FLAGS_SCS), $(KBUILD_CFLAG
> # causes a build failure. Remove profile optimization flags.
> KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%,
> $(KBUILD_CFLAGS))
> KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
> +
> +ifeq ($(CONFIG_UBSAN_KVM_EL2),y)
> +UBSAN_SANITIZE := y
> +# Always use brk and not hooks
> +ccflags-y += $(CFLAGS_UBSAN_FOR_TRAP)
> +endif
> diff --git a/include/linux/ubsan.h b/include/linux/ubsan.h
> index c843816f5f68..3ab8d38aedb8 100644
> --- a/include/linux/ubsan.h
> +++ b/include/linux/ubsan.h
> @@ -2,7 +2,7 @@
> #ifndef _LINUX_UBSAN_H
> #define _LINUX_UBSAN_H
>
> -#ifdef CONFIG_UBSAN_TRAP
> +#if defined(CONFIG_UBSAN_TRAP) || defined(CONFIG_UBSAN_KVM_EL2)
> const char *report_ubsan_failure(u32 check_type);
> #else
> static inline const char *report_ubsan_failure(u32 check_type)
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index 4216b3a4ff21..3878858eb473 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -166,4 +166,13 @@ config TEST_UBSAN
> This is a test module for UBSAN.
> It triggers various undefined behavior, and detect it.
>
> +config UBSAN_KVM_EL2
> + bool "UBSAN for KVM code at EL2"
> + depends on ARM64
> + help
> + Enable UBSAN when running on ARM64 with KVM in a split mode
> + (nvhe/hvhe/protected) for the hypervisor code running in EL2.
> + In this mode, any UBSAN violation in EL2 would panic the kernel
> + and information similar to UBSAN_TRAP would be printed.
> +
> endif # if UBSAN
> diff --git a/lib/ubsan.c b/lib/ubsan.c
> index 17993727fc96..a6ca235dd714 100644
> --- a/lib/ubsan.c
> +++ b/lib/ubsan.c
> @@ -19,7 +19,7 @@
>
> #include "ubsan.h"
>
> -#ifdef CONFIG_UBSAN_TRAP
> +#if defined(CONFIG_UBSAN_TRAP) || defined(CONFIG_UBSAN_KVM_EL2)
> /*
> * Only include matches for UBSAN checks that are actually compiled in.
> * The mappings of struct SanitizerKind (the -fsanitize=xxx args) to
> @@ -97,7 +97,9 @@ const char *report_ubsan_failure(u32 check_type)
> }
> }
>
> -#else
> +#endif
> +
> +#ifndef CONFIG_UBSAN_TRAP
> static const char * const type_check_kinds[] = {
> "load of",
> "store to",
> diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
> index 9e35198edbf0..68af6830af0f 100644
> --- a/scripts/Makefile.ubsan
> +++ b/scripts/Makefile.ubsan
> @@ -1,5 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
>
> +#Shared with KVM/arm64
Nitpick: Please add a space between "#" and "Shared", and end the line
with "."
> +export CFLAGS_UBSAN_FOR_TRAP := $(call
> cc-option,-fsanitize-trap=undefined,-fsanitize-undefined-trap-on-error)
> +
> # Enable available and selected UBSAN features.
> ubsan-cflags-$(CONFIG_UBSAN_ALIGNMENT) += -fsanitize=alignment
> ubsan-cflags-$(CONFIG_UBSAN_BOUNDS_STRICT) += -fsanitize=bounds-strict
> @@ -10,7 +13,7 @@ ubsan-cflags-$(CONFIG_UBSAN_DIV_ZERO) +=
> -fsanitize=integer-divide-by-zero
> ubsan-cflags-$(CONFIG_UBSAN_UNREACHABLE) += -fsanitize=unreachable
> ubsan-cflags-$(CONFIG_UBSAN_BOOL) += -fsanitize=bool
> ubsan-cflags-$(CONFIG_UBSAN_ENUM) += -fsanitize=enum
> -ubsan-cflags-$(CONFIG_UBSAN_TRAP) += $(call
> cc-option,-fsanitize-trap=undefined,-fsanitize-undefined-trap-on-error)
> +ubsan-cflags-$(CONFIG_UBSAN_TRAP) += $(CFLAGS_UBSAN_FOR_TRAP)
Another minor style request: please name this "CFLAGS_UBSAN_TRAP"
(nothing else in Kconfig uses "FOR" like this, and leaving it off sounds
more declarative).
>
> export CFLAGS_UBSAN := $(ubsan-cflags-y)
Otherwise, yes, looks good.
--
Kees Cook
