[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH RFC] xen: add undefined behaviour sanitizer



1. Introduce Kconfig UBSAN option.
2. Import and adapt a bunch of hooks from Linux kernel with as little
   modification as possible.
3. Provide mechanism to filter out unwanted files.

This is x86 only at the moment. It should be easy to make it work on
arm, but I don't have the test environment to figure out what files
need to be filtered out.

Currently two files are filtered out:

1. arch/x86/setup.o -- xen crashes very early without any output
2. drivers/acpi/tables/tbutils.o -- it has requirement to access
   misaligned memory before current is properly set up.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>

Though this is not going to be in 4.10 I think it would be valuable to
fix the issues it reports.

And example output on x86:
https://paste.debian.net/988890/
---
 xen/Kconfig.debug                |   8 +
 xen/Rules.mk                     |   4 +
 xen/arch/x86/Makefile            |   2 +
 xen/common/Makefile              |   1 +
 xen/common/ubsan/Makefile        |   1 +
 xen/common/ubsan/ubsan.c         | 486 +++++++++++++++++++++++++++++++++++++++
 xen/common/ubsan/ubsan.h         |  84 +++++++
 xen/drivers/acpi/tables/Makefile |   2 +
 xen/include/xen/sched.h          |   3 +
 9 files changed, 591 insertions(+)
 create mode 100644 xen/common/ubsan/Makefile
 create mode 100644 xen/common/ubsan/ubsan.c
 create mode 100644 xen/common/ubsan/ubsan.h

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 195d504147..e63b533459 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -121,6 +121,14 @@ config SCRUB_DEBUG
          Verify that pages that need to be scrubbed before being allocated to
          a guest are indeed scrubbed.
 
+config UBSAN
+       bool "Undefined behaviour sanitizer"
+       depends on X86
+       ---help---
+         Enable undefined behaviour sanitizer.
+
+         If unsure, say N here.
+
 endif # DEBUG || EXPERT
 
 endmenu
diff --git a/xen/Rules.mk b/xen/Rules.mk
index cafc67b86e..2659f8a4d1 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -119,6 +119,10 @@ ifeq ($(CONFIG_GCOV),y)
 $(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += 
-fprofile-arcs -ftest-coverage
 endif
 
+ifeq ($(CONFIG_UBSAN),y)
+$(filter-out %.init.o $(noubsan-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS 
+= -fsanitize=undefined
+endif
+
 ifeq ($(CONFIG_LTO),y)
 CFLAGS += -flto
 LDFLAGS-$(clang) += -plugin LLVMgold.so
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d5d58a205e..648f70799f 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -69,6 +69,8 @@ obj-y += hpet.o
 obj-y += vm_event.o
 obj-y += xstate.o
 
+noubsan-y += setup.o
+
 x86_emulate.o: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 
 efi-y := $(shell if [ ! -r $(BASEDIR)/include/xen/compile.h -o \
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 39e2614546..66cc2c8995 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -75,6 +75,7 @@ tmem-$(CONFIG_COMPAT) += compat/tmem_xen.o
 obj-$(CONFIG_TMEM) += $(tmem-y)
 
 subdir-$(CONFIG_GCOV) += gcov
+subdir-$(CONFIG_UBSAN) += ubsan
 
 subdir-y += libelf
 subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt
diff --git a/xen/common/ubsan/Makefile b/xen/common/ubsan/Makefile
new file mode 100644
index 0000000000..e6b85ea187
--- /dev/null
+++ b/xen/common/ubsan/Makefile
@@ -0,0 +1 @@
+obj-y += ubsan.o
diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
new file mode 100644
index 0000000000..aa359e712c
--- /dev/null
+++ b/xen/common/ubsan/ubsan.c
@@ -0,0 +1,486 @@
+/*
+ * UBSAN error reporting functions
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Author: Andrey Ryabinin <ryabinin.a.a@xxxxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Adapt to Xen by:
+ *  Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
+ *  Wei Liu <wei.liu2@xxxxxxxxxx>
+ */
+
+#include <xen/bitops.h>
+#include <xen/kernel.h>
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/sched.h>
+#include <xen/spinlock.h>
+#include <xen/percpu.h>
+
+#define pr_err(...) printk(XENLOG_ERR __VA_ARGS__)
+#define dump_stack dump_execution_state
+#define __noreturn noreturn
+#define CONFIG_ARCH_SUPPORTS_INT128
+#define u64 long long unsigned int
+#define s64 long long int
+
+#include "ubsan.h"
+
+const char *type_check_kinds[] = {
+       "load of",
+       "store to",
+       "reference binding to",
+       "member access within",
+       "member call on",
+       "constructor call on",
+       "downcast of",
+       "downcast of"
+};
+
+#define REPORTED_BIT 31
+
+#if (BITS_PER_LONG == 64) && defined(__BIG_ENDIAN)
+#define COLUMN_MASK (~(1U << REPORTED_BIT))
+#define LINE_MASK   (~0U)
+#else
+#define COLUMN_MASK   (~0U)
+#define LINE_MASK (~(1U << REPORTED_BIT))
+#endif
+
+#define VALUE_LENGTH 40
+
+static bool was_reported(struct source_location *location)
+{
+       return test_and_set_bit(REPORTED_BIT, &location->reported);
+}
+
+static void print_source_location(const char *prefix,
+                               struct source_location *loc)
+{
+       pr_err("%s %s:%d:%d\n", prefix, loc->file_name,
+               loc->line & LINE_MASK, loc->column & COLUMN_MASK);
+}
+
+static bool suppress_report(struct source_location *loc)
+{
+       return current->in_ubsan || was_reported(loc);
+}
+
+static bool type_is_int(struct type_descriptor *type)
+{
+       return type->type_kind == type_kind_int;
+}
+
+static bool type_is_signed(struct type_descriptor *type)
+{
+       WARN_ON(!type_is_int(type));
+       return  type->type_info & 1;
+}
+
+static unsigned type_bit_width(struct type_descriptor *type)
+{
+       return 1 << (type->type_info >> 1);
+}
+
+static bool is_inline_int(struct type_descriptor *type)
+{
+       unsigned inline_bits = sizeof(unsigned long)*8;
+       unsigned bits = type_bit_width(type);
+
+       WARN_ON(!type_is_int(type));
+
+       return bits <= inline_bits;
+}
+
+static s_max get_signed_val(struct type_descriptor *type, unsigned long val)
+{
+       if (is_inline_int(type)) {
+               unsigned extra_bits = sizeof(s_max)*8 - type_bit_width(type);
+               return ((s_max)val) << extra_bits >> extra_bits;
+       }
+
+       if (type_bit_width(type) == 64)
+               return *(s64 *)val;
+
+       return *(s_max *)val;
+}
+
+static bool val_is_negative(struct type_descriptor *type, unsigned long val)
+{
+       return type_is_signed(type) && get_signed_val(type, val) < 0;
+}
+
+static u_max get_unsigned_val(struct type_descriptor *type, unsigned long val)
+{
+       if (is_inline_int(type))
+               return val;
+
+       if (type_bit_width(type) == 64)
+               return *(u64 *)val;
+
+       return *(u_max *)val;
+}
+
+static void val_to_string(char *str, size_t size, struct type_descriptor *type,
+       unsigned long value)
+{
+       if (type_is_int(type)) {
+               if (type_bit_width(type) == 128) {
+#if defined(CONFIG_ARCH_SUPPORTS_INT128) && defined(__SIZEOF_INT128__)
+                       u_max val = get_unsigned_val(type, value);
+
+                       scnprintf(str, size, "0x%08x%08x%08x%08x",
+                               (u32)(val >> 96),
+                               (u32)(val >> 64),
+                               (u32)(val >> 32),
+                               (u32)(val));
+#else
+                       WARN_ON(1);
+#endif
+               } else if (type_is_signed(type)) {
+                       scnprintf(str, size, "%lld",
+                               (s64)get_signed_val(type, value));
+               } else {
+                       scnprintf(str, size, "%llu",
+                               (u64)get_unsigned_val(type, value));
+               }
+       }
+}
+
+static bool location_is_valid(struct source_location *loc)
+{
+       return loc->file_name != NULL;
+}
+
+static DEFINE_SPINLOCK(report_lock);
+
+static void ubsan_prologue(struct source_location *location,
+                       unsigned long *flags)
+{
+       current->in_ubsan++;
+       spin_lock_irqsave(&report_lock, *flags);
+
+       pr_err("========================================"
+               "========================================\n");
+       print_source_location("UBSAN: Undefined behaviour in", location);
+}
+
+static void ubsan_epilogue(unsigned long *flags)
+{
+       dump_stack();
+       pr_err("========================================"
+               "========================================\n");
+       spin_unlock_irqrestore(&report_lock, *flags);
+       current->in_ubsan--;
+}
+
+static void handle_overflow(struct overflow_data *data, unsigned long lhs,
+                       unsigned long rhs, char op)
+{
+
+       struct type_descriptor *type = data->type;
+       unsigned long flags;
+       char lhs_val_str[VALUE_LENGTH];
+       char rhs_val_str[VALUE_LENGTH];
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       val_to_string(lhs_val_str, sizeof(lhs_val_str), type, lhs);
+       val_to_string(rhs_val_str, sizeof(rhs_val_str), type, rhs);
+       pr_err("%s integer overflow:\n",
+               type_is_signed(type) ? "signed" : "unsigned");
+       pr_err("%s %c %s cannot be represented in type %s\n",
+               lhs_val_str,
+               op,
+               rhs_val_str,
+               type->type_name);
+
+       ubsan_epilogue(&flags);
+}
+
+void __ubsan_handle_add_overflow(struct overflow_data *data,
+                               unsigned long lhs,
+                               unsigned long rhs)
+{
+
+       handle_overflow(data, lhs, rhs, '+');
+}
+EXPORT_SYMBOL(__ubsan_handle_add_overflow);
+
+void __ubsan_handle_sub_overflow(struct overflow_data *data,
+                               unsigned long lhs,
+                               unsigned long rhs)
+{
+       handle_overflow(data, lhs, rhs, '-');
+}
+EXPORT_SYMBOL(__ubsan_handle_sub_overflow);
+
+void __ubsan_handle_mul_overflow(struct overflow_data *data,
+                               unsigned long lhs,
+                               unsigned long rhs)
+{
+       handle_overflow(data, lhs, rhs, '*');
+}
+EXPORT_SYMBOL(__ubsan_handle_mul_overflow);
+
+void __ubsan_handle_negate_overflow(struct overflow_data *data,
+                               unsigned long old_val)
+{
+       unsigned long flags;
+       char old_val_str[VALUE_LENGTH];
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       val_to_string(old_val_str, sizeof(old_val_str), data->type, old_val);
+
+       pr_err("negation of %s cannot be represented in type %s:\n",
+               old_val_str, data->type->type_name);
+
+       ubsan_epilogue(&flags);
+}
+EXPORT_SYMBOL(__ubsan_handle_negate_overflow);
+
+
+void __ubsan_handle_divrem_overflow(struct overflow_data *data,
+                               unsigned long lhs,
+                               unsigned long rhs)
+{
+       unsigned long flags;
+       char rhs_val_str[VALUE_LENGTH];
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       val_to_string(rhs_val_str, sizeof(rhs_val_str), data->type, rhs);
+
+       if (type_is_signed(data->type) && get_signed_val(data->type, rhs) == -1)
+               pr_err("division of %s by -1 cannot be represented in type 
%s\n",
+                       rhs_val_str, data->type->type_name);
+       else
+               pr_err("division by zero\n");
+
+       ubsan_epilogue(&flags);
+}
+EXPORT_SYMBOL(__ubsan_handle_divrem_overflow);
+
+static void handle_null_ptr_deref(struct type_mismatch_data *data)
+{
+       unsigned long flags;
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       pr_err("%s null pointer of type %s\n",
+               type_check_kinds[data->type_check_kind],
+               data->type->type_name);
+
+       ubsan_epilogue(&flags);
+}
+
+static void handle_missaligned_access(struct type_mismatch_data *data,
+                               unsigned long ptr)
+{
+       unsigned long flags;
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       pr_err("%s misaligned address %p for type %s\n",
+               type_check_kinds[data->type_check_kind],
+               (void *)ptr, data->type->type_name);
+       pr_err("which requires %ld byte alignment\n", data->alignment);
+
+       ubsan_epilogue(&flags);
+}
+
+static void handle_object_size_mismatch(struct type_mismatch_data *data,
+                                       unsigned long ptr)
+{
+       unsigned long flags;
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+       pr_err("%s address %pk with insufficient space\n",
+               type_check_kinds[data->type_check_kind],
+               (void *) ptr);
+       pr_err("for an object of type %s\n", data->type->type_name);
+       ubsan_epilogue(&flags);
+}
+
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *data,
+                               unsigned long ptr)
+{
+
+       if (!ptr)
+               handle_null_ptr_deref(data);
+       else if (data->alignment && !IS_ALIGNED(ptr, data->alignment))
+               handle_missaligned_access(data, ptr);
+       else
+               handle_object_size_mismatch(data, ptr);
+}
+EXPORT_SYMBOL(__ubsan_handle_type_mismatch);
+
+void __ubsan_handle_nonnull_return(struct nonnull_return_data *data)
+{
+       unsigned long flags;
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       pr_err("null pointer returned from function declared to never return 
null\n");
+
+       if (location_is_valid(&data->attr_location))
+               print_source_location("returns_nonnull attribute specified in",
+                               &data->attr_location);
+
+       ubsan_epilogue(&flags);
+}
+EXPORT_SYMBOL(__ubsan_handle_nonnull_return);
+
+void __ubsan_handle_vla_bound_not_positive(struct vla_bound_data *data,
+                                       unsigned long bound)
+{
+       unsigned long flags;
+       char bound_str[VALUE_LENGTH];
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       val_to_string(bound_str, sizeof(bound_str), data->type, bound);
+       pr_err("variable length array bound value %s <= 0\n", bound_str);
+
+       ubsan_epilogue(&flags);
+}
+EXPORT_SYMBOL(__ubsan_handle_vla_bound_not_positive);
+
+void __ubsan_handle_out_of_bounds(struct out_of_bounds_data *data,
+                               unsigned long index)
+{
+       unsigned long flags;
+       char index_str[VALUE_LENGTH];
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       val_to_string(index_str, sizeof(index_str), data->index_type, index);
+       pr_err("index %s is out of range for type %s\n", index_str,
+               data->array_type->type_name);
+       ubsan_epilogue(&flags);
+}
+EXPORT_SYMBOL(__ubsan_handle_out_of_bounds);
+
+void __ubsan_handle_shift_out_of_bounds(struct shift_out_of_bounds_data *data,
+                                       unsigned long lhs, unsigned long rhs)
+{
+       unsigned long flags;
+       struct type_descriptor *rhs_type = data->rhs_type;
+       struct type_descriptor *lhs_type = data->lhs_type;
+       char rhs_str[VALUE_LENGTH];
+       char lhs_str[VALUE_LENGTH];
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       val_to_string(rhs_str, sizeof(rhs_str), rhs_type, rhs);
+       val_to_string(lhs_str, sizeof(lhs_str), lhs_type, lhs);
+
+       if (val_is_negative(rhs_type, rhs))
+               pr_err("shift exponent %s is negative\n", rhs_str);
+
+       else if (get_unsigned_val(rhs_type, rhs) >=
+               type_bit_width(lhs_type))
+               pr_err("shift exponent %s is too large for %u-bit type %s\n",
+                       rhs_str,
+                       type_bit_width(lhs_type),
+                       lhs_type->type_name);
+       else if (val_is_negative(lhs_type, lhs))
+               pr_err("left shift of negative value %s\n",
+                       lhs_str);
+       else
+               pr_err("left shift of %s by %s places cannot be"
+                       " represented in type %s\n",
+                       lhs_str, rhs_str,
+                       lhs_type->type_name);
+
+       ubsan_epilogue(&flags);
+}
+EXPORT_SYMBOL(__ubsan_handle_shift_out_of_bounds);
+
+
+void __noreturn
+__ubsan_handle_builtin_unreachable(struct unreachable_data *data)
+{
+       unsigned long flags;
+
+       ubsan_prologue(&data->location, &flags);
+       pr_err("calling __builtin_unreachable()\n");
+       ubsan_epilogue(&flags);
+       panic("can't return from __builtin_unreachable()");
+}
+EXPORT_SYMBOL(__ubsan_handle_builtin_unreachable);
+
+void __ubsan_handle_load_invalid_value(struct invalid_value_data *data,
+                               unsigned long val)
+{
+       unsigned long flags;
+       char val_str[VALUE_LENGTH];
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       val_to_string(val_str, sizeof(val_str), data->type, val);
+
+       pr_err("load of value %s is not a valid value for type %s\n",
+               val_str, data->type->type_name);
+
+       ubsan_epilogue(&flags);
+}
+EXPORT_SYMBOL(__ubsan_handle_load_invalid_value);
+
+void __ubsan_handle_nonnull_arg(struct nonnull_arg_data *data)
+{
+       unsigned long flags;
+
+       if (suppress_report(&data->location))
+               return;
+
+       ubsan_prologue(&data->location, &flags);
+
+       pr_err("null pointer passed as argument %d, declared with nonnull 
attribute\n",
+              data->arg_index);
+
+       if (location_is_valid(&data->attr_location))
+               print_source_location("nonnull attribute declared in ",
+                                     &data->attr_location);
+
+       ubsan_epilogue(&flags);
+}
+EXPORT_SYMBOL(__ubsan_handle_nonnull_arg);
diff --git a/xen/common/ubsan/ubsan.h b/xen/common/ubsan/ubsan.h
new file mode 100644
index 0000000000..b2d18d4a53
--- /dev/null
+++ b/xen/common/ubsan/ubsan.h
@@ -0,0 +1,84 @@
+#ifndef _LIB_UBSAN_H
+#define _LIB_UBSAN_H
+
+enum {
+       type_kind_int = 0,
+       type_kind_float = 1,
+       type_unknown = 0xffff
+};
+
+struct type_descriptor {
+       u16 type_kind;
+       u16 type_info;
+       char type_name[1];
+};
+
+struct source_location {
+       const char *file_name;
+       union {
+               unsigned long reported;
+               struct {
+                       u32 line;
+                       u32 column;
+               };
+       };
+};
+
+struct overflow_data {
+       struct source_location location;
+       struct type_descriptor *type;
+};
+
+struct type_mismatch_data {
+       struct source_location location;
+       struct type_descriptor *type;
+       unsigned long alignment;
+       unsigned char type_check_kind;
+};
+
+struct nonnull_arg_data {
+       struct source_location location;
+       struct source_location attr_location;
+       int arg_index;
+};
+
+struct nonnull_return_data {
+       struct source_location location;
+       struct source_location attr_location;
+};
+
+struct vla_bound_data {
+       struct source_location location;
+       struct type_descriptor *type;
+};
+
+struct out_of_bounds_data {
+       struct source_location location;
+       struct type_descriptor *array_type;
+       struct type_descriptor *index_type;
+};
+
+struct shift_out_of_bounds_data {
+       struct source_location location;
+       struct type_descriptor *lhs_type;
+       struct type_descriptor *rhs_type;
+};
+
+struct unreachable_data {
+       struct source_location location;
+};
+
+struct invalid_value_data {
+       struct source_location location;
+       struct type_descriptor *type;
+};
+
+#if defined(CONFIG_ARCH_SUPPORTS_INT128) && defined(__SIZEOF_INT128__)
+typedef __int128 s_max;
+typedef unsigned __int128 u_max;
+#else
+typedef s64 s_max;
+typedef u64 u_max;
+#endif
+
+#endif
diff --git a/xen/drivers/acpi/tables/Makefile b/xen/drivers/acpi/tables/Makefile
index e2210e0a05..5309ba8d73 100644
--- a/xen/drivers/acpi/tables/Makefile
+++ b/xen/drivers/acpi/tables/Makefile
@@ -3,3 +3,5 @@ obj-bin-y += tbinstal.init.o
 obj-y += tbutils.o
 obj-bin-y += tbxface.init.o
 obj-bin-y += tbxfroot.init.o
+
+noubsan-y += tbutils.o
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 002ba29d6d..eac54dfc53 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -210,6 +210,9 @@ struct vcpu
     bool             hcall_compat;
 #endif
 
+#ifdef CONFIG_UBSAN
+    int              in_ubsan;
+#endif
 
     /*
      * > 0: a single port is being polled;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.