arch/arm64/include/asm/brk-imm.h | 2 ++ arch/arm64/kernel/traps.c | 21 +++++++++++++ include/linux/ubsan.h | 9 ++++++ lib/Makefile | 2 -- lib/ubsan.c | 54 ++++++++++++++++++++++++++++++++ lib/ubsan.h | 28 +++++++++++++++++ 6 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 include/linux/ubsan.h
On arm64, Clang encodes the UBSAN check type in the esr. Extract this
and actually report UBSAN traps with some specificity when building with
CONFIG_UBSAN_TRAP. Before:
Internal error: BRK handler: 00000000f20003e8 [#1] PREEMPT SMP
After:
Internal error: UBSAN: shift out of bounds: 00000000f2005514 [#1] PREEMPT SMP
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: John Stultz <jstultz@google.com>
Cc: Yongqin Liu <yongqin.liu@linaro.org>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Cc: Marco Elver <elver@google.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: llvm@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/arm64/include/asm/brk-imm.h | 2 ++
arch/arm64/kernel/traps.c | 21 +++++++++++++
include/linux/ubsan.h | 9 ++++++
lib/Makefile | 2 --
lib/ubsan.c | 54 ++++++++++++++++++++++++++++++++
lib/ubsan.h | 28 +++++++++++++++++
6 files changed, 114 insertions(+), 2 deletions(-)
create mode 100644 include/linux/ubsan.h
diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
index 6e000113e508..3f0f0d03268b 100644
--- a/arch/arm64/include/asm/brk-imm.h
+++ b/arch/arm64/include/asm/brk-imm.h
@@ -28,6 +28,8 @@
#define BUG_BRK_IMM 0x800
#define KASAN_BRK_IMM 0x900
#define KASAN_BRK_MASK 0x0ff
+#define UBSAN_BRK_IMM 0x5500
+#define UBSAN_BRK_MASK 0x00ff
#define CFI_BRK_IMM_TARGET GENMASK(4, 0)
#define CFI_BRK_IMM_TYPE GENMASK(9, 5)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 4c0caa589e12..87f42eb1c950 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -26,6 +26,7 @@
#include <linux/syscalls.h>
#include <linux/mm_types.h>
#include <linux/kasan.h>
+#include <linux/ubsan.h>
#include <linux/cfi.h>
#include <asm/atomic.h>
@@ -1074,6 +1075,19 @@ static struct break_hook kasan_break_hook = {
};
#endif
+#ifdef CONFIG_UBSAN_TRAP
+static int ubsan_handler(struct pt_regs *regs, unsigned long esr)
+{
+ die(report_ubsan_failure(regs, esr & UBSAN_BRK_MASK), regs, esr);
+ return DBG_HOOK_HANDLED;
+}
+
+static struct break_hook ubsan_break_hook = {
+ .fn = ubsan_handler,
+ .imm = UBSAN_BRK_IMM,
+ .mask = UBSAN_BRK_MASK,
+};
+#endif
#define esr_comment(esr) ((esr) & ESR_ELx_BRK64_ISS_COMMENT_MASK)
@@ -1091,6 +1105,10 @@ int __init early_brk64(unsigned long addr, unsigned long esr,
#ifdef CONFIG_KASAN_SW_TAGS
if ((esr_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
+#endif
+#ifdef CONFIG_UBSAN_TRAP
+ if ((esr_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM)
+ return ubsan_handler(regs, esr) != DBG_HOOK_HANDLED;
#endif
return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
}
@@ -1104,6 +1122,9 @@ void __init trap_init(void)
register_kernel_break_hook(&fault_break_hook);
#ifdef CONFIG_KASAN_SW_TAGS
register_kernel_break_hook(&kasan_break_hook);
+#endif
+#ifdef CONFIG_UBSAN_TRAP
+ register_kernel_break_hook(&ubsan_break_hook);
#endif
debug_traps_init();
}
diff --git a/include/linux/ubsan.h b/include/linux/ubsan.h
new file mode 100644
index 000000000000..bff7445498de
--- /dev/null
+++ b/include/linux/ubsan.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_UBSAN_H
+#define _LINUX_UBSAN_H
+
+#ifdef CONFIG_UBSAN_TRAP
+const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type);
+#endif
+
+#endif
diff --git a/lib/Makefile b/lib/Makefile
index 4d9461bfea42..81b988bf9448 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -340,9 +340,7 @@ quiet_cmd_build_OID_registry = GEN $@
clean-files += oid_registry_data.c
obj-$(CONFIG_UCS2_STRING) += ucs2_string.o
-ifneq ($(CONFIG_UBSAN_TRAP),y)
obj-$(CONFIG_UBSAN) += ubsan.o
-endif
UBSAN_SANITIZE_ubsan.o := n
KASAN_SANITIZE_ubsan.o := n
diff --git a/lib/ubsan.c b/lib/ubsan.c
index 60c7099857a0..98a671ec56e9 100644
--- a/lib/ubsan.c
+++ b/lib/ubsan.c
@@ -18,6 +18,58 @@
#include "ubsan.h"
+#ifdef CONFIG_UBSAN_TRAP
+const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type)
+{
+ switch (check_type) {
+ case ubsan_add_overflow:
+ return "UBSAN: addition overflow";
+ case ubsan_builtin_unreachable:
+ return "UBSAN: unreachable code";
+ case ubsan_cfi_check_fail:
+ return "UBSAN: CFI failure";
+ case ubsan_divrem_overflow:
+ return "UBSAN: divide/remainder overflow";
+ case ubsan_function_type_mismatch:
+ return "UBSAN: function type mismatch";
+ case ubsan_implicit_conversion:
+ return "UBSAN: implicit conversion";
+ case ubsan_invalid_builtin:
+ return "UBSAN: invalid builtin";
+ case ubsan_invalid_objc_cast:
+ return "UBSAN: invalid object cast";
+ case ubsan_load_invalid_value:
+ return "UBSAN: loading invalid value";
+ case ubsan_missing_return:
+ return "UBSAN: missing return";
+ case ubsan_mul_overflow:
+ return "UBSAN: multiplication overflow";
+ case ubsan_negate_overflow:
+ return "UBSAN: negation overflow";
+ case ubsan_nonnull_arg:
+ return "UBSAN: non-NULL argument";
+ case ubsan_nonnull_return:
+ return "UBSAN: non-NULL return";
+ case ubsan_out_of_bounds:
+ return "UBSAN: array index out of bounds";
+ case ubsan_pointer_overflow:
+ return "UBSAN: pointer overflow";
+ case ubsan_shift_out_of_bounds:
+ return "UBSAN: shift out of bounds";
+ case ubsan_sub_overflow:
+ return "UBSAN: subtraction overflow";
+ case ubsan_type_mismatch:
+ return "UBSAN: type mismatch";
+ case ubsan_alignment_assumption:
+ return "UBSAN: alignment assumption";
+ case ubsan_vla_bound_not_positive:
+ return "UBSAN: VLA bounds not positive";
+ default:
+ return "UBSAN: unknown failure";
+ }
+}
+
+#else
static const char * const type_check_kinds[] = {
"load of",
"store to",
@@ -384,3 +436,5 @@ void __ubsan_handle_alignment_assumption(void *_data, unsigned long ptr,
ubsan_epilogue();
}
EXPORT_SYMBOL(__ubsan_handle_alignment_assumption);
+
+#endif /* !CONFIG_UBSAN_TRAP */
diff --git a/lib/ubsan.h b/lib/ubsan.h
index 9a0b71c5ff9f..9c7f00f550f5 100644
--- a/lib/ubsan.h
+++ b/lib/ubsan.h
@@ -2,6 +2,34 @@
#ifndef _LIB_UBSAN_H
#define _LIB_UBSAN_H
+enum ubsan_checks {
+ ubsan_add_overflow,
+ ubsan_builtin_unreachable,
+ ubsan_cfi_check_fail,
+ ubsan_divrem_overflow,
+ ubsan_dynamic_type_cache_miss,
+ ubsan_float_cast_overflow,
+ ubsan_function_type_mismatch,
+ ubsan_implicit_conversion,
+ ubsan_invalid_builtin,
+ ubsan_invalid_objc_cast,
+ ubsan_load_invalid_value,
+ ubsan_missing_return,
+ ubsan_mul_overflow,
+ ubsan_negate_overflow,
+ ubsan_nullability_arg,
+ ubsan_nullability_return,
+ ubsan_nonnull_arg,
+ ubsan_nonnull_return,
+ ubsan_out_of_bounds,
+ ubsan_pointer_overflow,
+ ubsan_shift_out_of_bounds,
+ ubsan_sub_overflow,
+ ubsan_type_mismatch,
+ ubsan_alignment_assumption,
+ ubsan_vla_bound_not_positive,
+};
+
enum {
type_kind_int = 0,
type_kind_float = 1,
--
2.34.1
On 2/3/2023 4:06 AM, Kees Cook wrote:
> On arm64, Clang encodes the UBSAN check type in the esr. Extract this
> and actually report UBSAN traps with some specificity when building with
> CONFIG_UBSAN_TRAP. Before:
>
> Internal error: BRK handler: 00000000f20003e8 [#1] PREEMPT SMP
>
> After:
>
> Internal error: UBSAN: shift out of bounds: 00000000f2005514 [#1] PREEMPT SMP
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: John Stultz <jstultz@google.com>
> Cc: Yongqin Liu <yongqin.liu@linaro.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Andrey Konovalov <andreyknvl@gmail.com>
> Cc: Marco Elver <elver@google.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>
Thanks for handling this,,it will really help in decoding abstracted BRK
handler errors to some extent.
Acked-by: Mukesh Ojha <quic_mojha@quicinc.com>
-Mukesh
> ---
> arch/arm64/include/asm/brk-imm.h | 2 ++
> arch/arm64/kernel/traps.c | 21 +++++++++++++
> include/linux/ubsan.h | 9 ++++++
> lib/Makefile | 2 --
> lib/ubsan.c | 54 ++++++++++++++++++++++++++++++++
> lib/ubsan.h | 28 +++++++++++++++++
> 6 files changed, 114 insertions(+), 2 deletions(-)
> create mode 100644 include/linux/ubsan.h
>
> diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
> index 6e000113e508..3f0f0d03268b 100644
> --- a/arch/arm64/include/asm/brk-imm.h
> +++ b/arch/arm64/include/asm/brk-imm.h
> @@ -28,6 +28,8 @@
> #define BUG_BRK_IMM 0x800
> #define KASAN_BRK_IMM 0x900
> #define KASAN_BRK_MASK 0x0ff
> +#define UBSAN_BRK_IMM 0x5500
> +#define UBSAN_BRK_MASK 0x00ff
>
> #define CFI_BRK_IMM_TARGET GENMASK(4, 0)
> #define CFI_BRK_IMM_TYPE GENMASK(9, 5)
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 4c0caa589e12..87f42eb1c950 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -26,6 +26,7 @@
> #include <linux/syscalls.h>
> #include <linux/mm_types.h>
> #include <linux/kasan.h>
> +#include <linux/ubsan.h>
> #include <linux/cfi.h>
>
> #include <asm/atomic.h>
> @@ -1074,6 +1075,19 @@ static struct break_hook kasan_break_hook = {
> };
> #endif
>
> +#ifdef CONFIG_UBSAN_TRAP
> +static int ubsan_handler(struct pt_regs *regs, unsigned long esr)
> +{
> + die(report_ubsan_failure(regs, esr & UBSAN_BRK_MASK), regs, esr);
> + return DBG_HOOK_HANDLED;
> +}
> +
> +static struct break_hook ubsan_break_hook = {
> + .fn = ubsan_handler,
> + .imm = UBSAN_BRK_IMM,
> + .mask = UBSAN_BRK_MASK,
> +};
> +#endif
>
> #define esr_comment(esr) ((esr) & ESR_ELx_BRK64_ISS_COMMENT_MASK)
>
> @@ -1091,6 +1105,10 @@ int __init early_brk64(unsigned long addr, unsigned long esr,
> #ifdef CONFIG_KASAN_SW_TAGS
> if ((esr_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
> return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
> +#endif
> +#ifdef CONFIG_UBSAN_TRAP
> + if ((esr_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM)
> + return ubsan_handler(regs, esr) != DBG_HOOK_HANDLED;
> #endif
> return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
> }
> @@ -1104,6 +1122,9 @@ void __init trap_init(void)
> register_kernel_break_hook(&fault_break_hook);
> #ifdef CONFIG_KASAN_SW_TAGS
> register_kernel_break_hook(&kasan_break_hook);
> +#endif
> +#ifdef CONFIG_UBSAN_TRAP
> + register_kernel_break_hook(&ubsan_break_hook);
> #endif
> debug_traps_init();
> }
> diff --git a/include/linux/ubsan.h b/include/linux/ubsan.h
> new file mode 100644
> index 000000000000..bff7445498de
> --- /dev/null
> +++ b/include/linux/ubsan.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_UBSAN_H
> +#define _LINUX_UBSAN_H
> +
> +#ifdef CONFIG_UBSAN_TRAP
> +const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type);
> +#endif
> +
> +#endif
> diff --git a/lib/Makefile b/lib/Makefile
> index 4d9461bfea42..81b988bf9448 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -340,9 +340,7 @@ quiet_cmd_build_OID_registry = GEN $@
> clean-files += oid_registry_data.c
>
> obj-$(CONFIG_UCS2_STRING) += ucs2_string.o
> -ifneq ($(CONFIG_UBSAN_TRAP),y)
> obj-$(CONFIG_UBSAN) += ubsan.o
> -endif
>
> UBSAN_SANITIZE_ubsan.o := n
> KASAN_SANITIZE_ubsan.o := n
> diff --git a/lib/ubsan.c b/lib/ubsan.c
> index 60c7099857a0..98a671ec56e9 100644
> --- a/lib/ubsan.c
> +++ b/lib/ubsan.c
> @@ -18,6 +18,58 @@
>
> #include "ubsan.h"
>
> +#ifdef CONFIG_UBSAN_TRAP
> +const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type)
> +{
> + switch (check_type) {
> + case ubsan_add_overflow:
> + return "UBSAN: addition overflow";
> + case ubsan_builtin_unreachable:
> + return "UBSAN: unreachable code";
> + case ubsan_cfi_check_fail:
> + return "UBSAN: CFI failure";
> + case ubsan_divrem_overflow:
> + return "UBSAN: divide/remainder overflow";
> + case ubsan_function_type_mismatch:
> + return "UBSAN: function type mismatch";
> + case ubsan_implicit_conversion:
> + return "UBSAN: implicit conversion";
> + case ubsan_invalid_builtin:
> + return "UBSAN: invalid builtin";
> + case ubsan_invalid_objc_cast:
> + return "UBSAN: invalid object cast";
> + case ubsan_load_invalid_value:
> + return "UBSAN: loading invalid value";
> + case ubsan_missing_return:
> + return "UBSAN: missing return";
> + case ubsan_mul_overflow:
> + return "UBSAN: multiplication overflow";
> + case ubsan_negate_overflow:
> + return "UBSAN: negation overflow";
> + case ubsan_nonnull_arg:
> + return "UBSAN: non-NULL argument";
> + case ubsan_nonnull_return:
> + return "UBSAN: non-NULL return";
> + case ubsan_out_of_bounds:
> + return "UBSAN: array index out of bounds";
> + case ubsan_pointer_overflow:
> + return "UBSAN: pointer overflow";
> + case ubsan_shift_out_of_bounds:
> + return "UBSAN: shift out of bounds";
> + case ubsan_sub_overflow:
> + return "UBSAN: subtraction overflow";
> + case ubsan_type_mismatch:
> + return "UBSAN: type mismatch";
> + case ubsan_alignment_assumption:
> + return "UBSAN: alignment assumption";
> + case ubsan_vla_bound_not_positive:
> + return "UBSAN: VLA bounds not positive";
> + default:
> + return "UBSAN: unknown failure";
> + }
> +}
> +
> +#else
> static const char * const type_check_kinds[] = {
> "load of",
> "store to",
> @@ -384,3 +436,5 @@ void __ubsan_handle_alignment_assumption(void *_data, unsigned long ptr,
> ubsan_epilogue();
> }
> EXPORT_SYMBOL(__ubsan_handle_alignment_assumption);
> +
> +#endif /* !CONFIG_UBSAN_TRAP */
> diff --git a/lib/ubsan.h b/lib/ubsan.h
> index 9a0b71c5ff9f..9c7f00f550f5 100644
> --- a/lib/ubsan.h
> +++ b/lib/ubsan.h
> @@ -2,6 +2,34 @@
> #ifndef _LIB_UBSAN_H
> #define _LIB_UBSAN_H
>
> +enum ubsan_checks {
> + ubsan_add_overflow,
> + ubsan_builtin_unreachable,
> + ubsan_cfi_check_fail,
> + ubsan_divrem_overflow,
> + ubsan_dynamic_type_cache_miss,
> + ubsan_float_cast_overflow,
> + ubsan_function_type_mismatch,
> + ubsan_implicit_conversion,
> + ubsan_invalid_builtin,
> + ubsan_invalid_objc_cast,
> + ubsan_load_invalid_value,
> + ubsan_missing_return,
> + ubsan_mul_overflow,
> + ubsan_negate_overflow,
> + ubsan_nullability_arg,
> + ubsan_nullability_return,
> + ubsan_nonnull_arg,
> + ubsan_nonnull_return,
> + ubsan_out_of_bounds,
> + ubsan_pointer_overflow,
> + ubsan_shift_out_of_bounds,
> + ubsan_sub_overflow,
> + ubsan_type_mismatch,
> + ubsan_alignment_assumption,
> + ubsan_vla_bound_not_positive,
> +};
> +
> enum {
> type_kind_int = 0,
> type_kind_float = 1,
On Thu, 2 Feb 2023 at 23:37, Kees Cook <keescook@chromium.org> wrote:
>
> On arm64, Clang encodes the UBSAN check type in the esr. Extract this
> and actually report UBSAN traps with some specificity when building with
> CONFIG_UBSAN_TRAP. Before:
>
> Internal error: BRK handler: 00000000f20003e8 [#1] PREEMPT SMP
>
> After:
>
> Internal error: UBSAN: shift out of bounds: 00000000f2005514 [#1] PREEMPT SMP
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: John Stultz <jstultz@google.com>
> Cc: Yongqin Liu <yongqin.liu@linaro.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Andrey Konovalov <andreyknvl@gmail.com>
> Cc: Marco Elver <elver@google.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
One nit below
> ---
> arch/arm64/include/asm/brk-imm.h | 2 ++
> arch/arm64/kernel/traps.c | 21 +++++++++++++
> include/linux/ubsan.h | 9 ++++++
> lib/Makefile | 2 --
> lib/ubsan.c | 54 ++++++++++++++++++++++++++++++++
> lib/ubsan.h | 28 +++++++++++++++++
> 6 files changed, 114 insertions(+), 2 deletions(-)
> create mode 100644 include/linux/ubsan.h
>
> diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
> index 6e000113e508..3f0f0d03268b 100644
> --- a/arch/arm64/include/asm/brk-imm.h
> +++ b/arch/arm64/include/asm/brk-imm.h
> @@ -28,6 +28,8 @@
> #define BUG_BRK_IMM 0x800
> #define KASAN_BRK_IMM 0x900
> #define KASAN_BRK_MASK 0x0ff
> +#define UBSAN_BRK_IMM 0x5500
> +#define UBSAN_BRK_MASK 0x00ff
>
> #define CFI_BRK_IMM_TARGET GENMASK(4, 0)
> #define CFI_BRK_IMM_TYPE GENMASK(9, 5)
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 4c0caa589e12..87f42eb1c950 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -26,6 +26,7 @@
> #include <linux/syscalls.h>
> #include <linux/mm_types.h>
> #include <linux/kasan.h>
> +#include <linux/ubsan.h>
> #include <linux/cfi.h>
>
> #include <asm/atomic.h>
> @@ -1074,6 +1075,19 @@ static struct break_hook kasan_break_hook = {
> };
> #endif
>
> +#ifdef CONFIG_UBSAN_TRAP
> +static int ubsan_handler(struct pt_regs *regs, unsigned long esr)
> +{
> + die(report_ubsan_failure(regs, esr & UBSAN_BRK_MASK), regs, esr);
> + return DBG_HOOK_HANDLED;
> +}
> +
> +static struct break_hook ubsan_break_hook = {
> + .fn = ubsan_handler,
> + .imm = UBSAN_BRK_IMM,
> + .mask = UBSAN_BRK_MASK,
> +};
> +#endif
>
> #define esr_comment(esr) ((esr) & ESR_ELx_BRK64_ISS_COMMENT_MASK)
>
> @@ -1091,6 +1105,10 @@ int __init early_brk64(unsigned long addr, unsigned long esr,
> #ifdef CONFIG_KASAN_SW_TAGS
> if ((esr_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
> return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
> +#endif
> +#ifdef CONFIG_UBSAN_TRAP
> + if ((esr_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM)
> + return ubsan_handler(regs, esr) != DBG_HOOK_HANDLED;
> #endif
> return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
> }
> @@ -1104,6 +1122,9 @@ void __init trap_init(void)
> register_kernel_break_hook(&fault_break_hook);
> #ifdef CONFIG_KASAN_SW_TAGS
> register_kernel_break_hook(&kasan_break_hook);
> +#endif
> +#ifdef CONFIG_UBSAN_TRAP
> + register_kernel_break_hook(&ubsan_break_hook);
> #endif
> debug_traps_init();
> }
> diff --git a/include/linux/ubsan.h b/include/linux/ubsan.h
> new file mode 100644
> index 000000000000..bff7445498de
> --- /dev/null
> +++ b/include/linux/ubsan.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_UBSAN_H
> +#define _LINUX_UBSAN_H
> +
> +#ifdef CONFIG_UBSAN_TRAP
> +const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type);
> +#endif
> +
> +#endif
> diff --git a/lib/Makefile b/lib/Makefile
> index 4d9461bfea42..81b988bf9448 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -340,9 +340,7 @@ quiet_cmd_build_OID_registry = GEN $@
> clean-files += oid_registry_data.c
>
> obj-$(CONFIG_UCS2_STRING) += ucs2_string.o
> -ifneq ($(CONFIG_UBSAN_TRAP),y)
> obj-$(CONFIG_UBSAN) += ubsan.o
> -endif
>
> UBSAN_SANITIZE_ubsan.o := n
> KASAN_SANITIZE_ubsan.o := n
> diff --git a/lib/ubsan.c b/lib/ubsan.c
> index 60c7099857a0..98a671ec56e9 100644
> --- a/lib/ubsan.c
> +++ b/lib/ubsan.c
> @@ -18,6 +18,58 @@
>
> #include "ubsan.h"
>
> +#ifdef CONFIG_UBSAN_TRAP
> +const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type)
> +{
> + switch (check_type) {
> + case ubsan_add_overflow:
> + return "UBSAN: addition overflow";
It would be nice if we could avoid duplicating this UBSAN: prefix 21x times.
> + case ubsan_builtin_unreachable:
> + return "UBSAN: unreachable code";
> + case ubsan_cfi_check_fail:
> + return "UBSAN: CFI failure";
> + case ubsan_divrem_overflow:
> + return "UBSAN: divide/remainder overflow";
> + case ubsan_function_type_mismatch:
> + return "UBSAN: function type mismatch";
> + case ubsan_implicit_conversion:
> + return "UBSAN: implicit conversion";
> + case ubsan_invalid_builtin:
> + return "UBSAN: invalid builtin";
> + case ubsan_invalid_objc_cast:
> + return "UBSAN: invalid object cast";
> + case ubsan_load_invalid_value:
> + return "UBSAN: loading invalid value";
> + case ubsan_missing_return:
> + return "UBSAN: missing return";
> + case ubsan_mul_overflow:
> + return "UBSAN: multiplication overflow";
> + case ubsan_negate_overflow:
> + return "UBSAN: negation overflow";
> + case ubsan_nonnull_arg:
> + return "UBSAN: non-NULL argument";
> + case ubsan_nonnull_return:
> + return "UBSAN: non-NULL return";
> + case ubsan_out_of_bounds:
> + return "UBSAN: array index out of bounds";
> + case ubsan_pointer_overflow:
> + return "UBSAN: pointer overflow";
> + case ubsan_shift_out_of_bounds:
> + return "UBSAN: shift out of bounds";
> + case ubsan_sub_overflow:
> + return "UBSAN: subtraction overflow";
> + case ubsan_type_mismatch:
> + return "UBSAN: type mismatch";
> + case ubsan_alignment_assumption:
> + return "UBSAN: alignment assumption";
> + case ubsan_vla_bound_not_positive:
> + return "UBSAN: VLA bounds not positive";
> + default:
> + return "UBSAN: unknown failure";
> + }
> +}
> +
> +#else
> static const char * const type_check_kinds[] = {
> "load of",
> "store to",
> @@ -384,3 +436,5 @@ void __ubsan_handle_alignment_assumption(void *_data, unsigned long ptr,
> ubsan_epilogue();
> }
> EXPORT_SYMBOL(__ubsan_handle_alignment_assumption);
> +
> +#endif /* !CONFIG_UBSAN_TRAP */
> diff --git a/lib/ubsan.h b/lib/ubsan.h
> index 9a0b71c5ff9f..9c7f00f550f5 100644
> --- a/lib/ubsan.h
> +++ b/lib/ubsan.h
> @@ -2,6 +2,34 @@
> #ifndef _LIB_UBSAN_H
> #define _LIB_UBSAN_H
>
> +enum ubsan_checks {
> + ubsan_add_overflow,
> + ubsan_builtin_unreachable,
> + ubsan_cfi_check_fail,
> + ubsan_divrem_overflow,
> + ubsan_dynamic_type_cache_miss,
> + ubsan_float_cast_overflow,
> + ubsan_function_type_mismatch,
> + ubsan_implicit_conversion,
> + ubsan_invalid_builtin,
> + ubsan_invalid_objc_cast,
> + ubsan_load_invalid_value,
> + ubsan_missing_return,
> + ubsan_mul_overflow,
> + ubsan_negate_overflow,
> + ubsan_nullability_arg,
> + ubsan_nullability_return,
> + ubsan_nonnull_arg,
> + ubsan_nonnull_return,
> + ubsan_out_of_bounds,
> + ubsan_pointer_overflow,
> + ubsan_shift_out_of_bounds,
> + ubsan_sub_overflow,
> + ubsan_type_mismatch,
> + ubsan_alignment_assumption,
> + ubsan_vla_bound_not_positive,
> +};
> +
> enum {
> type_kind_int = 0,
> type_kind_float = 1,
> --
> 2.34.1
>
On Fri, Feb 03, 2023 at 12:34:38PM +0100, Ard Biesheuvel wrote:
> [...]
> > + switch (check_type) {
> > + case ubsan_add_overflow:
> > + return "UBSAN: addition overflow";
>
> It would be nice if we could avoid duplicating this UBSAN: prefix 21x times.
Yeah, agreed. I didn't see a way around it without weirdness or loss of
details. die() takes a string not a format string, so without really
odd shenanigans (re-write a single string in memory?) this is the most
straight forward. One idea, though, is to drop all the strings that'll
never happen -- i.e. wrap individual case statements in the associated
CONFIG_UBSAN_foo. I already dropped several strings that were never
implemented in the kernel's UBSAN. I could be even more picky. I'll try
this in v2...
--
Kees Cook
On Thu, Feb 02, 2023 at 10:36:57PM +0000, Kees Cook wrote:
> On arm64, Clang encodes the UBSAN check type in the esr. Extract this
> and actually report UBSAN traps with some specificity when building with
> CONFIG_UBSAN_TRAP. Before:
Really minor nit, but could we mention CONFIG_UBSAN_TRAP at the start, e.g.
change that first sentence to:
| When CONFIG_UBSAN_TRAP=y on arm64, Clang encodes the UBSAN check type into a
| BRK immediate, which is reported in the ESR.
I was initially confused since I was aware that there are usually direct
function calls for the reporting.
Other than that, this looks fine to me, and regardless of the above:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> Internal error: BRK handler: 00000000f20003e8 [#1] PREEMPT SMP
>
> After:
>
> Internal error: UBSAN: shift out of bounds: 00000000f2005514 [#1] PREEMPT SMP
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: John Stultz <jstultz@google.com>
> Cc: Yongqin Liu <yongqin.liu@linaro.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Andrey Konovalov <andreyknvl@gmail.com>
> Cc: Marco Elver <elver@google.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/arm64/include/asm/brk-imm.h | 2 ++
> arch/arm64/kernel/traps.c | 21 +++++++++++++
> include/linux/ubsan.h | 9 ++++++
> lib/Makefile | 2 --
> lib/ubsan.c | 54 ++++++++++++++++++++++++++++++++
> lib/ubsan.h | 28 +++++++++++++++++
> 6 files changed, 114 insertions(+), 2 deletions(-)
> create mode 100644 include/linux/ubsan.h
>
> diff --git a/arch/arm64/include/asm/brk-imm.h b/arch/arm64/include/asm/brk-imm.h
> index 6e000113e508..3f0f0d03268b 100644
> --- a/arch/arm64/include/asm/brk-imm.h
> +++ b/arch/arm64/include/asm/brk-imm.h
> @@ -28,6 +28,8 @@
> #define BUG_BRK_IMM 0x800
> #define KASAN_BRK_IMM 0x900
> #define KASAN_BRK_MASK 0x0ff
> +#define UBSAN_BRK_IMM 0x5500
> +#define UBSAN_BRK_MASK 0x00ff
>
> #define CFI_BRK_IMM_TARGET GENMASK(4, 0)
> #define CFI_BRK_IMM_TYPE GENMASK(9, 5)
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 4c0caa589e12..87f42eb1c950 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -26,6 +26,7 @@
> #include <linux/syscalls.h>
> #include <linux/mm_types.h>
> #include <linux/kasan.h>
> +#include <linux/ubsan.h>
> #include <linux/cfi.h>
>
> #include <asm/atomic.h>
> @@ -1074,6 +1075,19 @@ static struct break_hook kasan_break_hook = {
> };
> #endif
>
> +#ifdef CONFIG_UBSAN_TRAP
> +static int ubsan_handler(struct pt_regs *regs, unsigned long esr)
> +{
> + die(report_ubsan_failure(regs, esr & UBSAN_BRK_MASK), regs, esr);
> + return DBG_HOOK_HANDLED;
> +}
> +
> +static struct break_hook ubsan_break_hook = {
> + .fn = ubsan_handler,
> + .imm = UBSAN_BRK_IMM,
> + .mask = UBSAN_BRK_MASK,
> +};
> +#endif
>
> #define esr_comment(esr) ((esr) & ESR_ELx_BRK64_ISS_COMMENT_MASK)
>
> @@ -1091,6 +1105,10 @@ int __init early_brk64(unsigned long addr, unsigned long esr,
> #ifdef CONFIG_KASAN_SW_TAGS
> if ((esr_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM)
> return kasan_handler(regs, esr) != DBG_HOOK_HANDLED;
> +#endif
> +#ifdef CONFIG_UBSAN_TRAP
> + if ((esr_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM)
> + return ubsan_handler(regs, esr) != DBG_HOOK_HANDLED;
> #endif
> return bug_handler(regs, esr) != DBG_HOOK_HANDLED;
> }
> @@ -1104,6 +1122,9 @@ void __init trap_init(void)
> register_kernel_break_hook(&fault_break_hook);
> #ifdef CONFIG_KASAN_SW_TAGS
> register_kernel_break_hook(&kasan_break_hook);
> +#endif
> +#ifdef CONFIG_UBSAN_TRAP
> + register_kernel_break_hook(&ubsan_break_hook);
> #endif
> debug_traps_init();
> }
> diff --git a/include/linux/ubsan.h b/include/linux/ubsan.h
> new file mode 100644
> index 000000000000..bff7445498de
> --- /dev/null
> +++ b/include/linux/ubsan.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_UBSAN_H
> +#define _LINUX_UBSAN_H
> +
> +#ifdef CONFIG_UBSAN_TRAP
> +const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type);
> +#endif
> +
> +#endif
> diff --git a/lib/Makefile b/lib/Makefile
> index 4d9461bfea42..81b988bf9448 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -340,9 +340,7 @@ quiet_cmd_build_OID_registry = GEN $@
> clean-files += oid_registry_data.c
>
> obj-$(CONFIG_UCS2_STRING) += ucs2_string.o
> -ifneq ($(CONFIG_UBSAN_TRAP),y)
> obj-$(CONFIG_UBSAN) += ubsan.o
> -endif
>
> UBSAN_SANITIZE_ubsan.o := n
> KASAN_SANITIZE_ubsan.o := n
> diff --git a/lib/ubsan.c b/lib/ubsan.c
> index 60c7099857a0..98a671ec56e9 100644
> --- a/lib/ubsan.c
> +++ b/lib/ubsan.c
> @@ -18,6 +18,58 @@
>
> #include "ubsan.h"
>
> +#ifdef CONFIG_UBSAN_TRAP
> +const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type)
> +{
> + switch (check_type) {
> + case ubsan_add_overflow:
> + return "UBSAN: addition overflow";
> + case ubsan_builtin_unreachable:
> + return "UBSAN: unreachable code";
> + case ubsan_cfi_check_fail:
> + return "UBSAN: CFI failure";
> + case ubsan_divrem_overflow:
> + return "UBSAN: divide/remainder overflow";
> + case ubsan_function_type_mismatch:
> + return "UBSAN: function type mismatch";
> + case ubsan_implicit_conversion:
> + return "UBSAN: implicit conversion";
> + case ubsan_invalid_builtin:
> + return "UBSAN: invalid builtin";
> + case ubsan_invalid_objc_cast:
> + return "UBSAN: invalid object cast";
> + case ubsan_load_invalid_value:
> + return "UBSAN: loading invalid value";
> + case ubsan_missing_return:
> + return "UBSAN: missing return";
> + case ubsan_mul_overflow:
> + return "UBSAN: multiplication overflow";
> + case ubsan_negate_overflow:
> + return "UBSAN: negation overflow";
> + case ubsan_nonnull_arg:
> + return "UBSAN: non-NULL argument";
> + case ubsan_nonnull_return:
> + return "UBSAN: non-NULL return";
> + case ubsan_out_of_bounds:
> + return "UBSAN: array index out of bounds";
> + case ubsan_pointer_overflow:
> + return "UBSAN: pointer overflow";
> + case ubsan_shift_out_of_bounds:
> + return "UBSAN: shift out of bounds";
> + case ubsan_sub_overflow:
> + return "UBSAN: subtraction overflow";
> + case ubsan_type_mismatch:
> + return "UBSAN: type mismatch";
> + case ubsan_alignment_assumption:
> + return "UBSAN: alignment assumption";
> + case ubsan_vla_bound_not_positive:
> + return "UBSAN: VLA bounds not positive";
> + default:
> + return "UBSAN: unknown failure";
> + }
> +}
> +
> +#else
> static const char * const type_check_kinds[] = {
> "load of",
> "store to",
> @@ -384,3 +436,5 @@ void __ubsan_handle_alignment_assumption(void *_data, unsigned long ptr,
> ubsan_epilogue();
> }
> EXPORT_SYMBOL(__ubsan_handle_alignment_assumption);
> +
> +#endif /* !CONFIG_UBSAN_TRAP */
> diff --git a/lib/ubsan.h b/lib/ubsan.h
> index 9a0b71c5ff9f..9c7f00f550f5 100644
> --- a/lib/ubsan.h
> +++ b/lib/ubsan.h
> @@ -2,6 +2,34 @@
> #ifndef _LIB_UBSAN_H
> #define _LIB_UBSAN_H
>
> +enum ubsan_checks {
> + ubsan_add_overflow,
> + ubsan_builtin_unreachable,
> + ubsan_cfi_check_fail,
> + ubsan_divrem_overflow,
> + ubsan_dynamic_type_cache_miss,
> + ubsan_float_cast_overflow,
> + ubsan_function_type_mismatch,
> + ubsan_implicit_conversion,
> + ubsan_invalid_builtin,
> + ubsan_invalid_objc_cast,
> + ubsan_load_invalid_value,
> + ubsan_missing_return,
> + ubsan_mul_overflow,
> + ubsan_negate_overflow,
> + ubsan_nullability_arg,
> + ubsan_nullability_return,
> + ubsan_nonnull_arg,
> + ubsan_nonnull_return,
> + ubsan_out_of_bounds,
> + ubsan_pointer_overflow,
> + ubsan_shift_out_of_bounds,
> + ubsan_sub_overflow,
> + ubsan_type_mismatch,
> + ubsan_alignment_assumption,
> + ubsan_vla_bound_not_positive,
> +};
> +
> enum {
> type_kind_int = 0,
> type_kind_float = 1,
> --
> 2.34.1
>
On Fri, Feb 03, 2023 at 09:58:26AM +0000, Mark Rutland wrote: > On Thu, Feb 02, 2023 at 10:36:57PM +0000, Kees Cook wrote: > > On arm64, Clang encodes the UBSAN check type in the esr. Extract this > > and actually report UBSAN traps with some specificity when building with > > CONFIG_UBSAN_TRAP. Before: > > Really minor nit, but could we mention CONFIG_UBSAN_TRAP at the start, e.g. > change that first sentence to: > > | When CONFIG_UBSAN_TRAP=y on arm64, Clang encodes the UBSAN check type into a > | BRK immediate, which is reported in the ESR. > > I was initially confused since I was aware that there are usually direct > function calls for the reporting. Sure, I can rearrange the commit log. > > Other than that, this looks fine to me, and regardless of the above: > > Acked-by: Mark Rutland <mark.rutland@arm.com> Thanks! -Kees -- Kees Cook
On Thu, Feb 2, 2023 at 2:37 PM Kees Cook <keescook@chromium.org> wrote: > > On arm64, Clang encodes the UBSAN check type in the esr. Extract this > and actually report UBSAN traps with some specificity when building with > CONFIG_UBSAN_TRAP. Before: > > Internal error: BRK handler: 00000000f20003e8 [#1] PREEMPT SMP > > After: > > Internal error: UBSAN: shift out of bounds: 00000000f2005514 [#1] PREEMPT SMP > This is really great ! When I brought up the opaque error messages just a few hours ago, I didn't imagine you'd have a solution so quickly! I really appreciate your lightning fast effort here! Really, thanks so much! I think this will have a big impact for folks running into these new bug catchers. -john
© 2016 - 2025 Red Hat, Inc.