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

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



Hi Wei,

On 03/10/17 17:03, Wei Liu wrote:
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.

Sadly, it is not trivial on Arm to get UBSAN running. It is not because of UBSAN itself but because of the memory layout. We assume that Xen is always smaller than 2MB (assembly code included :/).

With UBSAN enabled, Xen binary go way beyond 2MB and will fail to build.

I actually have a development branch with a prototype for UBSAN for Arm but I haven't found time to upstream it. I will rebase it on your work.


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/

I have got some for Arm too. I will try to post the fixes for Xen 4.10.

[...]

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

I believe this should go in arch specific code. For instance, Arm does not support INT128.

+#define u64 long long unsigned int
+#define s64 long long int

Would not be better to have a typedef for those 2 and use uint64_t and int64_t?

Cheers,

--
Julien Grall

_______________________________________________
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®.