Skip to contentSkip to conversation

arm64: Support Clang UBSAN trap codes for better reporting

PATCH Superseded Kees Cook 5 participants 7 messages Feb 2, 2023 – Feb 3, 2023

Subsystems (3)
Library code arm64 Ubsan

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 arm64: Support Clang UBSAN trap codes for better reporting

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 <[email protected]>
Cc Will Deacon <[email protected]>
Cc Mark Rutland <[email protected]>
Cc John Stultz <[email protected]>
Cc Yongqin Liu <[email protected]>
Cc Sami Tolvanen <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Yury Norov <[email protected]>
Cc Andrey Konovalov <[email protected]>
Cc Marco Elver <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
 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
arch/arm64/include/asm/brk-imm.h +2 -0
index 6e000113e508..3f0f0d03268b 100644
@@ -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)
arch/arm64/kernel/traps.c +21 -0
index 4c0caa589e12..87f42eb1c950 100644
@@ -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();
 }
include/linux/ubsan.h new file +9 -0
new file mode 100644
index 000000000000..bff7445498de
@@ -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
lib/Makefile +0 -2
index 4d9461bfea42..81b988bf9448 100644
@@ -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
lib/ubsan.c +54 -0

63 lines not on this page · raw message

lib/ubsan.h +28 -0
index 9a0b71c5ff9f..9c7f00f550f5 100644
@@ -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,
Signature · 1 line
2.34.1
#1 lore raw
Quoted 11 lines · Kees Cook

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

reviewer Acked-by on v1 1/1
#2 lore raw
In reply to Kees Cook
Quoted 3 lines · Kees Cook

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 <[email protected]>

Mark.

Quoted 235 lines
  Internal error: BRK handler: 00000000f20003e8 [#1] PREEMPT SMP

After:

  Internal error: UBSAN: shift out of bounds: 00000000f2005514 [#1] PREEMPT SMP
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Mark Rutland <[email protected]>
Cc John Stultz <[email protected]>
Cc Yongqin Liu <[email protected]>
Cc Sami Tolvanen <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Yury Norov <[email protected]>
Cc Andrey Konovalov <[email protected]>
Cc Marco Elver <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
---
 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
arch/arm64/include/asm/brk-imm.h +2 -0
index 6e000113e508..3f0f0d03268b 100644
@@ -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)
arch/arm64/kernel/traps.c +21 -0
index 4c0caa589e12..87f42eb1c950 100644
@@ -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();
 }
include/linux/ubsan.h new file +9 -0
new file mode 100644
index 000000000000..bff7445498de
@@ -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
lib/Makefile +0 -2
index 4d9461bfea42..81b988bf9448 100644
@@ -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
lib/ubsan.c +54 -0
index 60c7099857a0..98a671ec56e9 100644
@@ -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 */
lib/ubsan.h +28 -0
index 9a0b71c5ff9f..9c7f00f550f5 100644
@@ -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,
Signature · 1 line
2.34.1
author
#3 lore raw
Quoted 5 lines · Mark Rutland
Quoted 3 lines · Kees Cook

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:

Quoted 8 lines

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.

Quoted 4 lines

Other than that, this looks fine to me, and regardless of the above:

Acked-by Mark Rutland <[email protected]>

Thanks!

-Kees

Signature · 1 line
Kees Cook
Reviewed-by on v1 1/1
#4 lore raw
In reply to Kees Cook
Quoted 11 lines · Kees Cook

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
Quoted 13 lines
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Mark Rutland <[email protected]>
Cc John Stultz <[email protected]>
Cc Yongqin Liu <[email protected]>
Cc Sami Tolvanen <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Yury Norov <[email protected]>
Cc Andrey Konovalov <[email protected]>
Cc Marco Elver <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>
Reviewed-by Ard Biesheuvel <[email protected]>

One nit below

Quoted 110 lines
---
 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
arch/arm64/include/asm/brk-imm.h +2 -0
index 6e000113e508..3f0f0d03268b 100644
@@ -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)
arch/arm64/kernel/traps.c +21 -0
index 4c0caa589e12..87f42eb1c950 100644
@@ -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();
 }
include/linux/ubsan.h new file +9 -0
new file mode 100644
index 000000000000..bff7445498de
@@ -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
lib/Makefile +0 -2
index 4d9461bfea42..81b988bf9448 100644
@@ -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
lib/ubsan.c +0 -0
index 60c7099857a0..98a671ec56e9 100644

lib/ubsan.c line 26

Quoted 9 lines
lib/ubsan.c +6 -0
@@ -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.

Quoted 97 lines
lib/ubsan.c +48 -0
+       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 */
lib/ubsan.h +28 -1
index 9a0b71c5ff9f..9c7f00f550f5 100644
@@ -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

author
#5 lore raw
Quoted 6 lines · Ard Biesheuvel

[...]

lib/ubsan.c line 26

Quoted 3 lines
lib/ubsan.c +3 -0
+       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...

Signature · 1 line
Kees Cook
Acked-by on v1 1/1
#6 lore raw
In reply to Kees Cook
Quoted 10 lines · Kees Cook

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
Quoted 13 lines
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Mark Rutland <[email protected]>
Cc John Stultz <[email protected]>
Cc Yongqin Liu <[email protected]>
Cc Sami Tolvanen <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Cc Yury Norov <[email protected]>
Cc Andrey Konovalov <[email protected]>
Cc Marco Elver <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Kees Cook <[email protected]>

Thanks for handling this,,it will really help in decoding abstracted BRK handler errors to some extent.

Acked-by Mukesh Ojha <[email protected]>

-Mukesh

Quoted 213 lines
---
  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

arch/arm64/include/asm/brk-imm.h +2 -0
index 6e000113e508..3f0f0d03268b 100644
@@ -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)
arch/arm64/kernel/traps.c +21 -0
index 4c0caa589e12..87f42eb1c950 100644
@@ -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();
  }
include/linux/ubsan.h new file +9 -0
new file mode 100644
index 000000000000..bff7445498de
@@ -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
lib/Makefile +0 -2
index 4d9461bfea42..81b988bf9448 100644
@@ -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
lib/ubsan.c +54 -0
index 60c7099857a0..98a671ec56e9 100644
@@ -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 */
lib/ubsan.h +28 -0
index 9a0b71c5ff9f..9c7f00f550f5 100644
@@ -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,