[PATCH] intel_txt: fix the build errors of intel_txt patch on non-X86 platforms (resend)

[PATCH] intel_txt: fix the build errors of intel_txt patch on non-X86 platforms (resend)

[Shane Wang]

[-- Attachment #1: Type: text/plain, Size: 17874 bytes --]

Move tboot.h from asm to linux to fix the build errors of intel_txt patch on 
non-X86 platforms. Remove the tboot code from generic code init/main.c and 
kernel/cpu.c.

---
  arch/x86/Kconfig              |    4
  arch/x86/include/asm/tboot.h  |  197 --------------------------------
  arch/x86/kernel/reboot.c      |    3
  arch/x86/kernel/setup.c       |    3
  arch/x86/kernel/smpboot.c     |    2
  arch/x86/kernel/tboot.c       |   58 +++++++--
  drivers/acpi/acpica/hwsleep.c |    2
  drivers/pci/dmar.c            |    2
  drivers/pci/intel-iommu.c     |    2
  include/linux/tboot.h         |  162 ++++++++++++++++++++++++++
  init/main.c                   |    3
  kernel/cpu.c                  |    6
  security/Kconfig              |    2
  13 files changed, 221 insertions(+), 225 deletions(-)


Signed-off-by: Shane Wang <shane.wang@intel.com>

diff -r c6f74b152a32 arch/x86/Kconfig
--- a/arch/x86/Kconfig	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/Kconfig	Tue Sep 01 07:28:21 2009 -0700
@@ -179,6 +179,10 @@ config ARCH_SUPPORTS_OPTIMIZED_INLINING

  config ARCH_SUPPORTS_DEBUG_PAGEALLOC
  	def_bool y
+
+config HAVE_INTEL_TXT
+	def_bool y
+	depends on EXPERIMENTAL && DMAR && ACPI

  # Use the generic interrupt handling code in kernel/irq/:
  config GENERIC_HARDIRQS
diff -r c6f74b152a32 arch/x86/include/asm/tboot.h
--- a/arch/x86/include/asm/tboot.h	Tue Sep 01 07:24:42 2009 -0700
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,197 +0,0 @@
-/*
- * tboot.h: shared data structure with tboot and kernel and functions
- *          used by kernel for runtime support of Intel(R) Trusted
- *          Execution Technology
- *
- * Copyright (c) 2006-2009, Intel Corporation
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
- *
- */
-
-#ifndef _ASM_TBOOT_H
-#define _ASM_TBOOT_H
-
-#include <acpi/acpi.h>
-
-/* these must have the values from 0-5 in this order */
-enum {
-	TB_SHUTDOWN_REBOOT = 0,
-	TB_SHUTDOWN_S5,
-	TB_SHUTDOWN_S4,
-	TB_SHUTDOWN_S3,
-	TB_SHUTDOWN_HALT,
-	TB_SHUTDOWN_WFS
-};
-
-#ifdef CONFIG_INTEL_TXT
-
-/* used to communicate between tboot and the launched kernel */
-
-#define TB_KEY_SIZE             64   /* 512 bits */
-
-#define MAX_TB_MAC_REGIONS      32
-
-struct tboot_mac_region {
-	u64  start;         /* must be 64 byte -aligned */
-	u32  size;          /* must be 64 byte -granular */
-} __packed;
-
-/* GAS - Generic Address Structure (ACPI 2.0+) */
-struct tboot_acpi_generic_address {
-	u8  space_id;
-	u8  bit_width;
-	u8  bit_offset;
-	u8  access_width;
-	u64 address;
-} __packed;
-
-/*
- * combines Sx info from FADT and FACS tables per ACPI 2.0+ spec
- * (http://www.acpi.info/)
- */
-struct tboot_acpi_sleep_info {
-	struct tboot_acpi_generic_address pm1a_cnt_blk;
-	struct tboot_acpi_generic_address pm1b_cnt_blk;
-	struct tboot_acpi_generic_address pm1a_evt_blk;
-	struct tboot_acpi_generic_address pm1b_evt_blk;
-	u16 pm1a_cnt_val;
-	u16 pm1b_cnt_val;
-	u64 wakeup_vector;
-	u32 vector_width;
-	u64 kernel_s3_resume_vector;
-} __packed;
-
-/*
- * shared memory page used for communication between tboot and kernel
- */
-struct tboot {
-	/*
-	 * version 3+ fields:
-	 */
-
-	/* TBOOT_UUID */
-	u8 uuid[16];
-
-	/* version number: 5 is current */
-	u32 version;
-
-	/* physical addr of tb_log_t log */
-	u32 log_addr;
-
-	/*
-	 * physical addr of entry point for tboot shutdown and
-	 * type of shutdown (TB_SHUTDOWN_*) being requested
-	 */
-	u32 shutdown_entry;
-	u32 shutdown_type;
-
-	/* kernel-specified ACPI info for Sx shutdown */
-	struct tboot_acpi_sleep_info acpi_sinfo;
-
-	/* tboot location in memory (physical) */
-	u32 tboot_base;
-	u32 tboot_size;
-
-	/* memory regions (phys addrs) for tboot to MAC on S3 */
-	u8 num_mac_regions;
-	struct tboot_mac_region mac_regions[MAX_TB_MAC_REGIONS];
-
-
-	/*
-	 * version 4+ fields:
-	 */
-
-	/* symmetric key for use by kernel; will be encrypted on S3 */
-	u8 s3_key[TB_KEY_SIZE];
-
-
-	/*
-	 * version 5+ fields:
-	 */
-
-	/* used to 4byte-align num_in_wfs */
-	u8 reserved_align[3];
-
-	/* number of processors in wait-for-SIPI */
-	u32 num_in_wfs;
-} __packed;
-
-/*
- * UUID for tboot data struct to facilitate matching
- * defined as {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} by tboot, which is
- * represented as {} in the char array used here
- */
-#define TBOOT_UUID	{0xff, 0x8d, 0x3c, 0x66, 0xb3, 0xe8, 0x82, 0x4b, 0xbf,\
-			 0xaa, 0x19, 0xea, 0x4d, 0x5, 0x7a, 0x8}
-
-extern struct tboot *tboot;
-
-static inline int tboot_enabled(void)
-{
-	return tboot != NULL;
-}
-
-extern void tboot_probe(void);
-extern void tboot_create_trampoline(void);
-extern void tboot_shutdown(u32 shutdown_type);
-extern void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control);
-extern int tboot_wait_for_aps(int num_aps);
-extern struct acpi_table_header *tboot_get_dmar_table(
-				      struct acpi_table_header *dmar_tbl);
-extern int tboot_force_iommu(void);
-
-#else     /* CONFIG_INTEL_TXT */
-
-static inline int tboot_enabled(void)
-{
-	return 0;
-}
-
-static inline void tboot_probe(void)
-{
-}
-
-static inline void tboot_create_trampoline(void)
-{
-}
-
-static inline void tboot_shutdown(u32 shutdown_type)
-{
-}
-
-static inline void tboot_sleep(u8 sleep_state, u32 pm1a_control,
-			       u32 pm1b_control)
-{
-}
-
-static inline int tboot_wait_for_aps(int num_aps)
-{
-	return 0;
-}
-
-static inline struct acpi_table_header *tboot_get_dmar_table(
-					struct acpi_table_header *dmar_tbl)
-{
-	return dmar_tbl;
-}
-
-static inline int tboot_force_iommu(void)
-{
-	return 0;
-}
-
-#endif /* !CONFIG_INTEL_TXT */
-
-#endif /* _ASM_TBOOT_H */
diff -r c6f74b152a32 arch/x86/kernel/reboot.c
--- a/arch/x86/kernel/reboot.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/kernel/reboot.c	Tue Sep 01 07:28:21 2009 -0700
@@ -4,6 +4,7 @@
  #include <linux/pm.h>
  #include <linux/efi.h>
  #include <linux/dmi.h>
+#include <linux/tboot.h>
  #include <acpi/reboot.h>
  #include <asm/io.h>
  #include <asm/apic.h>
@@ -23,8 +24,6 @@
  #else
  # include <asm/iommu.h>
  #endif
-
-#include <asm/tboot.h>

  /*
   * Power off function, if any
diff -r c6f74b152a32 arch/x86/kernel/setup.c
--- a/arch/x86/kernel/setup.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/kernel/setup.c	Tue Sep 01 07:28:21 2009 -0700
@@ -66,6 +66,7 @@

  #include <linux/percpu.h>
  #include <linux/crash_dump.h>
+#include <linux/tboot.h>

  #include <video/edid.h>

@@ -140,8 +141,6 @@ struct boot_params __initdata boot_param
  #else
  struct boot_params boot_params;
  #endif
-
-#include <asm/tboot.h>

  /*
   * Machine setup..
diff -r c6f74b152a32 arch/x86/kernel/smpboot.c
--- a/arch/x86/kernel/smpboot.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/kernel/smpboot.c	Tue Sep 01 07:28:21 2009 -0700
@@ -47,6 +47,7 @@
  #include <linux/bootmem.h>
  #include <linux/err.h>
  #include <linux/nmi.h>
+#include <linux/tboot.h>

  #include <asm/acpi.h>
  #include <asm/desc.h>
@@ -62,7 +63,6 @@
  #include <asm/vmi.h>
  #include <asm/apic.h>
  #include <asm/setup.h>
-#include <asm/tboot.h>
  #include <asm/uv/uv.h>
  #include <asm/debugreg.h>
  #include <linux/mc146818rtc.h>
diff -r c6f74b152a32 arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/kernel/tboot.c	Tue Sep 01 07:28:21 2009 -0700
@@ -22,11 +22,14 @@
  #include <linux/dma_remapping.h>
  #include <linux/init_task.h>
  #include <linux/spinlock.h>
+#include <linux/delay.h>
  #include <linux/sched.h>
  #include <linux/init.h>
  #include <linux/dmar.h>
+#include <linux/cpu.h>
  #include <linux/pfn.h>
  #include <linux/mm.h>
+#include <linux/tboot.h>

  #include <asm/trampoline.h>
  #include <asm/processor.h>
@@ -36,7 +39,6 @@
  #include <asm/fixmap.h>
  #include <asm/proto.h>
  #include <asm/setup.h>
-#include <asm/tboot.h>
  #include <asm/e820.h>
  #include <asm/io.h>

@@ -154,12 +156,9 @@ static int map_tboot_pages(unsigned long
  	return 0;
  }

-void tboot_create_trampoline(void)
+static void tboot_create_trampoline(void)
  {
  	u32 map_base, map_size;
-
-	if (!tboot_enabled())
-		return;

  	/* Create identity map for tboot shutdown code. */
  	map_base = PFN_DOWN(tboot->tboot_base);
@@ -295,20 +294,57 @@ void tboot_sleep(u8 sleep_state, u32 pm1
  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
  }

-int tboot_wait_for_aps(int num_aps)
+static atomic_t ap_wfs_count;
+
+static int tboot_wait_for_aps(int num_aps)
  {
  	unsigned long timeout;

+	timeout = AP_WAIT_TIMEOUT*HZ;
+	while (atomic_read((atomic_t *)&tboot->num_in_wfs) != num_aps &&
+	       timeout) {
+		mdelay(1);
+		timeout--;
+	}
+
+	if (timeout)
+		pr_warning("tboot wait for APs timeout\n");
+
+	return !(atomic_read((atomic_t *)&tboot->num_in_wfs) == num_aps);
+}
+
+static int __cpuinit tboot_cpu_callback(struct notifier_block *nfb,
+			unsigned long action, void *hcpu)
+{
+	switch (action) {
+	case CPU_DYING:
+		atomic_inc(&ap_wfs_count);
+		if (num_online_cpus() == 1)
+			if (tboot_wait_for_aps(atomic_read(&ap_wfs_count)))
+				return NOTIFY_BAD;
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block tboot_cpu_notifier __cpuinitdata =
+{
+	.notifier_call = tboot_cpu_callback,
+};
+
+static __init int tboot_late_init(void)
+{
  	if (!tboot_enabled())
  		return 0;

-	timeout = jiffies + AP_WAIT_TIMEOUT*HZ;
-	while (atomic_read((atomic_t *)&tboot->num_in_wfs) != num_aps &&
-	       time_before(jiffies, timeout))
-		cpu_relax();
+	tboot_create_trampoline();

-	return time_before(jiffies, timeout) ? 0 : 1;
+	atomic_set(&ap_wfs_count, 0);
+	register_hotcpu_notifier(&tboot_cpu_notifier);
+	return 0;
  }
+
+late_initcall(tboot_late_init);

  /*
   * TXT configuration registers (offsets from TXT_{PUB, PRIV}_CONFIG_REGS_BASE)
diff -r c6f74b152a32 drivers/acpi/acpica/hwsleep.c
--- a/drivers/acpi/acpica/hwsleep.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/drivers/acpi/acpica/hwsleep.c	Tue Sep 01 07:28:21 2009 -0700
@@ -45,7 +45,7 @@
  #include <acpi/acpi.h>
  #include "accommon.h"
  #include "actables.h"
-#include <asm/tboot.h>
+#include <linux/tboot.h>

  #define _COMPONENT          ACPI_HARDWARE
  ACPI_MODULE_NAME("hwsleep")
diff -r c6f74b152a32 drivers/pci/dmar.c
--- a/drivers/pci/dmar.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/drivers/pci/dmar.c	Tue Sep 01 07:28:21 2009 -0700
@@ -33,7 +33,7 @@
  #include <linux/timer.h>
  #include <linux/irq.h>
  #include <linux/interrupt.h>
-#include <asm/tboot.h>
+#include <linux/tboot.h>

  #undef PREFIX
  #define PREFIX "DMAR:"
diff -r c6f74b152a32 drivers/pci/intel-iommu.c
--- a/drivers/pci/intel-iommu.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/drivers/pci/intel-iommu.c	Tue Sep 01 07:28:21 2009 -0700
@@ -37,8 +37,8 @@
  #include <linux/iommu.h>
  #include <linux/intel-iommu.h>
  #include <linux/sysdev.h>
+#include <linux/tboot.h>
  #include <asm/cacheflush.h>
-#include <asm/tboot.h>
  #include <asm/iommu.h>
  #include "pci.h"

diff -r c6f74b152a32 include/linux/tboot.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/include/linux/tboot.h	Tue Sep 01 07:28:21 2009 -0700
@@ -0,0 +1,162 @@
+/*
+ * tboot.h: shared data structure with tboot and kernel and functions
+ *          used by kernel for runtime support of Intel(R) Trusted
+ *          Execution Technology
+ *
+ * Copyright (c) 2006-2009, Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#ifndef _LINUX_TBOOT_H
+#define _LINUX_TBOOT_H
+
+/* these must have the values from 0-5 in this order */
+enum {
+	TB_SHUTDOWN_REBOOT = 0,
+	TB_SHUTDOWN_S5,
+	TB_SHUTDOWN_S4,
+	TB_SHUTDOWN_S3,
+	TB_SHUTDOWN_HALT,
+	TB_SHUTDOWN_WFS
+};
+
+#ifdef CONFIG_INTEL_TXT
+#include <acpi/acpi.h>
+/* used to communicate between tboot and the launched kernel */
+
+#define TB_KEY_SIZE             64   /* 512 bits */
+
+#define MAX_TB_MAC_REGIONS      32
+
+struct tboot_mac_region {
+	u64  start;         /* must be 64 byte -aligned */
+	u32  size;          /* must be 64 byte -granular */
+} __packed;
+
+/* GAS - Generic Address Structure (ACPI 2.0+) */
+struct tboot_acpi_generic_address {
+	u8  space_id;
+	u8  bit_width;
+	u8  bit_offset;
+	u8  access_width;
+	u64 address;
+} __packed;
+
+/*
+ * combines Sx info from FADT and FACS tables per ACPI 2.0+ spec
+ * (http://www.acpi.info/)
+ */
+struct tboot_acpi_sleep_info {
+	struct tboot_acpi_generic_address pm1a_cnt_blk;
+	struct tboot_acpi_generic_address pm1b_cnt_blk;
+	struct tboot_acpi_generic_address pm1a_evt_blk;
+	struct tboot_acpi_generic_address pm1b_evt_blk;
+	u16 pm1a_cnt_val;
+	u16 pm1b_cnt_val;
+	u64 wakeup_vector;
+	u32 vector_width;
+	u64 kernel_s3_resume_vector;
+} __packed;
+
+/*
+ * shared memory page used for communication between tboot and kernel
+ */
+struct tboot {
+	/*
+	 * version 3+ fields:
+	 */
+
+	/* TBOOT_UUID */
+	u8 uuid[16];
+
+	/* version number: 5 is current */
+	u32 version;
+
+	/* physical addr of tb_log_t log */
+	u32 log_addr;
+
+	/*
+	 * physical addr of entry point for tboot shutdown and
+	 * type of shutdown (TB_SHUTDOWN_*) being requested
+	 */
+	u32 shutdown_entry;
+	u32 shutdown_type;
+
+	/* kernel-specified ACPI info for Sx shutdown */
+	struct tboot_acpi_sleep_info acpi_sinfo;
+
+	/* tboot location in memory (physical) */
+	u32 tboot_base;
+	u32 tboot_size;
+
+	/* memory regions (phys addrs) for tboot to MAC on S3 */
+	u8 num_mac_regions;
+	struct tboot_mac_region mac_regions[MAX_TB_MAC_REGIONS];
+
+
+	/*
+	 * version 4+ fields:
+	 */
+
+	/* symmetric key for use by kernel; will be encrypted on S3 */
+	u8 s3_key[TB_KEY_SIZE];
+
+
+	/*
+	 * version 5+ fields:
+	 */
+
+	/* used to 4byte-align num_in_wfs */
+	u8 reserved_align[3];
+
+	/* number of processors in wait-for-SIPI */
+	u32 num_in_wfs;
+} __packed;
+
+/*
+ * UUID for tboot data struct to facilitate matching
+ * defined as {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} by tboot, which is
+ * represented as {} in the char array used here
+ */
+#define TBOOT_UUID	{0xff, 0x8d, 0x3c, 0x66, 0xb3, 0xe8, 0x82, 0x4b, 0xbf,\
+			 0xaa, 0x19, 0xea, 0x4d, 0x5, 0x7a, 0x8}
+
+extern struct tboot *tboot;
+
+static inline int tboot_enabled(void)
+{
+	return tboot != NULL;
+}
+
+extern void tboot_probe(void);
+extern void tboot_shutdown(u32 shutdown_type);
+extern void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control);
+extern struct acpi_table_header *tboot_get_dmar_table(
+				      struct acpi_table_header *dmar_tbl);
+extern int tboot_force_iommu(void);
+
+#else
+
+#define tboot_probe()			do { } while (0)
+#define tboot_shutdown(shutdown_type)	do { } while (0)
+#define tboot_sleep(sleep_state, pm1a_control, pm1b_control)	\
+					do { } while (0)
+#define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
+#define tboot_force_iommu()		0
+
+#endif /* !CONFIG_INTEL_TXT */
+
+#endif /* _LINUX_TBOOT_H */
diff -r c6f74b152a32 init/main.c
--- a/init/main.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/init/main.c	Tue Sep 01 07:28:21 2009 -0700
@@ -73,7 +73,6 @@
  #include <asm/io.h>
  #include <asm/bugs.h>
  #include <asm/setup.h>
-#include <asm/tboot.h>
  #include <asm/sections.h>
  #include <asm/cacheflush.h>

@@ -716,8 +715,6 @@ asmlinkage void __init start_kernel(void

  	ftrace_init();

-	tboot_create_trampoline();
-
  	/* Do the rest non-__init'ed, we're now alive */
  	rest_init();
  }
diff -r c6f74b152a32 kernel/cpu.c
--- a/kernel/cpu.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/kernel/cpu.c	Tue Sep 01 07:28:21 2009 -0700
@@ -14,7 +14,6 @@
  #include <linux/kthread.h>
  #include <linux/stop_machine.h>
  #include <linux/mutex.h>
-#include <asm/tboot.h>

  #ifdef CONFIG_SMP
  /* Serializes the updates to cpu_online_mask, cpu_present_mask */
@@ -377,7 +376,7 @@ static cpumask_var_t frozen_cpus;

  int disable_nonboot_cpus(void)
  {
-	int cpu, first_cpu, error, num_cpus = 0;
+	int cpu, first_cpu, error;

  	error = stop_machine_create();
  	if (error)
@@ -392,7 +391,6 @@ int disable_nonboot_cpus(void)
  	for_each_online_cpu(cpu) {
  		if (cpu == first_cpu)
  			continue;
-		num_cpus++;
  		error = _cpu_down(cpu, 1);
  		if (!error) {
  			cpumask_set_cpu(cpu, frozen_cpus);
@@ -403,8 +401,6 @@ int disable_nonboot_cpus(void)
  			break;
  		}
  	}
-	/* ensure all CPUs have gone into wait-for-SIPI */
-	error |= tboot_wait_for_aps(num_cpus);

  	if (!error) {
  		BUG_ON(num_online_cpus() > 1);
diff -r c6f74b152a32 security/Kconfig
--- a/security/Kconfig	Tue Sep 01 07:24:42 2009 -0700
+++ b/security/Kconfig	Tue Sep 01 07:28:21 2009 -0700
@@ -131,7 +131,7 @@ config LSM_MMAP_MIN_ADDR

  config INTEL_TXT
  	bool "Enable Intel(R) Trusted Execution Technology (Intel(R) TXT)"
-	depends on EXPERIMENTAL && X86 && DMAR && ACPI
+	depends on HAVE_INTEL_TXT
  	help
  	  This option enables support for booting the kernel with the
  	  Trusted Boot (tboot) module. This will utilize


[-- Attachment #2: tboot_fix.patch --]
[-- Type: text/plain, Size: 17146 bytes --]

Move tboot.h from asm to linux to fix the build errors of intel_txt patch on non-X86 platforms. Remove the tboot code from generic code init/main.c and kernel/cpu.c.

Signed-off-by: Shane Wang <shane.wang@intel.com>

diff -r c6f74b152a32 arch/x86/Kconfig
--- a/arch/x86/Kconfig	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/Kconfig	Tue Sep 01 07:28:21 2009 -0700
@@ -179,6 +179,10 @@ config ARCH_SUPPORTS_OPTIMIZED_INLINING
 
 config ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	def_bool y
+
+config HAVE_INTEL_TXT
+	def_bool y
+	depends on EXPERIMENTAL && DMAR && ACPI
 
 # Use the generic interrupt handling code in kernel/irq/:
 config GENERIC_HARDIRQS
diff -r c6f74b152a32 arch/x86/include/asm/tboot.h
--- a/arch/x86/include/asm/tboot.h	Tue Sep 01 07:24:42 2009 -0700
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,197 +0,0 @@
-/*
- * tboot.h: shared data structure with tboot and kernel and functions
- *          used by kernel for runtime support of Intel(R) Trusted
- *          Execution Technology
- *
- * Copyright (c) 2006-2009, Intel Corporation
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
- *
- */
-
-#ifndef _ASM_TBOOT_H
-#define _ASM_TBOOT_H
-
-#include <acpi/acpi.h>
-
-/* these must have the values from 0-5 in this order */
-enum {
-	TB_SHUTDOWN_REBOOT = 0,
-	TB_SHUTDOWN_S5,
-	TB_SHUTDOWN_S4,
-	TB_SHUTDOWN_S3,
-	TB_SHUTDOWN_HALT,
-	TB_SHUTDOWN_WFS
-};
-
-#ifdef CONFIG_INTEL_TXT
-
-/* used to communicate between tboot and the launched kernel */
-
-#define TB_KEY_SIZE             64   /* 512 bits */
-
-#define MAX_TB_MAC_REGIONS      32
-
-struct tboot_mac_region {
-	u64  start;         /* must be 64 byte -aligned */
-	u32  size;          /* must be 64 byte -granular */
-} __packed;
-
-/* GAS - Generic Address Structure (ACPI 2.0+) */
-struct tboot_acpi_generic_address {
-	u8  space_id;
-	u8  bit_width;
-	u8  bit_offset;
-	u8  access_width;
-	u64 address;
-} __packed;
-
-/*
- * combines Sx info from FADT and FACS tables per ACPI 2.0+ spec
- * (http://www.acpi.info/)
- */
-struct tboot_acpi_sleep_info {
-	struct tboot_acpi_generic_address pm1a_cnt_blk;
-	struct tboot_acpi_generic_address pm1b_cnt_blk;
-	struct tboot_acpi_generic_address pm1a_evt_blk;
-	struct tboot_acpi_generic_address pm1b_evt_blk;
-	u16 pm1a_cnt_val;
-	u16 pm1b_cnt_val;
-	u64 wakeup_vector;
-	u32 vector_width;
-	u64 kernel_s3_resume_vector;
-} __packed;
-
-/*
- * shared memory page used for communication between tboot and kernel
- */
-struct tboot {
-	/*
-	 * version 3+ fields:
-	 */
-
-	/* TBOOT_UUID */
-	u8 uuid[16];
-
-	/* version number: 5 is current */
-	u32 version;
-
-	/* physical addr of tb_log_t log */
-	u32 log_addr;
-
-	/*
-	 * physical addr of entry point for tboot shutdown and
-	 * type of shutdown (TB_SHUTDOWN_*) being requested
-	 */
-	u32 shutdown_entry;
-	u32 shutdown_type;
-
-	/* kernel-specified ACPI info for Sx shutdown */
-	struct tboot_acpi_sleep_info acpi_sinfo;
-
-	/* tboot location in memory (physical) */
-	u32 tboot_base;
-	u32 tboot_size;
-
-	/* memory regions (phys addrs) for tboot to MAC on S3 */
-	u8 num_mac_regions;
-	struct tboot_mac_region mac_regions[MAX_TB_MAC_REGIONS];
-
-
-	/*
-	 * version 4+ fields:
-	 */
-
-	/* symmetric key for use by kernel; will be encrypted on S3 */
-	u8 s3_key[TB_KEY_SIZE];
-
-
-	/*
-	 * version 5+ fields:
-	 */
-
-	/* used to 4byte-align num_in_wfs */
-	u8 reserved_align[3];
-
-	/* number of processors in wait-for-SIPI */
-	u32 num_in_wfs;
-} __packed;
-
-/*
- * UUID for tboot data struct to facilitate matching
- * defined as {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} by tboot, which is
- * represented as {} in the char array used here
- */
-#define TBOOT_UUID	{0xff, 0x8d, 0x3c, 0x66, 0xb3, 0xe8, 0x82, 0x4b, 0xbf,\
-			 0xaa, 0x19, 0xea, 0x4d, 0x5, 0x7a, 0x8}
-
-extern struct tboot *tboot;
-
-static inline int tboot_enabled(void)
-{
-	return tboot != NULL;
-}
-
-extern void tboot_probe(void);
-extern void tboot_create_trampoline(void);
-extern void tboot_shutdown(u32 shutdown_type);
-extern void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control);
-extern int tboot_wait_for_aps(int num_aps);
-extern struct acpi_table_header *tboot_get_dmar_table(
-				      struct acpi_table_header *dmar_tbl);
-extern int tboot_force_iommu(void);
-
-#else     /* CONFIG_INTEL_TXT */
-
-static inline int tboot_enabled(void)
-{
-	return 0;
-}
-
-static inline void tboot_probe(void)
-{
-}
-
-static inline void tboot_create_trampoline(void)
-{
-}
-
-static inline void tboot_shutdown(u32 shutdown_type)
-{
-}
-
-static inline void tboot_sleep(u8 sleep_state, u32 pm1a_control,
-			       u32 pm1b_control)
-{
-}
-
-static inline int tboot_wait_for_aps(int num_aps)
-{
-	return 0;
-}
-
-static inline struct acpi_table_header *tboot_get_dmar_table(
-					struct acpi_table_header *dmar_tbl)
-{
-	return dmar_tbl;
-}
-
-static inline int tboot_force_iommu(void)
-{
-	return 0;
-}
-
-#endif /* !CONFIG_INTEL_TXT */
-
-#endif /* _ASM_TBOOT_H */
diff -r c6f74b152a32 arch/x86/kernel/reboot.c
--- a/arch/x86/kernel/reboot.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/kernel/reboot.c	Tue Sep 01 07:28:21 2009 -0700
@@ -4,6 +4,7 @@
 #include <linux/pm.h>
 #include <linux/efi.h>
 #include <linux/dmi.h>
+#include <linux/tboot.h>
 #include <acpi/reboot.h>
 #include <asm/io.h>
 #include <asm/apic.h>
@@ -23,8 +24,6 @@
 #else
 # include <asm/iommu.h>
 #endif
-
-#include <asm/tboot.h>
 
 /*
  * Power off function, if any
diff -r c6f74b152a32 arch/x86/kernel/setup.c
--- a/arch/x86/kernel/setup.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/kernel/setup.c	Tue Sep 01 07:28:21 2009 -0700
@@ -66,6 +66,7 @@
 
 #include <linux/percpu.h>
 #include <linux/crash_dump.h>
+#include <linux/tboot.h>
 
 #include <video/edid.h>
 
@@ -140,8 +141,6 @@ struct boot_params __initdata boot_param
 #else
 struct boot_params boot_params;
 #endif
-
-#include <asm/tboot.h>
 
 /*
  * Machine setup..
diff -r c6f74b152a32 arch/x86/kernel/smpboot.c
--- a/arch/x86/kernel/smpboot.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/kernel/smpboot.c	Tue Sep 01 07:28:21 2009 -0700
@@ -47,6 +47,7 @@
 #include <linux/bootmem.h>
 #include <linux/err.h>
 #include <linux/nmi.h>
+#include <linux/tboot.h>
 
 #include <asm/acpi.h>
 #include <asm/desc.h>
@@ -62,7 +63,6 @@
 #include <asm/vmi.h>
 #include <asm/apic.h>
 #include <asm/setup.h>
-#include <asm/tboot.h>
 #include <asm/uv/uv.h>
 #include <asm/debugreg.h>
 #include <linux/mc146818rtc.h>
diff -r c6f74b152a32 arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/arch/x86/kernel/tboot.c	Tue Sep 01 07:28:21 2009 -0700
@@ -22,11 +22,14 @@
 #include <linux/dma_remapping.h>
 #include <linux/init_task.h>
 #include <linux/spinlock.h>
+#include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/init.h>
 #include <linux/dmar.h>
+#include <linux/cpu.h>
 #include <linux/pfn.h>
 #include <linux/mm.h>
+#include <linux/tboot.h>
 
 #include <asm/trampoline.h>
 #include <asm/processor.h>
@@ -36,7 +39,6 @@
 #include <asm/fixmap.h>
 #include <asm/proto.h>
 #include <asm/setup.h>
-#include <asm/tboot.h>
 #include <asm/e820.h>
 #include <asm/io.h>
 
@@ -154,12 +156,9 @@ static int map_tboot_pages(unsigned long
 	return 0;
 }
 
-void tboot_create_trampoline(void)
+static void tboot_create_trampoline(void)
 {
 	u32 map_base, map_size;
-
-	if (!tboot_enabled())
-		return;
 
 	/* Create identity map for tboot shutdown code. */
 	map_base = PFN_DOWN(tboot->tboot_base);
@@ -295,20 +294,57 @@ void tboot_sleep(u8 sleep_state, u32 pm1
 	tboot_shutdown(acpi_shutdown_map[sleep_state]);
 }
 
-int tboot_wait_for_aps(int num_aps)
+static atomic_t ap_wfs_count;
+
+static int tboot_wait_for_aps(int num_aps)
 {
 	unsigned long timeout;
 
+	timeout = AP_WAIT_TIMEOUT*HZ;
+	while (atomic_read((atomic_t *)&tboot->num_in_wfs) != num_aps &&
+	       timeout) {
+		mdelay(1);
+		timeout--;
+	}
+
+	if (timeout)
+		pr_warning("tboot wait for APs timeout\n");
+
+	return !(atomic_read((atomic_t *)&tboot->num_in_wfs) == num_aps);
+}
+
+static int __cpuinit tboot_cpu_callback(struct notifier_block *nfb,
+			unsigned long action, void *hcpu)
+{
+	switch (action) {
+	case CPU_DYING:
+		atomic_inc(&ap_wfs_count);
+		if (num_online_cpus() == 1)
+			if (tboot_wait_for_aps(atomic_read(&ap_wfs_count)))
+				return NOTIFY_BAD;
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block tboot_cpu_notifier __cpuinitdata =
+{
+	.notifier_call = tboot_cpu_callback,
+};
+
+static __init int tboot_late_init(void)
+{
 	if (!tboot_enabled())
 		return 0;
 
-	timeout = jiffies + AP_WAIT_TIMEOUT*HZ;
-	while (atomic_read((atomic_t *)&tboot->num_in_wfs) != num_aps &&
-	       time_before(jiffies, timeout))
-		cpu_relax();
+	tboot_create_trampoline();
 
-	return time_before(jiffies, timeout) ? 0 : 1;
+	atomic_set(&ap_wfs_count, 0);
+	register_hotcpu_notifier(&tboot_cpu_notifier);
+	return 0;
 }
+
+late_initcall(tboot_late_init);
 
 /*
  * TXT configuration registers (offsets from TXT_{PUB, PRIV}_CONFIG_REGS_BASE)
diff -r c6f74b152a32 drivers/acpi/acpica/hwsleep.c
--- a/drivers/acpi/acpica/hwsleep.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/drivers/acpi/acpica/hwsleep.c	Tue Sep 01 07:28:21 2009 -0700
@@ -45,7 +45,7 @@
 #include <acpi/acpi.h>
 #include "accommon.h"
 #include "actables.h"
-#include <asm/tboot.h>
+#include <linux/tboot.h>
 
 #define _COMPONENT          ACPI_HARDWARE
 ACPI_MODULE_NAME("hwsleep")
diff -r c6f74b152a32 drivers/pci/dmar.c
--- a/drivers/pci/dmar.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/drivers/pci/dmar.c	Tue Sep 01 07:28:21 2009 -0700
@@ -33,7 +33,7 @@
 #include <linux/timer.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
-#include <asm/tboot.h>
+#include <linux/tboot.h>
 
 #undef PREFIX
 #define PREFIX "DMAR:"
diff -r c6f74b152a32 drivers/pci/intel-iommu.c
--- a/drivers/pci/intel-iommu.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/drivers/pci/intel-iommu.c	Tue Sep 01 07:28:21 2009 -0700
@@ -37,8 +37,8 @@
 #include <linux/iommu.h>
 #include <linux/intel-iommu.h>
 #include <linux/sysdev.h>
+#include <linux/tboot.h>
 #include <asm/cacheflush.h>
-#include <asm/tboot.h>
 #include <asm/iommu.h>
 #include "pci.h"
 
diff -r c6f74b152a32 include/linux/tboot.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/include/linux/tboot.h	Tue Sep 01 07:28:21 2009 -0700
@@ -0,0 +1,162 @@
+/*
+ * tboot.h: shared data structure with tboot and kernel and functions
+ *          used by kernel for runtime support of Intel(R) Trusted
+ *          Execution Technology
+ *
+ * Copyright (c) 2006-2009, Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#ifndef _LINUX_TBOOT_H
+#define _LINUX_TBOOT_H
+
+/* these must have the values from 0-5 in this order */
+enum {
+	TB_SHUTDOWN_REBOOT = 0,
+	TB_SHUTDOWN_S5,
+	TB_SHUTDOWN_S4,
+	TB_SHUTDOWN_S3,
+	TB_SHUTDOWN_HALT,
+	TB_SHUTDOWN_WFS
+};
+
+#ifdef CONFIG_INTEL_TXT
+#include <acpi/acpi.h>
+/* used to communicate between tboot and the launched kernel */
+
+#define TB_KEY_SIZE             64   /* 512 bits */
+
+#define MAX_TB_MAC_REGIONS      32
+
+struct tboot_mac_region {
+	u64  start;         /* must be 64 byte -aligned */
+	u32  size;          /* must be 64 byte -granular */
+} __packed;
+
+/* GAS - Generic Address Structure (ACPI 2.0+) */
+struct tboot_acpi_generic_address {
+	u8  space_id;
+	u8  bit_width;
+	u8  bit_offset;
+	u8  access_width;
+	u64 address;
+} __packed;
+
+/*
+ * combines Sx info from FADT and FACS tables per ACPI 2.0+ spec
+ * (http://www.acpi.info/)
+ */
+struct tboot_acpi_sleep_info {
+	struct tboot_acpi_generic_address pm1a_cnt_blk;
+	struct tboot_acpi_generic_address pm1b_cnt_blk;
+	struct tboot_acpi_generic_address pm1a_evt_blk;
+	struct tboot_acpi_generic_address pm1b_evt_blk;
+	u16 pm1a_cnt_val;
+	u16 pm1b_cnt_val;
+	u64 wakeup_vector;
+	u32 vector_width;
+	u64 kernel_s3_resume_vector;
+} __packed;
+
+/*
+ * shared memory page used for communication between tboot and kernel
+ */
+struct tboot {
+	/*
+	 * version 3+ fields:
+	 */
+
+	/* TBOOT_UUID */
+	u8 uuid[16];
+
+	/* version number: 5 is current */
+	u32 version;
+
+	/* physical addr of tb_log_t log */
+	u32 log_addr;
+
+	/*
+	 * physical addr of entry point for tboot shutdown and
+	 * type of shutdown (TB_SHUTDOWN_*) being requested
+	 */
+	u32 shutdown_entry;
+	u32 shutdown_type;
+
+	/* kernel-specified ACPI info for Sx shutdown */
+	struct tboot_acpi_sleep_info acpi_sinfo;
+
+	/* tboot location in memory (physical) */
+	u32 tboot_base;
+	u32 tboot_size;
+
+	/* memory regions (phys addrs) for tboot to MAC on S3 */
+	u8 num_mac_regions;
+	struct tboot_mac_region mac_regions[MAX_TB_MAC_REGIONS];
+
+
+	/*
+	 * version 4+ fields:
+	 */
+
+	/* symmetric key for use by kernel; will be encrypted on S3 */
+	u8 s3_key[TB_KEY_SIZE];
+
+
+	/*
+	 * version 5+ fields:
+	 */
+
+	/* used to 4byte-align num_in_wfs */
+	u8 reserved_align[3];
+
+	/* number of processors in wait-for-SIPI */
+	u32 num_in_wfs;
+} __packed;
+
+/*
+ * UUID for tboot data struct to facilitate matching
+ * defined as {663C8DFF-E8B3-4b82-AABF-19EA4D057A08} by tboot, which is
+ * represented as {} in the char array used here
+ */
+#define TBOOT_UUID	{0xff, 0x8d, 0x3c, 0x66, 0xb3, 0xe8, 0x82, 0x4b, 0xbf,\
+			 0xaa, 0x19, 0xea, 0x4d, 0x5, 0x7a, 0x8}
+
+extern struct tboot *tboot;
+
+static inline int tboot_enabled(void)
+{
+	return tboot != NULL;
+}
+
+extern void tboot_probe(void);
+extern void tboot_shutdown(u32 shutdown_type);
+extern void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control);
+extern struct acpi_table_header *tboot_get_dmar_table(
+				      struct acpi_table_header *dmar_tbl);
+extern int tboot_force_iommu(void);
+
+#else
+
+#define tboot_probe()			do { } while (0)
+#define tboot_shutdown(shutdown_type)	do { } while (0)
+#define tboot_sleep(sleep_state, pm1a_control, pm1b_control)	\
+					do { } while (0)
+#define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
+#define tboot_force_iommu()		0
+
+#endif /* !CONFIG_INTEL_TXT */
+
+#endif /* _LINUX_TBOOT_H */
diff -r c6f74b152a32 init/main.c
--- a/init/main.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/init/main.c	Tue Sep 01 07:28:21 2009 -0700
@@ -73,7 +73,6 @@
 #include <asm/io.h>
 #include <asm/bugs.h>
 #include <asm/setup.h>
-#include <asm/tboot.h>
 #include <asm/sections.h>
 #include <asm/cacheflush.h>
 
@@ -716,8 +715,6 @@ asmlinkage void __init start_kernel(void
 
 	ftrace_init();
 
-	tboot_create_trampoline();
-
 	/* Do the rest non-__init'ed, we're now alive */
 	rest_init();
 }
diff -r c6f74b152a32 kernel/cpu.c
--- a/kernel/cpu.c	Tue Sep 01 07:24:42 2009 -0700
+++ b/kernel/cpu.c	Tue Sep 01 07:28:21 2009 -0700
@@ -14,7 +14,6 @@
 #include <linux/kthread.h>
 #include <linux/stop_machine.h>
 #include <linux/mutex.h>
-#include <asm/tboot.h>
 
 #ifdef CONFIG_SMP
 /* Serializes the updates to cpu_online_mask, cpu_present_mask */
@@ -377,7 +376,7 @@ static cpumask_var_t frozen_cpus;
 
 int disable_nonboot_cpus(void)
 {
-	int cpu, first_cpu, error, num_cpus = 0;
+	int cpu, first_cpu, error;
 
 	error = stop_machine_create();
 	if (error)
@@ -392,7 +391,6 @@ int disable_nonboot_cpus(void)
 	for_each_online_cpu(cpu) {
 		if (cpu == first_cpu)
 			continue;
-		num_cpus++;
 		error = _cpu_down(cpu, 1);
 		if (!error) {
 			cpumask_set_cpu(cpu, frozen_cpus);
@@ -403,8 +401,6 @@ int disable_nonboot_cpus(void)
 			break;
 		}
 	}
-	/* ensure all CPUs have gone into wait-for-SIPI */
-	error |= tboot_wait_for_aps(num_cpus);
 
 	if (!error) {
 		BUG_ON(num_online_cpus() > 1);
diff -r c6f74b152a32 security/Kconfig
--- a/security/Kconfig	Tue Sep 01 07:24:42 2009 -0700
+++ b/security/Kconfig	Tue Sep 01 07:28:21 2009 -0700
@@ -131,7 +131,7 @@ config LSM_MMAP_MIN_ADDR
 
 config INTEL_TXT
 	bool "Enable Intel(R) Trusted Execution Technology (Intel(R) TXT)"
-	depends on EXPERIMENTAL && X86 && DMAR && ACPI
+	depends on HAVE_INTEL_TXT
 	help
 	  This option enables support for booting the kernel with the
 	  Trusted Boot (tboot) module. This will utilize

[PATCH] intel_txt: add s3 userspace memory integrity verification

[Shane Wang]

[-- Attachment #1: Type: text/plain, Size: 7677 bytes --]

This patch added verification for userspace memory integrity after s3 resume.
Integrity verification for other memory (say kernel itself) has been done by tboot.

Thanks for your comments.
Shane

---
  arch/x86/kernel/tboot.c |  170 ++++++++++++++++++++++++++++++++++++++
  drivers/acpi/sleep.c    |    4
  include/linux/tboot.h   |    7 +
  security/Kconfig        |    2
  4 files changed, 182 insertions(+), 1 deletion(-)


Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

diff -r 42870e183bd5 arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Fri Sep 25 06:06:39 2009 -0400
+++ b/arch/x86/kernel/tboot.c	Fri Sep 25 11:03:04 2009 -0400
@@ -20,6 +20,7 @@
   */

  #include <linux/dma_remapping.h>
+#include <linux/scatterlist.h>
  #include <linux/init_task.h>
  #include <linux/spinlock.h>
  #include <linux/delay.h>
@@ -30,12 +31,17 @@
  #include <linux/pfn.h>
  #include <linux/mm.h>
  #include <linux/tboot.h>
+#include <linux/random.h>
+
+#include <crypto/vmac.h>
+#include <crypto/hash.h>

  #include <asm/trampoline.h>
  #include <asm/processor.h>
  #include <asm/bootparam.h>
  #include <asm/pgtable.h>
  #include <asm/pgalloc.h>
+#include <asm/percpu.h>
  #include <asm/fixmap.h>
  #include <asm/proto.h>
  #include <asm/setup.h>
@@ -168,6 +174,80 @@ static void tboot_create_trampoline(void
  		      map_base, map_size);
  }

+static vmac_t mem_mac;
+static struct crypto_hash *tfm;
+
+static int tboot_gen_mem_integrity(const uint8_t key[], vmac_t *mac)
+{
+	int i, j, ret;
+	pg_data_t *pgdat;
+	struct hash_desc desc;
+	struct scatterlist sg[1];
+	struct page *page;
+	uint64_t paddr, rstart, rend;
+	unsigned long pfn;
+	uint8_t zeroed_key[VMAC_KEY_LEN];
+
+	if (!tfm)
+		tfm = crypto_alloc_hash("vmac(aes)", 0, CRYPTO_ALG_ASYNC);
+
+	if (IS_ERR(tfm)) {
+		tfm = NULL;
+		return -ENOMEM;
+	}
+
+	desc.tfm = tfm;
+	desc.flags = 0;
+
+	sg_init_table(sg, 1);
+
+	ret = crypto_hash_init(&desc);
+	if (ret)
+		return ret;
+	ret = crypto_hash_setkey(desc.tfm, key, VMAC_KEY_LEN);
+	if (ret)
+		return ret;
+
+	for_each_online_pgdat(pgdat) {
+		for (i = 0, pfn = pgdat->node_start_pfn;
+			i < pgdat->node_spanned_pages;
+			i++, pfn = pgdat->node_start_pfn + i) {
+
+			if (!pfn_valid(pfn) || !page_is_ram(pfn))
+				continue;
+
+			page = pfn_to_page(pfn);
+			paddr = page_to_phys(page);
+
+			/* If pg will be MACed by tboot, no need to MAC here */
+			for (j = 0; j < tboot->num_mac_regions; j++) {
+				rstart = tboot->mac_regions[j].start;
+				rend = rstart +	tboot->mac_regions[j].size;
+				if (((paddr + PAGE_SIZE) <= rstart)
+					|| (rend <= paddr))
+					continue;
+				break;
+			}
+
+			if (j == tboot->num_mac_regions) {
+				sg_set_page(sg, page, PAGE_SIZE, 0);
+				ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
+				if (ret)
+					return ret;
+			}
+		}
+	}
+	ret = crypto_hash_final(&desc, (uint8_t *)mac);
+	if (ret)
+		return ret;
+
+	/* Clean the key */
+	memset(zeroed_key, 0, sizeof(zeroed_key));
+	crypto_hash_setkey(desc.tfm, zeroed_key, VMAC_KEY_LEN);
+
+	return 0;
+}
+
  #ifdef CONFIG_ACPI_SLEEP

  static void add_mac_region(phys_addr_t start, unsigned long size)
@@ -197,6 +277,16 @@ static int tboot_setup_sleep(void)
  	/* kernel code + data + bss */
  	add_mac_region(virt_to_phys(_text), _end - _text);

+	/* stack */
+	add_mac_region(virt_to_phys(current_thread_info()), THREAD_SIZE);
+
+	/* MAC userspace memory not handled by tboot */
+	get_random_bytes(tboot->s3_key, sizeof(tboot->s3_key));
+	if (tboot_gen_mem_integrity(tboot->s3_key, &mem_mac)) {
+		panic("tboot: vmac generation failed\n");
+		return -1;
+	}
+
  	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;

  	return 0;
@@ -212,6 +302,68 @@ static int tboot_setup_sleep(void)
  }

  #endif
+
+static struct thread_info *low_ti, *current_ti;
+
+static void tboot_do_stack_switch(struct thread_info *new_ti,
+				struct thread_info *old_ti)
+{
+	memcpy(new_ti, old_ti, THREAD_SIZE);
+#ifdef CONFIG_X86_32
+	asm volatile (
+		" and %0, %%esp;        "
+		" add %1, %%esp;        "
+		: : "i" (THREAD_SIZE - 1), "r" (new_ti));
+#else
+	asm volatile (
+		" and %0, %%rsp;        "
+		" add %1, %%rsp;        "
+		: : "i" (THREAD_SIZE - 1), "r" (new_ti));
+	percpu_write(kernel_stack, (unsigned long)new_ti -
+				KERNEL_STACK_OFFSET + THREAD_SIZE);
+#endif
+	current->stack = new_ti;
+}
+
+void tboot_switch_stack(void)
+{
+	if (!tboot_enabled())
+		return;
+
+	current_ti = current_thread_info();
+
+	/* If thread info is above 4G, then switch stack */
+	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_ti)))
+		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
+		& 0xffffffff00000000ULL))
+		return;
+
+	if (low_ti == NULL)
+		low_ti = (struct thread_info *)
+				__get_free_pages(GFP_DMA32, THREAD_ORDER);
+
+	tboot_do_stack_switch(low_ti, current_ti);
+}
+
+void tboot_restore_stack(void)
+{
+	if (!tboot_enabled())
+		return;
+
+	if (current_ti == NULL)
+		BUG();
+
+	/* If thread info is above 4G, then restore stack */
+	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_ti)))
+		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
+		& 0xffffffff00000000ULL))
+		return;
+
+	if (low_ti == NULL)
+		BUG();
+
+	tboot_do_stack_switch(current_ti, low_ti);
+}

  void tboot_shutdown(u32 shutdown_type)
  {
@@ -292,6 +444,24 @@ void tboot_sleep(u8 sleep_state, u32 pm1
  	}

  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
+}
+
+void tboot_sx_resume(void)
+{
+	vmac_t mac;
+
+	if (!tboot_enabled())
+		return;
+
+	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
+		panic("tboot: vmac generation failed\n");
+	else if (mac != mem_mac)
+		panic("tboot: memory integrity was lost on resume\n");
+	else
+		pr_info("memory integrity OK\n");
+
+	/* Clean s3_key */
+	memset(tboot->s3_key, 0, sizeof(tboot->s3_key));
  }

  static atomic_t ap_wfs_count;
diff -r 42870e183bd5 drivers/acpi/sleep.c
--- a/drivers/acpi/sleep.c	Fri Sep 25 06:06:39 2009 -0400
+++ b/drivers/acpi/sleep.c	Fri Sep 25 11:03:04 2009 -0400
@@ -16,6 +16,7 @@
  #include <linux/device.h>
  #include <linux/suspend.h>
  #include <linux/reboot.h>
+#include <linux/tboot.h>

  #include <asm/io.h>

@@ -244,7 +245,10 @@ static int acpi_suspend_enter(suspend_st
  		break;

  	case ACPI_STATE_S3:
+		tboot_switch_stack();
  		do_suspend_lowlevel();
+		tboot_sx_resume();
+		tboot_restore_stack();
  		break;
  	}

diff -r 42870e183bd5 include/linux/tboot.h
--- a/include/linux/tboot.h	Fri Sep 25 06:06:39 2009 -0400
+++ b/include/linux/tboot.h	Fri Sep 25 11:03:04 2009 -0400
@@ -147,7 +147,9 @@ extern struct acpi_table_header *tboot_g
  extern struct acpi_table_header *tboot_get_dmar_table(
  				      struct acpi_table_header *dmar_tbl);
  extern int tboot_force_iommu(void);
-
+extern void tboot_sx_resume(void);
+extern void tboot_switch_stack(void);
+extern void tboot_restore_stack(void);
  #else

  #define tboot_probe()			do { } while (0)
@@ -156,6 +158,9 @@ extern int tboot_force_iommu(void);
  					do { } while (0)
  #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
  #define tboot_force_iommu()		0
+#define tboot_sx_resume()		do { } while (0)
+#define tboot_switch_stack()		do { } while (0)
+#define tboot_restore_stack()		do { } while (0)

  #endif /* !CONFIG_INTEL_TXT */

diff -r 42870e183bd5 security/Kconfig
--- a/security/Kconfig	Fri Sep 25 06:06:39 2009 -0400
+++ b/security/Kconfig	Fri Sep 25 11:03:04 2009 -0400
@@ -116,6 +116,8 @@ config INTEL_TXT
  config INTEL_TXT
  	bool "Enable Intel(R) Trusted Execution Technology (Intel(R) TXT)"
  	depends on HAVE_INTEL_TXT
+	select CRYPTO_VMAC
+	select CRYPTO_AES
  	help
  	  This option enables support for booting the kernel with the
  	  Trusted Boot (tboot) module. This will utilize

[-- Attachment #2: s3_integrity.patch --]
[-- Type: text/plain, Size: 7374 bytes --]

This patch added verification for userspace memory integrity after s3 resume.
Integrity verification for other memory (say kernel itself) has been done by tboot.

Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

diff -r 42870e183bd5 arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Fri Sep 25 06:06:39 2009 -0400
+++ b/arch/x86/kernel/tboot.c	Fri Sep 25 11:03:04 2009 -0400
@@ -20,6 +20,7 @@
  */
 
 #include <linux/dma_remapping.h>
+#include <linux/scatterlist.h>
 #include <linux/init_task.h>
 #include <linux/spinlock.h>
 #include <linux/delay.h>
@@ -30,12 +31,17 @@
 #include <linux/pfn.h>
 #include <linux/mm.h>
 #include <linux/tboot.h>
+#include <linux/random.h>
+
+#include <crypto/vmac.h>
+#include <crypto/hash.h>
 
 #include <asm/trampoline.h>
 #include <asm/processor.h>
 #include <asm/bootparam.h>
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
+#include <asm/percpu.h>
 #include <asm/fixmap.h>
 #include <asm/proto.h>
 #include <asm/setup.h>
@@ -168,6 +174,80 @@ static void tboot_create_trampoline(void
 		      map_base, map_size);
 }
 
+static vmac_t mem_mac;
+static struct crypto_hash *tfm;
+
+static int tboot_gen_mem_integrity(const uint8_t key[], vmac_t *mac)
+{
+	int i, j, ret;
+	pg_data_t *pgdat;
+	struct hash_desc desc;
+	struct scatterlist sg[1];
+	struct page *page;
+	uint64_t paddr, rstart, rend;
+	unsigned long pfn;
+	uint8_t zeroed_key[VMAC_KEY_LEN];
+
+	if (!tfm)
+		tfm = crypto_alloc_hash("vmac(aes)", 0, CRYPTO_ALG_ASYNC);
+
+	if (IS_ERR(tfm)) {
+		tfm = NULL;
+		return -ENOMEM;
+	}
+
+	desc.tfm = tfm;
+	desc.flags = 0;
+
+	sg_init_table(sg, 1);
+
+	ret = crypto_hash_init(&desc);
+	if (ret)
+		return ret;
+	ret = crypto_hash_setkey(desc.tfm, key, VMAC_KEY_LEN);
+	if (ret)
+		return ret;
+
+	for_each_online_pgdat(pgdat) {
+		for (i = 0, pfn = pgdat->node_start_pfn;
+			i < pgdat->node_spanned_pages;
+			i++, pfn = pgdat->node_start_pfn + i) {
+
+			if (!pfn_valid(pfn) || !page_is_ram(pfn))
+				continue;
+
+			page = pfn_to_page(pfn);
+			paddr = page_to_phys(page);
+
+			/* If pg will be MACed by tboot, no need to MAC here */
+			for (j = 0; j < tboot->num_mac_regions; j++) {
+				rstart = tboot->mac_regions[j].start;
+				rend = rstart +	tboot->mac_regions[j].size;
+				if (((paddr + PAGE_SIZE) <= rstart)
+					|| (rend <= paddr))
+					continue;
+				break;
+			}
+
+			if (j == tboot->num_mac_regions) {
+				sg_set_page(sg, page, PAGE_SIZE, 0);
+				ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
+				if (ret)
+					return ret;
+			}
+		}
+	}
+	ret = crypto_hash_final(&desc, (uint8_t *)mac);
+	if (ret)
+		return ret;
+
+	/* Clean the key */
+	memset(zeroed_key, 0, sizeof(zeroed_key));
+	crypto_hash_setkey(desc.tfm, zeroed_key, VMAC_KEY_LEN);
+
+	return 0;
+}
+
 #ifdef CONFIG_ACPI_SLEEP
 
 static void add_mac_region(phys_addr_t start, unsigned long size)
@@ -197,6 +277,16 @@ static int tboot_setup_sleep(void)
 	/* kernel code + data + bss */
 	add_mac_region(virt_to_phys(_text), _end - _text);
 
+	/* stack */
+	add_mac_region(virt_to_phys(current_thread_info()), THREAD_SIZE);
+
+	/* MAC userspace memory not handled by tboot */
+	get_random_bytes(tboot->s3_key, sizeof(tboot->s3_key));
+	if (tboot_gen_mem_integrity(tboot->s3_key, &mem_mac)) {
+		panic("tboot: vmac generation failed\n");
+		return -1;
+	}
+
 	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
 
 	return 0;
@@ -212,6 +302,68 @@ static int tboot_setup_sleep(void)
 }
 
 #endif
+
+static struct thread_info *low_ti, *current_ti;
+
+static void tboot_do_stack_switch(struct thread_info *new_ti,
+				struct thread_info *old_ti)
+{
+	memcpy(new_ti, old_ti, THREAD_SIZE);
+#ifdef CONFIG_X86_32
+	asm volatile (
+		" and %0, %%esp;        "
+		" add %1, %%esp;        "
+		: : "i" (THREAD_SIZE - 1), "r" (new_ti));
+#else
+	asm volatile (
+		" and %0, %%rsp;        "
+		" add %1, %%rsp;        "
+		: : "i" (THREAD_SIZE - 1), "r" (new_ti));
+	percpu_write(kernel_stack, (unsigned long)new_ti -
+				KERNEL_STACK_OFFSET + THREAD_SIZE);
+#endif
+	current->stack = new_ti;
+}
+
+void tboot_switch_stack(void)
+{
+	if (!tboot_enabled())
+		return;
+
+	current_ti = current_thread_info();
+
+	/* If thread info is above 4G, then switch stack */
+	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_ti)))
+		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
+		& 0xffffffff00000000ULL))
+		return;
+
+	if (low_ti == NULL)
+		low_ti = (struct thread_info *)
+				__get_free_pages(GFP_DMA32, THREAD_ORDER);
+
+	tboot_do_stack_switch(low_ti, current_ti);
+}
+
+void tboot_restore_stack(void)
+{
+	if (!tboot_enabled())
+		return;
+
+	if (current_ti == NULL)
+		BUG();
+
+	/* If thread info is above 4G, then restore stack */
+	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_ti)))
+		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
+		& 0xffffffff00000000ULL))
+		return;
+
+	if (low_ti == NULL)
+		BUG();
+
+	tboot_do_stack_switch(current_ti, low_ti);
+}
 
 void tboot_shutdown(u32 shutdown_type)
 {
@@ -292,6 +444,24 @@ void tboot_sleep(u8 sleep_state, u32 pm1
 	}
 
 	tboot_shutdown(acpi_shutdown_map[sleep_state]);
+}
+
+void tboot_sx_resume(void)
+{
+	vmac_t mac;
+
+	if (!tboot_enabled())
+		return;
+
+	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
+		panic("tboot: vmac generation failed\n");
+	else if (mac != mem_mac)
+		panic("tboot: memory integrity was lost on resume\n");
+	else
+		pr_info("memory integrity OK\n");
+
+	/* Clean s3_key */
+	memset(tboot->s3_key, 0, sizeof(tboot->s3_key));
 }
 
 static atomic_t ap_wfs_count;
diff -r 42870e183bd5 drivers/acpi/sleep.c
--- a/drivers/acpi/sleep.c	Fri Sep 25 06:06:39 2009 -0400
+++ b/drivers/acpi/sleep.c	Fri Sep 25 11:03:04 2009 -0400
@@ -16,6 +16,7 @@
 #include <linux/device.h>
 #include <linux/suspend.h>
 #include <linux/reboot.h>
+#include <linux/tboot.h>
 
 #include <asm/io.h>
 
@@ -244,7 +245,10 @@ static int acpi_suspend_enter(suspend_st
 		break;
 
 	case ACPI_STATE_S3:
+		tboot_switch_stack();
 		do_suspend_lowlevel();
+		tboot_sx_resume();
+		tboot_restore_stack();
 		break;
 	}
 
diff -r 42870e183bd5 include/linux/tboot.h
--- a/include/linux/tboot.h	Fri Sep 25 06:06:39 2009 -0400
+++ b/include/linux/tboot.h	Fri Sep 25 11:03:04 2009 -0400
@@ -147,7 +147,9 @@ extern struct acpi_table_header *tboot_g
 extern struct acpi_table_header *tboot_get_dmar_table(
 				      struct acpi_table_header *dmar_tbl);
 extern int tboot_force_iommu(void);
-
+extern void tboot_sx_resume(void);
+extern void tboot_switch_stack(void);
+extern void tboot_restore_stack(void);
 #else
 
 #define tboot_probe()			do { } while (0)
@@ -156,6 +158,9 @@ extern int tboot_force_iommu(void);
 					do { } while (0)
 #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
 #define tboot_force_iommu()		0
+#define tboot_sx_resume()		do { } while (0)
+#define tboot_switch_stack()		do { } while (0)
+#define tboot_restore_stack()		do { } while (0)
 
 #endif /* !CONFIG_INTEL_TXT */
 
diff -r 42870e183bd5 security/Kconfig
--- a/security/Kconfig	Fri Sep 25 06:06:39 2009 -0400
+++ b/security/Kconfig	Fri Sep 25 11:03:04 2009 -0400
@@ -116,6 +116,8 @@ config INTEL_TXT
 config INTEL_TXT
 	bool "Enable Intel(R) Trusted Execution Technology (Intel(R) TXT)"
 	depends on HAVE_INTEL_TXT
+	select CRYPTO_VMAC
+	select CRYPTO_AES
 	help
 	  This option enables support for booting the kernel with the
 	  Trusted Boot (tboot) module. This will utilize

[PATCH] intel_txt: fix the buggy timeout warning logic in tboot

[Shane Wang]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

This patch based on an original version by H. Peter Anvin fixed the buggy 
timeout warning logic in tboot.

---
  arch/x86/kernel/tboot.c |    9 +++++----
  1 file changed, 5 insertions(+), 4 deletions(-)


Signed-off-by: Shane Wang <shane.wang@intel.com>

diff -r 0edd117ada44 arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Wed Sep 23 10:06:56 2009 -0700
+++ b/arch/x86/kernel/tboot.c	Mon Sep 28 07:42:18 2009 -0700
@@ -301,16 +301,17 @@ static int tboot_wait_for_aps(int num_ap
  	unsigned long timeout;

  	timeout = AP_WAIT_TIMEOUT*HZ;
-	while (atomic_read((atomic_t *)&tboot->num_in_wfs) != num_aps &&
-	       timeout) {
+	while (timeout) {
+		if (atomic_read((atomic_t *)&tboot->num_in_wfs) == num_aps)
+			break;
  		mdelay(1);
  		timeout--;
  	}

-	if (timeout)
+	if (!timeout)
  		pr_warning("tboot wait for APs timeout\n");

-	return !(atomic_read((atomic_t *)&tboot->num_in_wfs) == num_aps);
+	return !timeout;
  }

  static int __cpuinit tboot_cpu_callback(struct notifier_block *nfb,

[-- Attachment #2: timeout_fix.patch --]
[-- Type: text/plain, Size: 902 bytes --]

This patch based on an original version by H. Peter Anvin fixed the buggy timeout warning logic in tboot.

Signed-off-by: Shane Wang <shane.wang@intel.com>

diff -r 0edd117ada44 arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Wed Sep 23 10:06:56 2009 -0700
+++ b/arch/x86/kernel/tboot.c	Mon Sep 28 07:42:18 2009 -0700
@@ -301,16 +301,17 @@ static int tboot_wait_for_aps(int num_ap
 	unsigned long timeout;
 
 	timeout = AP_WAIT_TIMEOUT*HZ;
-	while (atomic_read((atomic_t *)&tboot->num_in_wfs) != num_aps &&
-	       timeout) {
+	while (timeout) {
+		if (atomic_read((atomic_t *)&tboot->num_in_wfs) == num_aps)
+			break;
 		mdelay(1);
 		timeout--;
 	}
 
-	if (timeout)
+	if (!timeout)
 		pr_warning("tboot wait for APs timeout\n");
 
-	return !(atomic_read((atomic_t *)&tboot->num_in_wfs) == num_aps);
+	return !timeout;
 }
 
 static int __cpuinit tboot_cpu_callback(struct notifier_block *nfb,

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Pavel Machek]

On Sun 2009-09-27 17:07:28, Shane Wang wrote:
> This patch added verification for userspace memory integrity after s3 resume.
> Integrity verification for other memory (say kernel itself) has been done by tboot.
>

AFAICT, it verifies userspace _and_ kernel memory, that's why it does
magic stack switching. Why not verify everything in tboot?

Is kernel<->tboot abi described somewhere?

> @@ -168,6 +174,80 @@ static void tboot_create_trampoline(void
>  		      map_base, map_size);
>  }
>
> +static vmac_t mem_mac;
> +static struct crypto_hash *tfm;

Could these be automatic?

> +	for_each_online_pgdat(pgdat) {
> +		for (i = 0, pfn = pgdat->node_start_pfn;
> +			i < pgdat->node_spanned_pages;
> +			i++, pfn = pgdat->node_start_pfn + i) {
> +
> +			if (!pfn_valid(pfn) || !page_is_ram(pfn))
> +				continue;
> +
> +			page = pfn_to_page(pfn);
> +			paddr = page_to_phys(page);
> +
> +			/* If pg will be MACed by tboot, no need to MAC here */
> +			for (j = 0; j < tboot->num_mac_regions; j++) {
> +				rstart = tboot->mac_regions[j].start;
> +				rend = rstart +	tboot->mac_regions[j].size;
> +				if (((paddr + PAGE_SIZE) <= rstart)
> +					|| (rend <= paddr))
> +					continue;
> +				break;
> +			}
> +
> +			if (j == tboot->num_mac_regions) {
> +				sg_set_page(sg, page, PAGE_SIZE, 0);
> +				ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
> +				if (ret)
> +					return ret;
> +			}
> +		}
> +	}

This looks over all memory, afaict. Does it correctly handle highmem?

> @@ -197,6 +277,16 @@ static int tboot_setup_sleep(void)
>  	/* kernel code + data + bss */
>  	add_mac_region(virt_to_phys(_text), _end - _text);
>
> +	/* stack */
> +	add_mac_region(virt_to_phys(current_thread_info()), THREAD_SIZE);
> +
> +	/* MAC userspace memory not handled by tboot */
> +	get_random_bytes(tboot->s3_key, sizeof(tboot->s3_key));
> +	if (tboot_gen_mem_integrity(tboot->s3_key, &mem_mac)) {
> +		panic("tboot: vmac generation failed\n");
> +		return -1;
> +	}
> +
>  	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
>
>  	return 0;
> @@ -212,6 +302,68 @@ static int tboot_setup_sleep(void)
>  }
>
>  #endif
> +
> +static struct thread_info *low_ti, *current_ti;
> +
> +static void tboot_do_stack_switch(struct thread_info *new_ti,
> +				struct thread_info *old_ti)
> +{
> +	memcpy(new_ti, old_ti, THREAD_SIZE);

Memcpy does some rather odd stuff with fpu in some
configs. Like... you may get copy of thread info with fpu marked
enabled, while it really is disabled when memcpy exits.

> +#ifdef CONFIG_X86_32
> +	asm volatile (
> +		" and %0, %%esp;        "
> +		" add %1, %%esp;        "
> +		: : "i" (THREAD_SIZE - 1), "r" (new_ti));
> +#else
> +	asm volatile (
> +		" and %0, %%rsp;        "
> +		" add %1, %%rsp;        "
> +		: : "i" (THREAD_SIZE - 1), "r" (new_ti));
> +	percpu_write(kernel_stack, (unsigned long)new_ti -
> +				KERNEL_STACK_OFFSET + THREAD_SIZE);
> +#endif
> +	current->stack = new_ti;
> +}

This is pretty subtle&fragile -- see memcpy above. Why is it needed?

> +	current_ti = current_thread_info();
> +
> +	/* If thread info is above 4G, then switch stack */
> +	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_ti)))
> +		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
> +		& 0xffffffff00000000ULL))
> +		return;

How can thread info be above 4G on 32-bit?

Why does 4G limit matter on 64-bit?

> +void tboot_sx_resume(void)
> +{
> +	vmac_t mac;
> +
> +	if (!tboot_enabled())
> +		return;
> +
> +	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
> +		panic("tboot: vmac generation failed\n");
> +	else if (mac != mem_mac)
> +		panic("tboot: memory integrity was lost on resume\n");
> +	else
> +		pr_info("memory integrity OK\n");

So I corrupt memory, but also corrupt tboot_enabled() to return 0....

And... does panic kill the machine quickly enough that no 'bad stuff'
happens? (Whats bad stuff in this context, anyway?).

> @@ -244,7 +245,10 @@ static int acpi_suspend_enter(suspend_st
>  		break;
>
>  	case ACPI_STATE_S3:
> +		tboot_switch_stack();
>  		do_suspend_lowlevel();
> +		tboot_sx_resume();
> +		tboot_restore_stack();
>  		break;
>  	}
>

Did you audit all code before sx_resume()? If it trusts  data not
checksummed by tboot, attacker may be able to hijack code execution
and bypass your protection, no? 

What about S1? 

And what about hibernation, btw?
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Andi Kleen]

> So I corrupt memory, but also corrupt tboot_enabled() to return 0....
> 
> And... does panic kill the machine quickly enough that no 'bad stuff'
> happens? (Whats bad stuff in this context, anyway?).

panic has notifiers, so indeed someone could put arbitary code into panic.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

RE: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Wang, Shane]

part of answers.
Pavel Machek wrote:
> On Sun 2009-09-27 17:07:28, Shane Wang wrote:
>> This patch added verification for userspace memory integrity after
>> s3 resume. Integrity verification for other memory (say kernel
>> itself) has been done by tboot. 
>> 
> 
> AFAICT, it verifies userspace _and_ kernel memory, that's why it does
> magic stack switching. Why not verify everything in tboot?
Because tboot only can access <4G mem and the memory is sparse.
Tboot likes to MAC the continuous mem.

> 
> Is kernel<->tboot abi described somewhere?
> 
>> @@ -168,6 +174,80 @@ static void tboot_create_trampoline(void  		   
>>  map_base, map_size); }
>> 
>> +static vmac_t mem_mac;
>> +static struct crypto_hash *tfm;
> 
> Could these be automatic?
We don't wish the memory is changing when MACing, including the static variables.

>> +void tboot_sx_resume(void)
>> +{
>> +	vmac_t mac;
>> +
>> +	if (!tboot_enabled())
>> +		return;
>> +
>> +	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
>> +		panic("tboot: vmac generation failed\n");
>> +	else if (mac != mem_mac)
>> +		panic("tboot: memory integrity was lost on resume\n"); +	else
>> +		pr_info("memory integrity OK\n");
> 
> So I corrupt memory, but also corrupt tboot_enabled() to return 0....
You corrupt the memory and tboot_enabled(). tboot MACing will find it.

 
> And... does panic kill the machine quickly enough that no 'bad stuff'
> happens? (Whats bad stuff in this context, anyway?).
Do you have some suggestions on it?

Shane

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Pavel Machek]

Hi!

Please wrap mails at column 72 (or so).

> > AFAICT, it verifies userspace _and_ kernel memory, that's why it does
> > magic stack switching. Why not verify everything in tboot?
> Because tboot only can access <4G mem without paging. And the memory is sparse. We can't/needn't set unlimited sparse mem ranges to the MAC array with limited elements in the shared page, in order to pass the parameters.
> On the other hand, it is reasonable for tboot to verify kernel, and kernel to verify userspace memory.
>

Are  you sure x86-64 kernel & modules is always below 4GB? I don't
think so.

 
> >> +static vmac_t mem_mac;
> >> +static struct crypto_hash *tfm;
> > 
> > Could these be automatic?
> Maybe, but I don't wish other files can access the variables and take tfm as an example, I'd like to allocate memory to it once and then initialize it once in order to avoid impact of memory change to MACing.
>

You use stack, anyway.

> > Why does 4G limit matter on 64-bit?
> tboot can't access >4G, see above.

Too bad, then its broken by design.

> >> +	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
> >> +		panic("tboot: vmac generation failed\n");
> >> +	else if (mac != mem_mac)
> >> +		panic("tboot: memory integrity was lost on resume\n"); +	else
> >> +		pr_info("memory integrity OK\n");
> > 
> > So I corrupt memory, but also corrupt tboot_enabled() to return 0....
> > 
> > And... does panic kill the machine quickly enough that no 'bad stuff'
> > happens? (Whats bad stuff in this context, anyway?).

I'd really like you to answer that.


> >> @@ -244,7 +245,10 @@ static int acpi_suspend_enter(suspend_st 
> >> break; 
> >> 
> >>  	case ACPI_STATE_S3:
> >> +		tboot_switch_stack();
> >>  		do_suspend_lowlevel();
> >> +		tboot_sx_resume();
> >> +		tboot_restore_stack();
> >>  		break;
> >>  	}
> >> 
> > 
> > Did you audit all code before sx_resume()? If it trusts  data not
> > checksummed by tboot, attacker may be able to hijack code execution
> > and bypass your protection, no?
> Yes, kernel code is audited by tboot before resume.

So no, you did not audit do_suspend_lowlevel to make sure it does not
follow function pointers. Bad.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Pavel Machek]

> This patch added verification for userspace memory integrity after
>  S3 resume.

It does not work.

> Integrity verification for other memory (say kernel itself) has been done by tboot.
>

Not true. Kernel  uses memory above 4G on x86-64. Including... say
console writing functions.

You can patch holes, but without description 'what does this protect
against' it is almost impossible to evaluate.


> +void tboot_do_suspend_lowlevel(void)
> +{
> +	int ret = -1;
> +
> +	if (!tboot_enabled()) {
> +		do_suspend_lowlevel();
> +		return;
> +	}
> +
> +	ret = tboot_pre_stack_switch();
> +	if (!ret) {
> +		tboot_switch_stack_call(tboot_do_suspend_lowlevel_call,
> +					(u64)new_stack_ptr);

...and here you add requirements to suspend_lowlevel that were not
there before. ("May not act on unchecksummed memory"), without
documenting them.

NAK.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

RE: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Wang, Shane]

(Sorry for so late response)

Thanks.
Shane

Pavel Machek wrote:
> On Sun 2009-09-27 17:07:28, Shane Wang wrote:
>> This patch added verification for userspace memory integrity after
>> s3 resume. Integrity verification for other memory (say kernel
>> itself) has been done by tboot. 
>> 
> 
> AFAICT, it verifies userspace _and_ kernel memory, that's why it does
> magic stack switching. Why not verify everything in tboot?
Because tboot only can access <4G mem without paging. And the memory is sparse. We can't/needn't set unlimited sparse mem ranges to the MAC array with limited elements in the shared page, in order to pass the parameters.
On the other hand, it is reasonable for tboot to verify kernel, and kernel to verify userspace memory.

>> @@ -168,6 +174,80 @@ static void tboot_create_trampoline(void  		   
>>  map_base, map_size); }
>> 
>> +static vmac_t mem_mac;
>> +static struct crypto_hash *tfm;
> 
> Could these be automatic?
Maybe, but I don't wish other files can access the variables and take tfm as an example, I'd like to allocate memory to it once and then initialize it once in order to avoid impact of memory change to MACing.

> 
>> +	for_each_online_pgdat(pgdat) {
>> +		for (i = 0, pfn = pgdat->node_start_pfn;
>> +			i < pgdat->node_spanned_pages;
>> +			i++, pfn = pgdat->node_start_pfn + i) {
>> +
>> +			if (!pfn_valid(pfn) || !page_is_ram(pfn))
>> +				continue;
>> +
>> +			page = pfn_to_page(pfn);
>> +			paddr = page_to_phys(page);
>> +
>> +			/* If pg will be MACed by tboot, no need to MAC here */
>> +			for (j = 0; j < tboot->num_mac_regions; j++) {
>> +				rstart = tboot->mac_regions[j].start;
>> +				rend = rstart +	tboot->mac_regions[j].size;
>> +				if (((paddr + PAGE_SIZE) <= rstart)
>> +					|| (rend <= paddr))
>> +					continue;
>> +				break;
>> +			}
>> +
>> +			if (j == tboot->num_mac_regions) {
>> +				sg_set_page(sg, page, PAGE_SIZE, 0);
>> +				ret = crypto_hash_update(&desc, sg, PAGE_SIZE); +				if (ret)
>> +					return ret;
>> +			}
>> +		}
>> +	}
> 
> This looks over all memory, afaict. Does it correctly handle highmem?
It looks over all online memory, and should handle highmem since highmem has been added into the online nodes.
refer to show_mem() in lib/show_mem.c.

> 
>> @@ -197,6 +277,16 @@ static int tboot_setup_sleep(void)
>>  	/* kernel code + data + bss */
>>  	add_mac_region(virt_to_phys(_text), _end - _text);
>> 
>> +	/* stack */
>> +	add_mac_region(virt_to_phys(current_thread_info()), THREAD_SIZE); +
>> +	/* MAC userspace memory not handled by tboot */
>> +	get_random_bytes(tboot->s3_key, sizeof(tboot->s3_key));
>> +	if (tboot_gen_mem_integrity(tboot->s3_key, &mem_mac)) {
>> +		panic("tboot: vmac generation failed\n");
>> +		return -1;
>> +	}
>> +
>>  	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
>> 
>>  	return 0;
>> @@ -212,6 +302,68 @@ static int tboot_setup_sleep(void)  }
>> 
>>  #endif
>> +
>> +static struct thread_info *low_ti, *current_ti;
>> +
>> +static void tboot_do_stack_switch(struct thread_info *new_ti,
>> +				struct thread_info *old_ti)
>> +{
>> +	memcpy(new_ti, old_ti, THREAD_SIZE);
> 
> Memcpy does some rather odd stuff with fpu in some
> configs. Like... you may get copy of thread info with fpu marked
> enabled, while it really is disabled when memcpy exits.
We gave up thread_info but stack, like interrupt stack and call.

> 
>> +#ifdef CONFIG_X86_32
>> +	asm volatile (
>> +		" and %0, %%esp;        "
>> +		" add %1, %%esp;        "
>> +		: : "i" (THREAD_SIZE - 1), "r" (new_ti));
>> +#else
>> +	asm volatile (
>> +		" and %0, %%rsp;        "
>> +		" add %1, %%rsp;        "
>> +		: : "i" (THREAD_SIZE - 1), "r" (new_ti));
>> +	percpu_write(kernel_stack, (unsigned long)new_ti -
>> +				KERNEL_STACK_OFFSET + THREAD_SIZE);
>> +#endif
>> +	current->stack = new_ti;
>> +}
> 
> This is pretty subtle&fragile -- see memcpy above. Why is it needed?
Gave it up.

> 
>> +	current_ti = current_thread_info();
>> +
>> +	/* If thread info is above 4G, then switch stack */
>> +	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_ti)))
>> +		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
>> +		& 0xffffffff00000000ULL))
>> +		return;
> 
> How can thread info be above 4G on 32-bit?
OK, I fixed it.

> 
> Why does 4G limit matter on 64-bit?
tboot can't access >4G, see above.

> 
>> +void tboot_sx_resume(void)
>> +{
>> +	vmac_t mac;
>> +
>> +	if (!tboot_enabled())
>> +		return;
>> +
>> +	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
>> +		panic("tboot: vmac generation failed\n");
>> +	else if (mac != mem_mac)
>> +		panic("tboot: memory integrity was lost on resume\n"); +	else
>> +		pr_info("memory integrity OK\n");
> 
> So I corrupt memory, but also corrupt tboot_enabled() to return 0....
> 
> And... does panic kill the machine quickly enough that no 'bad stuff'
> happens? (Whats bad stuff in this context, anyway?).
> 
>> @@ -244,7 +245,10 @@ static int acpi_suspend_enter(suspend_st 
>> break; 
>> 
>>  	case ACPI_STATE_S3:
>> +		tboot_switch_stack();
>>  		do_suspend_lowlevel();
>> +		tboot_sx_resume();
>> +		tboot_restore_stack();
>>  		break;
>>  	}
>> 
> 
> Did you audit all code before sx_resume()? If it trusts  data not
> checksummed by tboot, attacker may be able to hijack code execution
> and bypass your protection, no?
Yes, kernel code is audited by tboot before resume.

> 
> What about S1?
> 
> And what about hibernation, btw
No need memory MACing for S1/S4, it is another way in the tboot code.
> 									Pavel

[PATCH] intel_txt: add s3 userspace memory integrity verification

[Shane Wang]

[-- Attachment #1: Type: text/plain, Size: 10630 bytes --]

This patch added verification for userspace memory integrity after S3 resume.
Integrity verification for other memory (say kernel itself) has been done by tboot.

Thanks
Shane

---
  arch/x86/kernel/entry_64.S |   20 +++
  arch/x86/kernel/tboot.c    |  226 +++++++++++++++++++++++++++++++++++
  drivers/acpi/sleep.c       |    3
  include/linux/mm.h         |    8 -
  include/linux/tboot.h      |    3
  security/Kconfig           |    2
  6 files changed, 256 insertions(+), 6 deletions(-)

Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

diff -r c878d454dc8b arch/x86/kernel/entry_64.S
--- a/arch/x86/kernel/entry_64.S	Wed Dec 02 01:06:32 2009 -0800
+++ b/arch/x86/kernel/entry_64.S	Thu Dec 03 07:22:17 2009 -0800
@@ -1275,6 +1275,26 @@ ENTRY(call_softirq)
  	CFI_ENDPROC
  END(call_softirq)

+#ifdef CONFIG_INTEL_TXT
+/* void tboot_switch_stack_call(void (*target_func)(void), u64 new_rsp) */
+ENTRY(tboot_switch_stack_call)
+	CFI_STARTPROC
+	push %rbp
+	CFI_ADJUST_CFA_OFFSET	8
+	CFI_REL_OFFSET	rbp,0
+	mov %rsp, %rbp
+	CFI_DEF_CFA_REGISTER	rbp
+	mov %rsi, %rsp
+	push %rbp
+	call *%rdi
+	leaveq
+	CFI_DEF_CFA_REGISTER	rsp
+	CFI_ADJUST_CFA_OFFSET	-8
+	ret
+	CFI_ENDPROC
+END(tboot_switch_stack_call)
+#endif
+
  #ifdef CONFIG_XEN
  zeroentry xen_hypervisor_callback xen_do_hypervisor_callback

diff -r c878d454dc8b arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Wed Dec 02 01:06:32 2009 -0800
+++ b/arch/x86/kernel/tboot.c	Thu Dec 03 07:22:17 2009 -0800
@@ -20,6 +20,7 @@
   */

  #include <linux/dma_remapping.h>
+#include <linux/scatterlist.h>
  #include <linux/init_task.h>
  #include <linux/spinlock.h>
  #include <linux/delay.h>
@@ -30,12 +31,17 @@
  #include <linux/pfn.h>
  #include <linux/mm.h>
  #include <linux/tboot.h>
+#include <linux/random.h>
+
+#include <crypto/vmac.h>
+#include <crypto/hash.h>

  #include <asm/trampoline.h>
  #include <asm/processor.h>
  #include <asm/bootparam.h>
  #include <asm/pgtable.h>
  #include <asm/pgalloc.h>
+#include <asm/percpu.h>
  #include <asm/fixmap.h>
  #include <asm/proto.h>
  #include <asm/setup.h>
@@ -168,6 +174,159 @@ static void tboot_create_trampoline(void
  		      map_base, map_size);
  }

+#ifdef CONFIG_X86_64
+static char *new_stack, *new_stack_ptr;
+
+static int tboot_pre_stack_switch(void)
+{
+	BUG_ON((new_stack != NULL) || (new_stack_ptr != NULL));
+
+	/*
+	 * as long as thread info is above 4G, then switch stack,
+	 * since tboot can't access >4G stack for MACing
+	 */
+	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_thread_info())))
+		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
+		& 0xffffffff00000000UL))
+		return -1;
+
+	new_stack = (char *)__get_free_pages(GFP_DMA32, IRQ_STACK_ORDER);
+
+	BUG_ON(new_stack == NULL);
+	memset(new_stack, 0, IRQ_STACK_SIZE);
+	new_stack_ptr = new_stack + IRQ_STACK_SIZE - 64;
+
+	return 0;
+}
+
+static void tboot_post_stack_switch(void)
+{
+	BUG_ON((new_stack == NULL) || (new_stack_ptr == NULL));
+
+	free_pages((unsigned long)new_stack, IRQ_STACK_ORDER);
+	new_stack = NULL;
+	new_stack_ptr = NULL;
+}
+
+extern void tboot_switch_stack_call(void (*target_func)(void), u64 new_rsp);
+
+#else /* CONFIG_X86_32 */
+
+#define tboot_pre_stack_switch()			(-1)
+#define tboot_post_stack_switch()			do { } while (0)
+#define tboot_switch_stack_call(target_func, new_rsp)	do { } while (0)
+
+#endif
+
+static vmac_t mem_mac;
+static struct crypto_hash *tfm;
+
+static int tboot_gen_mem_integrity(const uint8_t key[], vmac_t *mac)
+{
+	int i, j, ret;
+	pg_data_t *pgdat;
+	struct hash_desc desc;
+	struct scatterlist sg[1];
+	struct page *page;
+	uint64_t paddr, rstart, rend;
+	unsigned long pfn;
+	uint8_t zeroed_key[VMAC_KEY_LEN];
+
+	if (!tfm)
+		tfm = crypto_alloc_hash("vmac(aes)", 0, CRYPTO_ALG_ASYNC);
+
+	if (IS_ERR(tfm)) {
+		tfm = NULL;
+		return -ENOMEM;
+	}
+
+	desc.tfm = tfm;
+	desc.flags = 0;
+
+	sg_init_table(sg, 1);
+
+	ret = crypto_hash_init(&desc);
+	if (ret)
+		return ret;
+	ret = crypto_hash_setkey(desc.tfm, key, VMAC_KEY_LEN);
+	if (ret)
+		return ret;
+
+	for_each_online_pgdat(pgdat) {
+		unsigned long flags;
+
+		pgdat_resize_lock(pgdat, &flags);
+		for (i = 0, pfn = pgdat->node_start_pfn;
+			i < pgdat->node_spanned_pages;
+			i++, pfn = pgdat->node_start_pfn + i) {
+
+			if (!pfn_valid(pfn) || !page_is_ram(pfn))
+				continue;
+
+			page = pfn_to_page(pfn);
+			paddr = page_to_phys(page);
+
+			/* If pg will be MACed by tboot, no need to MAC here */
+			for (j = 0; j < tboot->num_mac_regions; j++) {
+				rstart = tboot->mac_regions[j].start;
+				rend = rstart +	tboot->mac_regions[j].size;
+				if (((paddr + PAGE_SIZE) <= rstart)
+					|| (rend <= paddr))
+					continue;
+				break;
+			}
+
+			if (j == tboot->num_mac_regions) {
+				sg_set_page(sg, page, PAGE_SIZE, 0);
+#ifdef CONFIG_DEBUG_PAGEALLOC
+			/*
+			 * check if the page we are going to MAC is marked as
+			 * present in the kernel page tables.
+			 */
+			if (!kernel_page_present(page)) {
+				kernel_map_pages(page, 1, 1);
+				ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
+				kernel_map_pages(page, 1, 0);
+			} else
+#endif
+				ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
+				if (ret) {
+					pgdat_resize_unlock(pgdat, &flags);
+					return ret;
+				}
+			}
+		}
+		pgdat_resize_unlock(pgdat, &flags);
+	}
+
+#ifdef CONFIG_X86_64
+	/*
+	 * for stack > 4G, we should MAC the stack in the kernel after switch,
+	 * for stack < 4G, the stack is MACed by tboot
+	 */
+	if (new_stack) {
+		for (i = 0, page = virt_to_page((void *)current_thread_info());
+			i < (1 << THREAD_ORDER);
+			i++, page++) {
+			sg_set_page(sg, page, PAGE_SIZE, 0);
+			ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
+			if (ret)
+				return ret;
+		}
+	}
+#endif
+
+	ret = crypto_hash_final(&desc, (uint8_t *)mac);
+	if (ret)
+		return ret;
+
+	/* Clean the key */
+	memset(zeroed_key, 0, sizeof(zeroed_key));
+	crypto_hash_setkey(desc.tfm, zeroed_key, VMAC_KEY_LEN);
+
+	return 0;
+}
+
  #ifdef CONFIG_ACPI_SLEEP

  static void add_mac_region(phys_addr_t start, unsigned long size)
@@ -196,6 +355,27 @@ static int tboot_setup_sleep(void)

  	/* kernel code + data + bss */
  	add_mac_region(virt_to_phys(_text), _end - _text);
+
+	/* stack */
+#ifdef CONFIG_X86_64
+	/*
+	 * if stack > 4G, we should MAC the stack in the kernel after switch,
+	 * if stack < 4G, the stack is MACed by tboot
+	 */
+	if (new_stack)
+		add_mac_region(virt_to_phys(new_stack),
+				IRQ_STACK_SIZE); /* > 4G */
+	else
+#endif
+		add_mac_region(virt_to_phys(current_thread_info()),
+				THREAD_SIZE); /* < 4G */
+
+	/* MAC userspace memory not handled by tboot */
+	get_random_bytes(tboot->s3_key, sizeof(tboot->s3_key));
+	if (tboot_gen_mem_integrity(tboot->s3_key, &mem_mac)) {
+		panic("tboot: vmac generation failed\n");
+		return -1;
+	}

  	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;

@@ -292,6 +472,52 @@ void tboot_sleep(u8 sleep_state, u32 pm1
  	}

  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
+}
+
+static void tboot_sx_resume(void)
+{
+	vmac_t mac;
+
+	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
+		panic("tboot: vmac generation failed\n");
+	else if (mac != mem_mac)
+#ifdef CONFIG_DEBUG_KERNEL
+		pr_debug("tboot: memory integrity %llx -> %llx\n",
+				mem_mac, mac);
+#else
+		panic("tboot: memory integrity was lost on resume\n");
+#endif
+	else
+		pr_info("memory integrity OK\n");
+
+	/* Clean s3_key */
+	memset(tboot->s3_key, 0, sizeof(tboot->s3_key));
+}
+
+extern void do_suspend_lowlevel(void);
+
+static void tboot_do_suspend_lowlevel_call(void)
+{
+	do_suspend_lowlevel();
+	tboot_sx_resume();
+}
+
+void tboot_do_suspend_lowlevel(void)
+{
+	int ret = -1;
+
+	if (!tboot_enabled()) {
+		do_suspend_lowlevel();
+		return;
+	}
+
+	ret = tboot_pre_stack_switch();
+	if (!ret) {
+		tboot_switch_stack_call(tboot_do_suspend_lowlevel_call,
+					(u64)new_stack_ptr);
+		tboot_post_stack_switch();
+	} else
+		tboot_do_suspend_lowlevel_call();
  }

  static atomic_t ap_wfs_count;
diff -r c878d454dc8b drivers/acpi/sleep.c
--- a/drivers/acpi/sleep.c	Wed Dec 02 01:06:32 2009 -0800
+++ b/drivers/acpi/sleep.c	Thu Dec 03 07:22:17 2009 -0800
@@ -16,6 +16,7 @@
  #include <linux/device.h>
  #include <linux/suspend.h>
  #include <linux/reboot.h>
+#include <linux/tboot.h>

  #include <asm/io.h>

@@ -244,7 +245,7 @@ static int acpi_suspend_enter(suspend_st
  		break;

  	case ACPI_STATE_S3:
-		do_suspend_lowlevel();
+		tboot_do_suspend_lowlevel();
  		break;
  	}

diff -r c878d454dc8b include/linux/mm.h
--- a/include/linux/mm.h	Wed Dec 02 01:06:32 2009 -0800
+++ b/include/linux/mm.h	Thu Dec 03 07:22:17 2009 -0800
@@ -1263,18 +1263,18 @@ static inline void enable_debug_pageallo
  {
  	debug_pagealloc_enabled = 1;
  }
-#ifdef CONFIG_HIBERNATION
+#if defined(CONFIG_HIBERNATION) || defined(CONFIG_INTEL_TXT)
  extern bool kernel_page_present(struct page *page);
-#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_HIBERNATION || CONFIG_INTEL_TXT */
  #else
  static inline void
  kernel_map_pages(struct page *page, int numpages, int enable) {}
  static inline void enable_debug_pagealloc(void)
  {
  }
-#ifdef CONFIG_HIBERNATION
+#if defined(CONFIG_HIBERNATION) || defined(CONFIG_INTEL_TXT)
  static inline bool kernel_page_present(struct page *page) { return true; }
-#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_HIBERNATION || CONFIG_INTEL_TXT */
  #endif

  extern struct vm_area_struct *get_gate_vma(struct task_struct *tsk);
diff -r c878d454dc8b include/linux/tboot.h
--- a/include/linux/tboot.h	Wed Dec 02 01:06:32 2009 -0800
+++ b/include/linux/tboot.h	Thu Dec 03 07:22:17 2009 -0800
@@ -147,7 +147,7 @@ extern struct acpi_table_header *tboot_g
  extern struct acpi_table_header *tboot_get_dmar_table(
  				      struct acpi_table_header *dmar_tbl);
  extern int tboot_force_iommu(void);
-
+extern void tboot_do_suspend_lowlevel(void);
  #else

  #define tboot_probe()			do { } while (0)
@@ -156,6 +156,7 @@ extern int tboot_force_iommu(void);
  					do { } while (0)
  #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
  #define tboot_force_iommu()		0
+#define tboot_do_suspend_lowlevel()	do_suspend_lowlevel()

  #endif /* !CONFIG_INTEL_TXT */

diff -r c878d454dc8b security/Kconfig
--- a/security/Kconfig	Wed Dec 02 01:06:32 2009 -0800
+++ b/security/Kconfig	Thu Dec 03 07:22:17 2009 -0800
@@ -116,6 +116,8 @@ config INTEL_TXT
  config INTEL_TXT
  	bool "Enable Intel(R) Trusted Execution Technology (Intel(R) TXT)"
  	depends on HAVE_INTEL_TXT
+	select CRYPTO_VMAC
+	select CRYPTO_AES
  	help
  	  This option enables support for booting the kernel with the
  	  Trusted Boot (tboot) module. This will utilize

[-- Attachment #2: s3.patch --]
[-- Type: text/plain, Size: 10252 bytes --]

This patch added verification for userspace memory integrity after S3 resume.
Integrity verification for other memory (say kernel itself) has been done by tboot.

Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

diff -r c878d454dc8b arch/x86/kernel/entry_64.S
--- a/arch/x86/kernel/entry_64.S	Wed Dec 02 01:06:32 2009 -0800
+++ b/arch/x86/kernel/entry_64.S	Thu Dec 03 07:22:17 2009 -0800
@@ -1275,6 +1275,26 @@ ENTRY(call_softirq)
 	CFI_ENDPROC
 END(call_softirq)
 
+#ifdef CONFIG_INTEL_TXT
+/* void tboot_switch_stack_call(void (*target_func)(void), u64 new_rsp) */
+ENTRY(tboot_switch_stack_call)
+	CFI_STARTPROC
+	push %rbp
+	CFI_ADJUST_CFA_OFFSET	8
+	CFI_REL_OFFSET	rbp,0
+	mov %rsp, %rbp
+	CFI_DEF_CFA_REGISTER	rbp
+	mov %rsi, %rsp
+	push %rbp
+	call *%rdi
+	leaveq
+	CFI_DEF_CFA_REGISTER	rsp
+	CFI_ADJUST_CFA_OFFSET	-8
+	ret
+	CFI_ENDPROC
+END(tboot_switch_stack_call)
+#endif
+
 #ifdef CONFIG_XEN
 zeroentry xen_hypervisor_callback xen_do_hypervisor_callback
 
diff -r c878d454dc8b arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Wed Dec 02 01:06:32 2009 -0800
+++ b/arch/x86/kernel/tboot.c	Thu Dec 03 07:22:17 2009 -0800
@@ -20,6 +20,7 @@
  */
 
 #include <linux/dma_remapping.h>
+#include <linux/scatterlist.h>
 #include <linux/init_task.h>
 #include <linux/spinlock.h>
 #include <linux/delay.h>
@@ -30,12 +31,17 @@
 #include <linux/pfn.h>
 #include <linux/mm.h>
 #include <linux/tboot.h>
+#include <linux/random.h>
+
+#include <crypto/vmac.h>
+#include <crypto/hash.h>
 
 #include <asm/trampoline.h>
 #include <asm/processor.h>
 #include <asm/bootparam.h>
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
+#include <asm/percpu.h>
 #include <asm/fixmap.h>
 #include <asm/proto.h>
 #include <asm/setup.h>
@@ -168,6 +174,159 @@ static void tboot_create_trampoline(void
 		      map_base, map_size);
 }
 
+#ifdef CONFIG_X86_64
+static char *new_stack, *new_stack_ptr;
+
+static int tboot_pre_stack_switch(void)
+{
+	BUG_ON((new_stack != NULL) || (new_stack_ptr != NULL));
+
+	/*
+	 * as long as thread info is above 4G, then switch stack,
+	 * since tboot can't access >4G stack for MACing
+	 */
+	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_thread_info())))
+		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
+		& 0xffffffff00000000UL))
+		return -1;
+
+	new_stack = (char *)__get_free_pages(GFP_DMA32, IRQ_STACK_ORDER);
+
+	BUG_ON(new_stack == NULL);
+	memset(new_stack, 0, IRQ_STACK_SIZE);
+	new_stack_ptr = new_stack + IRQ_STACK_SIZE - 64;
+
+	return 0;
+}
+
+static void tboot_post_stack_switch(void)
+{
+	BUG_ON((new_stack == NULL) || (new_stack_ptr == NULL));
+
+	free_pages((unsigned long)new_stack, IRQ_STACK_ORDER);
+	new_stack = NULL;
+	new_stack_ptr = NULL;
+}
+
+extern void tboot_switch_stack_call(void (*target_func)(void), u64 new_rsp);
+
+#else /* CONFIG_X86_32 */
+
+#define tboot_pre_stack_switch()			(-1)
+#define tboot_post_stack_switch()			do { } while (0)
+#define tboot_switch_stack_call(target_func, new_rsp)	do { } while (0)
+
+#endif
+
+static vmac_t mem_mac;
+static struct crypto_hash *tfm;
+
+static int tboot_gen_mem_integrity(const uint8_t key[], vmac_t *mac)
+{
+	int i, j, ret;
+	pg_data_t *pgdat;
+	struct hash_desc desc;
+	struct scatterlist sg[1];
+	struct page *page;
+	uint64_t paddr, rstart, rend;
+	unsigned long pfn;
+	uint8_t zeroed_key[VMAC_KEY_LEN];
+
+	if (!tfm)
+		tfm = crypto_alloc_hash("vmac(aes)", 0, CRYPTO_ALG_ASYNC);
+
+	if (IS_ERR(tfm)) {
+		tfm = NULL;
+		return -ENOMEM;
+	}
+
+	desc.tfm = tfm;
+	desc.flags = 0;
+
+	sg_init_table(sg, 1);
+
+	ret = crypto_hash_init(&desc);
+	if (ret)
+		return ret;
+	ret = crypto_hash_setkey(desc.tfm, key, VMAC_KEY_LEN);
+	if (ret)
+		return ret;
+
+	for_each_online_pgdat(pgdat) {
+		unsigned long flags;
+
+		pgdat_resize_lock(pgdat, &flags);
+		for (i = 0, pfn = pgdat->node_start_pfn;
+			i < pgdat->node_spanned_pages;
+			i++, pfn = pgdat->node_start_pfn + i) {
+
+			if (!pfn_valid(pfn) || !page_is_ram(pfn))
+				continue;
+
+			page = pfn_to_page(pfn);
+			paddr = page_to_phys(page);
+
+			/* If pg will be MACed by tboot, no need to MAC here */
+			for (j = 0; j < tboot->num_mac_regions; j++) {
+				rstart = tboot->mac_regions[j].start;
+				rend = rstart +	tboot->mac_regions[j].size;
+				if (((paddr + PAGE_SIZE) <= rstart)
+					|| (rend <= paddr))
+					continue;
+				break;
+			}
+
+			if (j == tboot->num_mac_regions) {
+				sg_set_page(sg, page, PAGE_SIZE, 0);
+#ifdef CONFIG_DEBUG_PAGEALLOC
+			/*
+			 * check if the page we are going to MAC is marked as
+			 * present in the kernel page tables.
+			 */
+			if (!kernel_page_present(page)) {
+				kernel_map_pages(page, 1, 1);
+				ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
+				kernel_map_pages(page, 1, 0);
+			} else
+#endif
+				ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
+				if (ret) {
+					pgdat_resize_unlock(pgdat, &flags);
+					return ret;
+				}
+			}
+		}
+		pgdat_resize_unlock(pgdat, &flags);
+	}
+
+#ifdef CONFIG_X86_64
+	/*
+	 * for stack > 4G, we should MAC the stack in the kernel after switch,
+	 * for stack < 4G, the stack is MACed by tboot
+	 */
+	if (new_stack) {
+		for (i = 0, page = virt_to_page((void *)current_thread_info());
+			i < (1 << THREAD_ORDER);
+			i++, page++) {
+			sg_set_page(sg, page, PAGE_SIZE, 0);
+			ret = crypto_hash_update(&desc, sg, PAGE_SIZE);
+			if (ret)
+				return ret;
+		}
+	}
+#endif
+
+	ret = crypto_hash_final(&desc, (uint8_t *)mac);
+	if (ret)
+		return ret;
+
+	/* Clean the key */
+	memset(zeroed_key, 0, sizeof(zeroed_key));
+	crypto_hash_setkey(desc.tfm, zeroed_key, VMAC_KEY_LEN);
+
+	return 0;
+}
+
 #ifdef CONFIG_ACPI_SLEEP
 
 static void add_mac_region(phys_addr_t start, unsigned long size)
@@ -196,6 +355,27 @@ static int tboot_setup_sleep(void)
 
 	/* kernel code + data + bss */
 	add_mac_region(virt_to_phys(_text), _end - _text);
+
+	/* stack */
+#ifdef CONFIG_X86_64
+	/*
+	 * if stack > 4G, we should MAC the stack in the kernel after switch,
+	 * if stack < 4G, the stack is MACed by tboot
+	 */
+	if (new_stack)
+		add_mac_region(virt_to_phys(new_stack),
+				IRQ_STACK_SIZE); /* > 4G */
+	else
+#endif
+		add_mac_region(virt_to_phys(current_thread_info()),
+				THREAD_SIZE); /* < 4G */
+
+	/* MAC userspace memory not handled by tboot */
+	get_random_bytes(tboot->s3_key, sizeof(tboot->s3_key));
+	if (tboot_gen_mem_integrity(tboot->s3_key, &mem_mac)) {
+		panic("tboot: vmac generation failed\n");
+		return -1;
+	}
 
 	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
 
@@ -292,6 +472,52 @@ void tboot_sleep(u8 sleep_state, u32 pm1
 	}
 
 	tboot_shutdown(acpi_shutdown_map[sleep_state]);
+}
+
+static void tboot_sx_resume(void)
+{
+	vmac_t mac;
+
+	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
+		panic("tboot: vmac generation failed\n");
+	else if (mac != mem_mac)
+#ifdef CONFIG_DEBUG_KERNEL
+		pr_debug("tboot: memory integrity %llx -> %llx\n",
+				mem_mac, mac);
+#else
+		panic("tboot: memory integrity was lost on resume\n");
+#endif
+	else
+		pr_info("memory integrity OK\n");
+
+	/* Clean s3_key */
+	memset(tboot->s3_key, 0, sizeof(tboot->s3_key));
+}
+
+extern void do_suspend_lowlevel(void);
+
+static void tboot_do_suspend_lowlevel_call(void)
+{
+	do_suspend_lowlevel();
+	tboot_sx_resume();
+}
+
+void tboot_do_suspend_lowlevel(void)
+{
+	int ret = -1;
+
+	if (!tboot_enabled()) {
+		do_suspend_lowlevel();
+		return;
+	}
+
+	ret = tboot_pre_stack_switch();
+	if (!ret) {
+		tboot_switch_stack_call(tboot_do_suspend_lowlevel_call,
+					(u64)new_stack_ptr);
+		tboot_post_stack_switch();
+	} else
+		tboot_do_suspend_lowlevel_call();
 }
 
 static atomic_t ap_wfs_count;
diff -r c878d454dc8b drivers/acpi/sleep.c
--- a/drivers/acpi/sleep.c	Wed Dec 02 01:06:32 2009 -0800
+++ b/drivers/acpi/sleep.c	Thu Dec 03 07:22:17 2009 -0800
@@ -16,6 +16,7 @@
 #include <linux/device.h>
 #include <linux/suspend.h>
 #include <linux/reboot.h>
+#include <linux/tboot.h>
 
 #include <asm/io.h>
 
@@ -244,7 +245,7 @@ static int acpi_suspend_enter(suspend_st
 		break;
 
 	case ACPI_STATE_S3:
-		do_suspend_lowlevel();
+		tboot_do_suspend_lowlevel();
 		break;
 	}
 
diff -r c878d454dc8b include/linux/mm.h
--- a/include/linux/mm.h	Wed Dec 02 01:06:32 2009 -0800
+++ b/include/linux/mm.h	Thu Dec 03 07:22:17 2009 -0800
@@ -1263,18 +1263,18 @@ static inline void enable_debug_pageallo
 {
 	debug_pagealloc_enabled = 1;
 }
-#ifdef CONFIG_HIBERNATION
+#if defined(CONFIG_HIBERNATION) || defined(CONFIG_INTEL_TXT)
 extern bool kernel_page_present(struct page *page);
-#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_HIBERNATION || CONFIG_INTEL_TXT */
 #else
 static inline void
 kernel_map_pages(struct page *page, int numpages, int enable) {}
 static inline void enable_debug_pagealloc(void)
 {
 }
-#ifdef CONFIG_HIBERNATION
+#if defined(CONFIG_HIBERNATION) || defined(CONFIG_INTEL_TXT)
 static inline bool kernel_page_present(struct page *page) { return true; }
-#endif /* CONFIG_HIBERNATION */
+#endif /* CONFIG_HIBERNATION || CONFIG_INTEL_TXT */
 #endif
 
 extern struct vm_area_struct *get_gate_vma(struct task_struct *tsk);
diff -r c878d454dc8b include/linux/tboot.h
--- a/include/linux/tboot.h	Wed Dec 02 01:06:32 2009 -0800
+++ b/include/linux/tboot.h	Thu Dec 03 07:22:17 2009 -0800
@@ -147,7 +147,7 @@ extern struct acpi_table_header *tboot_g
 extern struct acpi_table_header *tboot_get_dmar_table(
 				      struct acpi_table_header *dmar_tbl);
 extern int tboot_force_iommu(void);
-
+extern void tboot_do_suspend_lowlevel(void);
 #else
 
 #define tboot_probe()			do { } while (0)
@@ -156,6 +156,7 @@ extern int tboot_force_iommu(void);
 					do { } while (0)
 #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
 #define tboot_force_iommu()		0
+#define tboot_do_suspend_lowlevel()	do_suspend_lowlevel()
 
 #endif /* !CONFIG_INTEL_TXT */
 
diff -r c878d454dc8b security/Kconfig
--- a/security/Kconfig	Wed Dec 02 01:06:32 2009 -0800
+++ b/security/Kconfig	Thu Dec 03 07:22:17 2009 -0800
@@ -116,6 +116,8 @@ config INTEL_TXT
 config INTEL_TXT
 	bool "Enable Intel(R) Trusted Execution Technology (Intel(R) TXT)"
 	depends on HAVE_INTEL_TXT
+	select CRYPTO_VMAC
+	select CRYPTO_AES
 	help
 	  This option enables support for booting the kernel with the
 	  Trusted Boot (tboot) module. This will utilize

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Andi Kleen]

On Fri, Dec 04, 2009 at 05:12:11PM +0800, Shane Wang wrote:
> diff -r c878d454dc8b arch/x86/kernel/entry_64.S
> --- a/arch/x86/kernel/entry_64.S	Wed Dec 02 01:06:32 2009 -0800
> +++ b/arch/x86/kernel/entry_64.S	Thu Dec 03 07:22:17 2009 -0800
> @@ -1275,6 +1275,26 @@ ENTRY(call_softirq)
>  	CFI_ENDPROC
>  END(call_softirq)
> 
> +#ifdef CONFIG_INTEL_TXT
> +/* void tboot_switch_stack_call(void (*target_func)(void), u64 new_rsp) */
> +ENTRY(tboot_switch_stack_call)

I would drop the tboot_ prefix, stack switching can be useful for other
things too.

> +	CFI_STARTPROC
> +	push %rbp
> +	CFI_ADJUST_CFA_OFFSET	8
> +	CFI_REL_OFFSET	rbp,0
> +	mov %rsp, %rbp
> +	CFI_DEF_CFA_REGISTER	rbp

Did you verify that the dwarf2 really works and gdb can backtrace through
it?

> +{
> +	BUG_ON((new_stack != NULL) || (new_stack_ptr != NULL));
> +
> +	/*
> +	 * as long as thread info is above 4G, then switch stack,
> +	 * since tboot can't access >4G stack for MACing
> +	 */
> +	if (!((PFN_PHYS(PFN_DOWN(virt_to_phys(current_thread_info())))
> +		+ (PFN_UP(THREAD_SIZE) << PAGE_SHIFT))
> +		& 0xffffffff00000000UL))
> +		return -1;

All the PFN_*s seem somewhat redundant, it would be easier to keep
everything shifted up in the first place.

> +
> +	new_stack = (char *)__get_free_pages(GFP_DMA32, IRQ_STACK_ORDER);
> +
> +	BUG_ON(new_stack == NULL);

GFP_REPEAT at least? BUG_ON is a nasty way to handle out of memory

> +	memset(new_stack, 0, IRQ_STACK_SIZE);

GFP_ZERO
> +	new_stack_ptr = new_stack + IRQ_STACK_SIZE - 64;

Why - 64?

> +
> +	return 0;
> +}
> +
> +static void tboot_post_stack_switch(void)
> +{
> +	BUG_ON((new_stack == NULL) || (new_stack_ptr == NULL));
> +
> +	free_pages((unsigned long)new_stack, IRQ_STACK_ORDER);
> +	new_stack = NULL;
> +	new_stack_ptr = NULL;
> +}
> +
> +extern void tboot_switch_stack_call(void (*target_func)(void), u64 
> new_rsp);

Typically those should be in some header.

> +	struct page *page;
> +	uint64_t paddr, rstart, rend;
> +	unsigned long pfn;
> +	uint8_t zeroed_key[VMAC_KEY_LEN];
> +
> +	if (!tfm)
> +		tfm = crypto_alloc_hash("vmac(aes)", 0, CRYPTO_ALG_ASYNC);
> +
> +	if (IS_ERR(tfm)) {
> +		tfm = NULL;
> +		return -ENOMEM;
> +	}
> +
> +	desc.tfm = tfm;
> +	desc.flags = 0;
> +
> +	sg_init_table(sg, 1);
> +
> +	ret = crypto_hash_init(&desc);
> +	if (ret)
> +		return ret;
> +	ret = crypto_hash_setkey(desc.tfm, key, VMAC_KEY_LEN);
> +	if (ret)
> +		return ret;
> +
> +	for_each_online_pgdat(pgdat) {

No locking against memory hotplug? Even if user space is still down
doing that would be safer

> +		unsigned long flags;
> +
> +		pgdat_resize_lock(pgdat, &flags);
> +		for (i = 0, pfn = pgdat->node_start_pfn;
> +			i < pgdat->node_spanned_pages;
> +			i++, pfn = pgdat->node_start_pfn + i) {
> +
> +			if (!pfn_valid(pfn) || !page_is_ram(pfn))
> +				continue;

You probably should consider a faster way to skip holes, doing
them piece by piece might well be a performance problem on very
holey systems. Especially page_is_ram() is quite slow.

> +					|| (rend <= paddr))
> +					continue;
> +				break;
> +			}
> +
> +			if (j == tboot->num_mac_regions) {
> +				sg_set_page(sg, page, PAGE_SIZE, 0);
> +#ifdef CONFIG_DEBUG_PAGEALLOC
> +			/*
> +			 * check if the page we are going to MAC is marked as
> +			 * present in the kernel page tables.
> +			 */
> +			if (!kernel_page_present(page)) {
> +				kernel_map_pages(page, 1, 1);
> +				ret = crypto_hash_update(&desc, sg, 
> PAGE_SIZE);
> +				kernel_map_pages(page, 1, 0);

Nasty, might be racy -- better use a separate mapping in this case.

> +#ifdef CONFIG_X86_64
> +	/*
> +	 * for stack > 4G, we should MAC the stack in the kernel after 
> switch,
> +	 * for stack < 4G, the stack is MACed by tboot
> +	 */

This special case seems quite ugly, with all its ifdefs and lots
of special code. 
Can't you just MAC the stack always? Shouldn't be that expensive. 

> +	else
> +#endif
> +		add_mac_region(virt_to_phys(current_thread_info()),
> +				THREAD_SIZE); /* < 4G */
> +
> +	/* MAC userspace memory not handled by tboot */
> +	get_random_bytes(tboot->s3_key, sizeof(tboot->s3_key));

That's early in boot isn't it? It's quite doubtful you'll get
anything even vaguely random out of get_random_bytes at this point, may be not
even the time.

>  	}
> 
>  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
> +}
> +
> +static void tboot_sx_resume(void)
> +{
> +	vmac_t mac;
> +
> +	if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
> +		panic("tboot: vmac generation failed\n");
> +	else if (mac != mem_mac)
> +#ifdef CONFIG_DEBUG_KERNEL
> +		pr_debug("tboot: memory integrity %llx -> %llx\n",
> +				mem_mac, mac);
> +#else
> +		panic("tboot: memory integrity was lost on resume\n");
> +#endif

I don't think DEBUG_KERNEL is supposed to be used like this, better
probably a separate option if it makes sense.

Typical problem with panics at this point: is anything even visible
on the screen?

>  
> +#ifdef CONFIG_INTEL_TXT
> +/* void tboot_switch_stack_call(void (*target_func)(void), u64 new_rsp) */
> +ENTRY(tboot_switch_stack_call)

Hmm, I thought i had seen that earlier already? Is it a copy?

BTW there's no reason all of this has to be in entry*.S, that
file is already quite crowded.

The patch is duplicated here?

-Andi

RE: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Cihula, Joseph]

> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Friday, December 04, 2009 12:20 AM
>
> Hi!
>
> Please wrap mails at column 72 (or so).
>
> > > AFAICT, it verifies userspace _and_ kernel memory, that's why it does
> > > magic stack switching. Why not verify everything in tboot?
> > Because tboot only can access <4G mem without paging. And the memory is sparse. We
> can't/needn't set unlimited sparse mem ranges to the MAC array with limited elements in the
> shared page, in order to pass the parameters.
> > On the other hand, it is reasonable for tboot to verify kernel, and kernel to verify
> userspace memory.
> >
>
> Are  you sure x86-64 kernel & modules is always below 4GB? I don't
> think so.

The only part of the kernel that really needs to be below 4GB is what is used by the code on resume to do the MAC verification of the remainder of the memory.  This is all static code and variables.  The regions we pass to tboot for MAC'ing are:
S3 real mode resume code:  acpi_wakeup_address, WAKEUP_SIZE
AP trampoline code:  virt_to_phys(trampoline_base), TRAMPOLINE_SIZE
kernel static code and data:  virt_to_phys(_text), _end - _text
the stack we use (either new one or existing depending--see code):
        virt_to_phys(new_stack), IRQ_STACK_SIZE
        virt_to_phys(current_thread_info()), THREAD_SIZE


Can you post a .config and platform configuration for which the tboot, MAC, or do_suspend_lowlevel() are not within the regions above?  We'll be happy to address the issue once we understand the conditions.

> > >> +static vmac_t mem_mac;
> > >> +static struct crypto_hash *tfm;
> > >
> > > Could these be automatic?
> > Maybe, but I don't wish other files can access the variables and take tfm as an example, I'd
> like to allocate memory to it once and then initialize it once in order to avoid impact of
> memory change to MACing.
> >
>
> You use stack, anyway.

Do you mean as static local variables?  If so, I'm not sure if that would be any better.

> > > Why does 4G limit matter on 64-bit?
> > tboot can't access >4G, see above.
>
> Too bad, then its broken by design.

See above.

> > >> +        if (tboot_gen_mem_integrity(tboot->s3_key, &mac))
> > >> +                panic("tboot: vmac generation failed\n");
> > >> +        else if (mac != mem_mac)
> > >> +                panic("tboot: memory integrity was lost on resume\n"); +        else
> > >> +                pr_info("memory integrity OK\n");
> > >
> > > So I corrupt memory, but also corrupt tboot_enabled() to return 0....
> > >
> > > And... does panic kill the machine quickly enough that no 'bad stuff'
> > > happens? (Whats bad stuff in this context, anyway?).
>
> I'd really like you to answer that.

"bad stuff" would be the execution of any code (or use of any data that affects execution) that was not verified by tboot.  As long as panic() is within the code ranges MAC'ed by tboot (see above), it would be covered.  Do you know of some panic() code paths that are outside of this?

> > >> @@ -244,7 +245,10 @@ static int acpi_suspend_enter(suspend_st
> > >> break;
> > >>
> > >>          case ACPI_STATE_S3:
> > >> +                tboot_switch_stack();
> > >>                  do_suspend_lowlevel();
> > >> +                tboot_sx_resume();
> > >> +                tboot_restore_stack();
> > >>                  break;
> > >>          }
> > >>
> > >
> > > Did you audit all code before sx_resume()? If it trusts  data not
> > > checksummed by tboot, attacker may be able to hijack code execution
> > > and bypass your protection, no?
> > Yes, kernel code is audited by tboot before resume.
>
> So no, you did not audit do_suspend_lowlevel to make sure it does not
> follow function pointers. Bad.

We aren't aware of any code or data used by the resume path that is outside of the tboot-MAC'ed regions above--if you can point out any then we will gladly address them.

>                                                                       Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

RE: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Cihula, Joseph]

> From: Pavel Machek [mailto:pavel@ucw.cz]
> Sent: Friday, December 04, 2009 12:29 AM
>
> > This patch added verification for userspace memory integrity after
> >  S3 resume.
>
> It does not work.
>
> > Integrity verification for other memory (say kernel itself) has been done by tboot.
> >
>
> Not true. Kernel  uses memory above 4G on x86-64. Including... say
> console writing functions.

I've responded to that in your previous email thread, so let's keep the technical discussion of what is MAC'ed to just that one thread.

> You can patch holes, but without description 'what does this protect
> against' it is almost impossible to evaluate.
>
>
> > +void tboot_do_suspend_lowlevel(void)
> > +{
> > +   int ret = -1;
> > +
> > +   if (!tboot_enabled()) {
> > +           do_suspend_lowlevel();
> > +           return;
> > +   }
> > +
> > +   ret = tboot_pre_stack_switch();
> > +   if (!ret) {
> > +           tboot_switch_stack_call(tboot_do_suspend_lowlevel_call,
> > +                                   (u64)new_stack_ptr);
>
> ...and here you add requirements to suspend_lowlevel that were not
> there before. ("May not act on unchecksummed memory"), without
> documenting them.

Really the only requirement is (as discussed in the previous thread) that it only use code and data within [_text, _end - _text]--do you think that this really needs to be called out?  do_suspend_lowlevel() is a very simple function that has just the one (resume path) call to another simple function--it doesn't seem likely that it would violate this.

>
> NAK.
>                                                                       Pavel
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Andi Kleen]

> "bad stuff" would be the execution of any code (or use of any data that affects execution) that was not verified by tboot.  As long as panic() is within the code ranges MAC'ed by tboot (see above), it would be covered.  Do you know of some panic() code paths that are outside of this?

Not code path, but the code called by panic (console drivers, debuggers etc.)
can well use data that is stored >4GB

This can include structures with indirect pointers, like notifier chains.

Notifier chains have a special checker than can check
for <4GB, but there are other call vectors too.

> > > > checksummed by tboot, attacker may be able to hijack code execution
> > > > and bypass your protection, no?
> > > Yes, kernel code is audited by tboot before resume.
> >
> > So no, you did not audit do_suspend_lowlevel to make sure it does not
> > follow function pointers. Bad.
> 
> We aren't aware of any code or data used by the resume path that is outside of the tboot-MAC'ed regions above--if you can point out any then we will gladly address them.

Code coverage is not enough, you need data coverage too.  If someone 
modifies kernel data it's typically easy to subvert code as a next step.


-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

RE: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Cihula, Joseph]

> From: Andi Kleen [mailto:andi@firstfloor.org]
> Sent: Friday, December 04, 2009 9:14 AM
>
> > "bad stuff" would be the execution of any code (or use of any data that affects execution)
> that was not verified by tboot.  As long as panic() is within the code ranges MAC'ed by tboot
> (see above), it would be covered.  Do you know of some panic() code paths that are outside of
> this?
>
> Not code path, but the code called by panic (console drivers, debuggers etc.)
> can well use data that is stored >4GB
>
> This can include structures with indirect pointers, like notifier chains.
>
> Notifier chains have a special checker than can check
> for <4GB, but there are other call vectors too.

Since, as you pointed out in a previous email, it is doubtful that there will be any user-visible output at this point, we can change this path to a tboot reset (which will give us some serial output at least).  Is it going to be similarly unsafe to do a printk()?

> > > > > checksummed by tboot, attacker may be able to hijack code execution
> > > > > and bypass your protection, no?
> > > > Yes, kernel code is audited by tboot before resume.
> > >
> > > So no, you did not audit do_suspend_lowlevel to make sure it does not
> > > follow function pointers. Bad.
> >
> > We aren't aware of any code or data used by the resume path that is outside of the tboot-
> MAC'ed regions above--if you can point out any then we will gladly address them.
>
> Code coverage is not enough, you need data coverage too.  If someone
> modifies kernel data it's typically easy to subvert code as a next step.

Agreed, which is why I said "code or data".  We'll take another look at the couple of fns that are within this path, but if you have any specific examples can you please post them.

>
>
> -Andi
> --
> ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[H. Peter Anvin]

On 12/04/2009 09:13 AM, Andi Kleen wrote:
>>>
>>> So no, you did not audit do_suspend_lowlevel to make sure it does not
>>> follow function pointers. Bad.
>>
>> We aren't aware of any code or data used by the resume path that is outside of the tboot-MAC'ed regions above--if you can point out any then we will gladly address them.
> 
> Code coverage is not enough, you need data coverage too.  If someone 
> modifies kernel data it's typically easy to subvert code as a next step.
> 

The only function pointers that are invoked on the do_suspend_lowlevel
path are some paravirt_crap pointers, but those are located inside
kernel static data.

This is not to say that this isn't a new constraint, and should be
documented, and checked ahead of time...

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Andi Kleen]

On Fri, Dec 04, 2009 at 09:41:24AM -0800, Cihula, Joseph wrote:
> > From: Andi Kleen [mailto:andi@firstfloor.org]
> > Sent: Friday, December 04, 2009 9:14 AM
> >
> > > "bad stuff" would be the execution of any code (or use of any data that affects execution)
> > that was not verified by tboot.  As long as panic() is within the code ranges MAC'ed by tboot
> > (see above), it would be covered.  Do you know of some panic() code paths that are outside of
> > this?
> >
> > Not code path, but the code called by panic (console drivers, debuggers etc.)
> > can well use data that is stored >4GB
> >
> > This can include structures with indirect pointers, like notifier chains.
> >
> > Notifier chains have a special checker than can check
> > for <4GB, but there are other call vectors too.
> 
> Since, as you pointed out in a previous email, it is doubtful that there will be any user-visible output at this point, we can change this path to a tboot reset (which will give us some serial output at least).  Is it going to be similarly unsafe to do a printk()?

Yes printk is similarly unsafe. It calls all the console machinery,
which has a lot of data.

Perhaps early_printk(), that is relatively self contained, but doesn't
always work.

Of course you would need to have a timeout before reset, and at this point the
delay loops are not calibrated yet, so you don't know how to wait.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Andi Kleen]

On Fri, Dec 04, 2009 at 09:53:37AM -0800, H. Peter Anvin wrote:
> On 12/04/2009 09:13 AM, Andi Kleen wrote:
> >>>
> >>> So no, you did not audit do_suspend_lowlevel to make sure it does not
> >>> follow function pointers. Bad.
> >>
> >> We aren't aware of any code or data used by the resume path that is outside of the tboot-MAC'ed regions above--if you can point out any then we will gladly address them.
> > 
> > Code coverage is not enough, you need data coverage too.  If someone 
> > modifies kernel data it's typically easy to subvert code as a next step.
> > 
> 
> The only function pointers that are invoked on the do_suspend_lowlevel
> path are some paravirt_crap pointers, but those are located inside
> kernel static data.

Was referring to panic(), like Pavel said.

It would be relatively easy to subvert something called by panic
that just jumps back to after the MAC checks.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

RE: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Cihula, Joseph]

> From: Andi Kleen [mailto:andi@firstfloor.org]
> Sent: Friday, December 04, 2009 12:10 PM
>
> On Fri, Dec 04, 2009 at 09:41:24AM -0800, Cihula, Joseph wrote:
> > > From: Andi Kleen [mailto:andi@firstfloor.org]
> > > Sent: Friday, December 04, 2009 9:14 AM
> > >
> > > > "bad stuff" would be the execution of any code (or use of any data that affects
> execution)
> > > that was not verified by tboot.  As long as panic() is within the code ranges MAC'ed by
> tboot
> > > (see above), it would be covered.  Do you know of some panic() code paths that are outside
> of
> > > this?
> > >
> > > Not code path, but the code called by panic (console drivers, debuggers etc.)
> > > can well use data that is stored >4GB
> > >
> > > This can include structures with indirect pointers, like notifier chains.
> > >
> > > Notifier chains have a special checker than can check
> > > for <4GB, but there are other call vectors too.
> >
> > Since, as you pointed out in a previous email, it is doubtful that there will be any user-
> visible output at this point, we can change this path to a tboot reset (which will give us
> some serial output at least).  Is it going to be similarly unsafe to do a printk()?
>
> Yes printk is similarly unsafe. It calls all the console machinery,
> which has a lot of data.
>
> Perhaps early_printk(), that is relatively self contained, but doesn't
> always work.
>
> Of course you would need to have a timeout before reset, and at this point the
> delay loops are not calibrated yet, so you don't know how to wait.

I would expect that early_printk() coupled with tboot's serial output would be sufficient for a case such as this.  If we've done our work correctly, loss of integrity should only occur when the system is attacked across the S3 transition--which should be fairly rare and which should place a premium on prevention of the attacked code from executing.  Esp. on servers, there may not be anyone to see console output anyway.  Does early_printk() and a tboot reset seem like a reasonable approach?

>
> -Andi
>
> --
> ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Andi Kleen]

> > Of course you would need to have a timeout before reset, and at this point the
> > delay loops are not calibrated yet, so you don't know how to wait.

That was actually wrong, since you're coming back from S3
udelay should work. nm.

> 
> I would expect that early_printk() coupled with tboot's serial output would be sufficient for a case such as this.  If we've done our work correctly, loss of integrity should only occur when the system is attacked across the S3 transition--which should be fairly rare and which should place a premium on prevention of the attacked code from executing.  Esp. on servers, there may not be anyone to see console output anyway.  Does early_printk() and a tboot reset seem like a reasonable approach?

At least classical vga/serial early_printk should be safe, I'm not sure
about the early USB code recently added though, some auditing on that
first would be good.

early_printk defaults to VGA text output, so if you do a reset you would
need a delay first, otherwise noone can see it ever. But one could be
done with udelay()

It'll be also invisible with frame buffer active, which is the common
case for distributions.  So basically in most cases the message would be
invisible.

(not that panic is much better by default in this regard though,
at least not without Jesse's recent frame buffer work ...)

-Andi


-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[H. Peter Anvin]

On 12/04/2009 12:17 PM, Cihula, Joseph wrote:
> 
> I would expect that early_printk() coupled with tboot's serial output would be sufficient for a case such as this.  If we've done our work correctly, loss of integrity should only occur when the system is attacked across the S3 transition--which should be fairly rare and which should place a premium on prevention of the attacked code from executing.  Esp. on servers, there may not be anyone to see console output anyway.  Does early_printk() and a tboot reset seem like a reasonable approach?
> 

The zeroeth-order thing is you should reliably crash and reboot.  This
is pretty normal for most S3 resume failures anyway, so although it is
somewhat unfortunate for debugging, it is isn't exactly an unreasonable
thing to do.

Getting a message out is a nice plus, but not a requirement.
Guaranteeing the integrity is an absolute requirement.

	-hpa

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Pavel Machek]

On Fri 2009-12-04 08:46:04, Cihula, Joseph wrote:
> > From: Pavel Machek [mailto:pavel@ucw.cz]
> > Sent: Friday, December 04, 2009 12:20 AM
> >
> > Hi!
> >
> > Please wrap mails at column 72 (or so).
> >
> > > > AFAICT, it verifies userspace _and_ kernel memory, that's why it does
> > > > magic stack switching. Why not verify everything in tboot?
> > > Because tboot only can access <4G mem without paging. And the memory is sparse. We
> > can't/needn't set unlimited sparse mem ranges to the MAC array with limited elements in the
> > shared page, in order to pass the parameters.
> > > On the other hand, it is reasonable for tboot to verify kernel, and kernel to verify
> > userspace memory.
> > >
> >
> > Are  you sure x86-64 kernel & modules is always below 4GB? I don't
> > think so.
> 
> The only part of the kernel that really needs to be below 4GB is what is used by the code on resume to do the MAC verification of the remainder of the memory.  This is all static code and variables.  The regions we pass to tboot for MAC'ing are:
> S3 real mode resume code:  acpi_wakeup_address, WAKEUP_SIZE
> AP trampoline code:  virt_to_phys(trampoline_base), TRAMPOLINE_SIZE
> kernel static code and data:  virt_to_phys(_text), _end - _text
> the stack we use (either new one or existing depending--see code):
>         virt_to_phys(new_stack), IRQ_STACK_SIZE
>         virt_to_phys(current_thread_info()), THREAD_SIZE
> 

Yes, so... what guarantees these actually are under 4GB? Perhaps you
should put them to spearate section and use linker magic?

You certainly should annotate them somehow so people modifying the
code know about this new requirement. 

> Can you post a .config and platform configuration for which the tboot, MAC, or do_suspend_lowlevel() are not within the regions above?  We'll be happy to address the issue once we understand the conditions.
> 

Can you prove that for all .config s on all machines they are under
4GB? On all future kernels, too? I thought so.

> > > >> +static vmac_t mem_mac;
> > > >> +static struct crypto_hash *tfm;
> > > >
> > > > Could these be automatic?
> > > Maybe, but I don't wish other files can access the variables and take tfm as an example, I'd
> > like to allocate memory to it once and then initialize it once in order to avoid impact of
> > memory change to MACing.
> > >
> >
> > You use stack, anyway.
> 
> Do you mean as static local variables?  If so, I'm not sure if that would be any better.
> 

Those two variables (mem_mac and tfm) are static. I do not think they
need to be, and code would be much nicer if they were not.

> > > > So I corrupt memory, but also corrupt tboot_enabled() to return 0....
> > > >
> > > > And... does panic kill the machine quickly enough that no 'bad stuff'
> > > > happens? (Whats bad stuff in this context, anyway?).
> >
> > I'd really like you to answer that.
> 
> "bad stuff" would be the execution of any code (or use of any data that affects execution) that was not verified by tboot.  As long as panic() is within the code ranges MAC'ed by tboot (see above), it would be covered.  Do you know of some panic() code paths that are outside of this?
> 

(Please wrap at column 72).

Yes, I know that panic() has problems.

> > > > Did you audit all code before sx_resume()? If it trusts  data not
> > > > checksummed by tboot, attacker may be able to hijack code execution
> > > > and bypass your protection, no?
> > > Yes, kernel code is audited by tboot before resume.
> >
> > So no, you did not audit do_suspend_lowlevel to make sure it does not
> > follow function pointers. Bad.
> 
> We aren't aware of any code or data used by the resume path that is outside of the tboot-MAC'ed regions above--if you can point out any then we will gladly address them.
>

I'm asking you to do the code audit, and you tell me to do it
myself. I'm not interested in doing your work. Yes, they are some
obvious places (like panic), that you missed. There are probably more.

Design your code properly so that it does not depend on details like
that. That means checksumming in tboot.

[Or do the auditing work. But don't try playing "we fixed all bugs
you told us about, so it must be perfect" game with me.]
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Pavel Machek]

> > > +
> > > +   ret = tboot_pre_stack_switch();
> > > +   if (!ret) {
> > > +           tboot_switch_stack_call(tboot_do_suspend_lowlevel_call,
> > > +                                   (u64)new_stack_ptr);
> >
> > ...and here you add requirements to suspend_lowlevel that were not
> > there before. ("May not act on unchecksummed memory"), without
> > documenting them.
> 
> Really the only requirement is (as discussed in the previous thread) that it only use code and data within [_text, _end - _text]--do you think that this really needs to be called out?  do_suspend_lowlevel() is a very simple function that has just the one (resume path) call to another simple function--it doesn't seem likely that it would violate this.
> 

Resume code is already pretty tricky, and you add yet another layer of
trickery. Yes, it needs to be properly documented.

And... how does it interact with restore_processor_state? I don't
think do_suspend_lowlevel is as simple as you think...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[H. Peter Anvin]

On 12/04/2009 02:15 PM, Pavel Machek wrote:
>>>
>>> Are  you sure x86-64 kernel & modules is always below 4GB? I don't
>>> think so.

The x86-64 kernel is run where it is loaded by the boot loader.  For
most boot loaders, that will mean < 4 GB.  This is not the case for
modules, and they cannot and should not rely on modules inside
restricted zone.

This effectively becomes a constraint on whatever boot loader is used to
load the kernel for it to be compatible with tboot.

	-hpa

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Pavel Machek]

On Fri 2009-12-04 09:53:37, H. Peter Anvin wrote:
> On 12/04/2009 09:13 AM, Andi Kleen wrote:
> >>>
> >>> So no, you did not audit do_suspend_lowlevel to make sure it does not
> >>> follow function pointers. Bad.
> >>
> >> We aren't aware of any code or data used by the resume path that is outside of the tboot-MAC'ed regions above--if you can point out any then we will gladly address them.
> > 
> > Code coverage is not enough, you need data coverage too.  If someone 
> > modifies kernel data it's typically easy to subvert code as a next step.
> > 
> 
> The only function pointers that are invoked on the do_suspend_lowlevel
> path are some paravirt_crap pointers, but those are located inside
> kernel static data.

What guarantees kernel static data are below 4GB? What prevents me
from booting with funny memmap where first 1MB is mapped, and then
memory above 4GB? What prevents Chinese company to ship machine with
such funny memmap?
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[Pavel Machek]

On Fri 2009-12-04 14:24:24, H. Peter Anvin wrote:
> On 12/04/2009 02:15 PM, Pavel Machek wrote:
> >>>
> >>> Are  you sure x86-64 kernel & modules is always below 4GB? I don't
> >>> think so.
> 
> The x86-64 kernel is run where it is loaded by the boot loader.  For
> most boot loaders, that will mean < 4 GB.  This is not the case for
> modules, and they cannot and should not rely on modules inside
> restricted zone.
> 
> This effectively becomes a constraint on whatever boot loader is used to
> load the kernel for it to be compatible with tboot.

Having "security" technology that silently fails with funny bootloader
is pretty bad, I'd say.

Instead of doing this properly (in tboot), Joseph hopes to save some
work by basically splitting kernel into two parts, "trusted" and
"untrusted". But doing that properly would be too much work, so he
just handwaves and hopes for the best.

Unfortunately that

a) does not work (panic, printks)

b) places funny constraints all over the code (documenting them would
be too much work, so we get more handwaving)

c) reduces future flexibility (trusted code can not be over 4GB, it is
silent security hole when it is) 

I'd prefer to see this done properly; tboot should simply verify all
the memory for us. It is separate piece of code, trusted, and can
probably fit itself under 4GB.

If that seems like too much work, then please go all over the code,
and mark all the code that is "trusted" (has to be under 4GB) and
properly document it, so that future modifications will not break that
assumption. Putting all that code into one section should be enough.

(Going into infinite loop is probably enough when memory corruption is
detected; there's no chance you can put printk/panic/neccessary
drivers all into the "trusted" section.)

Aha, and look. Your tboot_gen_mem_integrity is ran on resume, so it is
"trusted". Unfortunately, it calls crypto_alloc_hash, so you need to
audit that, and probably kmalloc it boils down into. I bet kmalloc
touches memory >4GB. And I bet crypto modules can be
... well... modules. That means over 4GB.

This is broken by design, right?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] intel_txt: add s3 userspace memory integrity verification

[H. Peter Anvin]

On 12/04/2009 02:39 PM, Pavel Machek wrote:
> 
> Having "security" technology that silently fails with funny bootloader
> is pretty bad, I'd say.
> 

Yes, but this wouldn't be a silent failure -- such a boot loader
wouldn't be able to boot tboot itself either, nor would be able to boot
32-bit kernels (which, in fact, not all boot loaders can); the tboot
boot process in fact in many ways treats tboot itself as a 32-bit
primary kernel, with the Linux kernel as a secondary kernel.

So, this particular failure would not be silent by any means.

	-hpa

[PATCH v2] intel_txt: add support for S3 memory integrity protection within Intel(R) TXT launched kernel

[Wang, Shane]

[-- Attachment #1: Type: text/plain, Size: 4608 bytes --]

v2: Based on a complexity analysis and tradeoff, we moved all MAC'ing into tboot.

This patch adds support for S3 memory integrity protection within an Intel(R) TXT launched kernel, for all kernel and userspace memory.  All RAM used by the kernel and userspace, as indicated by memory ranges of type E820_RAM and E820_RESERVED_KERN in the e820 table, will be integrity protected.

The MAINTAINERS file is also updated to reflect the maintainers of the TXT-related code.

Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

 Documentation/intel_txt.txt |   14 +++++++-------
 MAINTAINERS                 |   11 +++++++++++
 arch/x86/include/asm/e820.h |    7 ++++++-
 arch/x86/kernel/tboot.c     |   17 ++++++++---------
 4 files changed, 32 insertions(+), 17 deletions(-)

diff -r d2911aa1461d Documentation/intel_txt.txt
--- a/Documentation/intel_txt.txt	Thu Mar 04 09:37:53 2010 -0500
+++ b/Documentation/intel_txt.txt	Tue Mar 09 10:40:17 2010 -0500
@@ -161,13 +161,13 @@ o  In order to put a system into any of 
       has been restored, it will restore the TPM PCRs and then
       transfer control back to the kernel's S3 resume vector.
       In order to preserve system integrity across S3, the kernel
-      provides tboot with a set of memory ranges (kernel
-      code/data/bss, S3 resume code, and AP trampoline) that tboot
-      will calculate a MAC (message authentication code) over and then
-      seal with the TPM.  On resume and once the measured environment
-      has been re-established, tboot will re-calculate the MAC and
-      verify it against the sealed value.  Tboot's policy determines
-      what happens if the verification fails.
+      provides tboot with a set of memory ranges (RAM and RESERVED_KERN
+      in the e820 table, but not any memory that BIOS might alter over
+      the S3 transition) that tboot will calculate a MAC (message
+      authentication code) over and then seal with the TPM. On resume
+      and once the measured environment has been re-established, tboot
+      will re-calculate the MAC and verify it against the sealed value.
+      Tboot's policy determines what happens if the verification fails.
 
 That's pretty much it for TXT support.
 
diff -r d2911aa1461d MAINTAINERS
--- a/MAINTAINERS	Thu Mar 04 09:37:53 2010 -0500
+++ b/MAINTAINERS	Tue Mar 09 10:40:17 2010 -0500
@@ -2891,6 +2891,17 @@ F:	Documentation/networking/README.ipw22
 F:	Documentation/networking/README.ipw2200
 F:	drivers/net/wireless/ipw2x00/ipw2200.*
 
+INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
+M:	Joseph Cihula <joseph.cihula@intel.com>
+M:	Shane Wang <shane.wang@intel.com>
+L:	tboot-devel@lists.sourceforge.net
+W:	http://tboot.sourceforge.net
+T:	Mercurial http://www.bughost.org/repos.hg/tboot.hg
+S:	Supported
+F:	Documentation/intel_txt.txt
+F:	include/linux/tboot.h
+F:	arch/x86/kernel/tboot.c
+
 INTEL WIRELESS WIMAX CONNECTION 2400
 M:	Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
 M:	linux-wimax@intel.com
diff -r d2911aa1461d arch/x86/include/asm/e820.h
--- a/arch/x86/include/asm/e820.h	Thu Mar 04 09:37:53 2010 -0500
+++ b/arch/x86/include/asm/e820.h	Tue Mar 09 10:40:17 2010 -0500
@@ -45,7 +45,12 @@
 #define E820_NVS	4
 #define E820_UNUSABLE	5
 
-/* reserved RAM used by kernel itself */
+/*
+ * reserved RAM used by kernel itself
+ * if CONFIG_INTEL_TXT is enabled, memory of this type will be
+ * included in the S3 integrity calculation and so should not include
+ * any memory that BIOS might alter over the S3 transition
+ */
 #define E820_RESERVED_KERN        128
 
 #ifndef __ASSEMBLY__
diff -r d2911aa1461d arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Thu Mar 04 09:37:53 2010 -0500
+++ b/arch/x86/kernel/tboot.c	Tue Mar 09 10:40:17 2010 -0500
@@ -139,18 +139,17 @@ static void add_mac_region(phys_addr_t s
 
 static void __init tboot_setup_sleep(void)
 {
+	int i;
+
 	tboot->num_mac_regions = 0;
 
-	/* S3 resume code */
-	add_mac_region(acpi_wakeup_address, WAKEUP_SIZE);
+	for (i = 0; i < e820.nr_map; i++) {
+		if ((e820.map[i].type != E820_RAM)
+		 && (e820.map[i].type != E820_RESERVED_KERN))
+			continue;
 
-#ifdef CONFIG_X86_TRAMPOLINE
-	/* AP trampoline code */
-	add_mac_region(virt_to_phys(trampoline_base), TRAMPOLINE_SIZE);
-#endif
-
-	/* kernel code + data + bss */
-	add_mac_region(virt_to_phys(_text), _end - _text);
+		add_mac_region(e820.map[i].addr, e820.map[i].size);
+	}
 
 	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
 }

[-- Attachment #2: s3_memory_integrity.patch --]
[-- Type: application/octet-stream, Size: 4253 bytes --]

v2: Based on a complexity analysis and tradeoff, we moved all MAC'ing into tboot.

This patch adds support for S3 memory integrity protection within an Intel(R) TXT launched kernel, for all kernel and userspace memory.  All RAM used by the kernel and userspace, as indicated by memory ranges of type E820_RAM and E820_RESERVED_KERN in the e820 table, will be integrity protected.

The MAINTAINERS file is also updated to reflect the maintainers of the TXT-related code.

Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

diff -r d2911aa1461d Documentation/intel_txt.txt
--- a/Documentation/intel_txt.txt	Thu Mar 04 09:37:53 2010 -0500
+++ b/Documentation/intel_txt.txt	Tue Mar 09 10:40:17 2010 -0500
@@ -161,13 +161,13 @@ o  In order to put a system into any of 
       has been restored, it will restore the TPM PCRs and then
       transfer control back to the kernel's S3 resume vector.
       In order to preserve system integrity across S3, the kernel
-      provides tboot with a set of memory ranges (kernel
-      code/data/bss, S3 resume code, and AP trampoline) that tboot
-      will calculate a MAC (message authentication code) over and then
-      seal with the TPM.  On resume and once the measured environment
-      has been re-established, tboot will re-calculate the MAC and
-      verify it against the sealed value.  Tboot's policy determines
-      what happens if the verification fails.
+      provides tboot with a set of memory ranges (RAM and RESERVED_KERN
+      in the e820 table, but not any memory that BIOS might alter over
+      the S3 transition) that tboot will calculate a MAC (message
+      authentication code) over and then seal with the TPM. On resume
+      and once the measured environment has been re-established, tboot
+      will re-calculate the MAC and verify it against the sealed value.
+      Tboot's policy determines what happens if the verification fails.
 
 That's pretty much it for TXT support.
 
diff -r d2911aa1461d MAINTAINERS
--- a/MAINTAINERS	Thu Mar 04 09:37:53 2010 -0500
+++ b/MAINTAINERS	Tue Mar 09 10:40:17 2010 -0500
@@ -2891,6 +2891,17 @@ F:	Documentation/networking/README.ipw22
 F:	Documentation/networking/README.ipw2200
 F:	drivers/net/wireless/ipw2x00/ipw2200.*
 
+INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
+M:	Joseph Cihula <joseph.cihula@intel.com>
+M:	Shane Wang <shane.wang@intel.com>
+L:	tboot-devel@lists.sourceforge.net
+W:	http://tboot.sourceforge.net
+T:	Mercurial http://www.bughost.org/repos.hg/tboot.hg
+S:	Supported
+F:	Documentation/intel_txt.txt
+F:	include/linux/tboot.h
+F:	arch/x86/kernel/tboot.c
+
 INTEL WIRELESS WIMAX CONNECTION 2400
 M:	Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
 M:	linux-wimax@intel.com
diff -r d2911aa1461d arch/x86/include/asm/e820.h
--- a/arch/x86/include/asm/e820.h	Thu Mar 04 09:37:53 2010 -0500
+++ b/arch/x86/include/asm/e820.h	Tue Mar 09 10:40:17 2010 -0500
@@ -45,7 +45,12 @@
 #define E820_NVS	4
 #define E820_UNUSABLE	5
 
-/* reserved RAM used by kernel itself */
+/*
+ * reserved RAM used by kernel itself
+ * if CONFIG_INTEL_TXT is enabled, memory of this type will be
+ * included in the S3 integrity calculation and so should not include
+ * any memory that BIOS might alter over the S3 transition
+ */
 #define E820_RESERVED_KERN        128
 
 #ifndef __ASSEMBLY__
diff -r d2911aa1461d arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Thu Mar 04 09:37:53 2010 -0500
+++ b/arch/x86/kernel/tboot.c	Tue Mar 09 10:40:17 2010 -0500
@@ -139,18 +139,17 @@ static void add_mac_region(phys_addr_t s
 
 static void __init tboot_setup_sleep(void)
 {
+	int i;
+
 	tboot->num_mac_regions = 0;
 
-	/* S3 resume code */
-	add_mac_region(acpi_wakeup_address, WAKEUP_SIZE);
+	for (i = 0; i < e820.nr_map; i++) {
+		if ((e820.map[i].type != E820_RAM)
+		 && (e820.map[i].type != E820_RESERVED_KERN))
+			continue;
 
-#ifdef CONFIG_X86_TRAMPOLINE
-	/* AP trampoline code */
-	add_mac_region(virt_to_phys(trampoline_base), TRAMPOLINE_SIZE);
-#endif
-
-	/* kernel code + data + bss */
-	add_mac_region(virt_to_phys(_text), _end - _text);
+		add_mac_region(e820.map[i].addr, e820.map[i].size);
+	}
 
 	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
 }

Re: [PATCH v2] intel_txt: add support for S3 memory integrity protection within Intel(R) TXT launched kernel

[Pavel Machek]

> v2: Based on a complexity analysis and tradeoff, we moved all MAC'ing into tboot.
> 
> This patch adds support for S3 memory integrity protection within an Intel(R) TXT launched kernel, for all kernel and userspace memory.  All RAM used by the kernel and userspace, as indicated by memory ranges of type E820_RAM and E820_RESERVED_KERN in the e820 table, will be integrity protected.
> 

Please wrap at 80 columns.

> The MAINTAINERS file is also updated to reflect the maintainers of
>  the TXT-related code.

> Signed-off-by: Shane Wang <shane.wang@intel.com>
> Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

Certainly looks better now. ACK.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH v3] intel_txt: add support for S3 memory integrity protection within Intel(R) TXT launched kernel

[Shane Wang]

[-- Attachment #1: Type: text/plain, Size: 5120 bytes --]

<compared with v2, this patch adds a check of array size in tboot.c, and a note 
to specify which c/s of tboot supports this kind of MACing in intel_txt.txt>

v3: Based on a complexity analysis and tradeoff, we moved all MAC'ing into
tboot.

This patch adds support for S3 memory integrity protection within an Intel(R)
TXT launched kernel, for all kernel and userspace memory.  All RAM used by the
kernel and userspace, as indicated by memory ranges of type E820_RAM and
E820_RESERVED_KERN in the e820 table, will be integrity protected.

The MAINTAINERS file is also updated to reflect the maintainers of the
TXT-related code.

Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

  Documentation/intel_txt.txt |   16 +++++++++-------
  MAINTAINERS                 |   11 +++++++++++
  arch/x86/include/asm/e820.h |    7 ++++++-
  arch/x86/kernel/tboot.c     |   20 +++++++++++---------
  4 files changed, 37 insertions(+), 17 deletions(-)

diff -r d2911aa1461d Documentation/intel_txt.txt
--- a/Documentation/intel_txt.txt	Thu Mar 04 09:37:53 2010 -0500
+++ b/Documentation/intel_txt.txt	Wed Mar 10 08:18:48 2010 -0500
@@ -161,13 +161,15 @@ o  In order to put a system into any of
        has been restored, it will restore the TPM PCRs and then
        transfer control back to the kernel's S3 resume vector.
        In order to preserve system integrity across S3, the kernel
-      provides tboot with a set of memory ranges (kernel
-      code/data/bss, S3 resume code, and AP trampoline) that tboot
-      will calculate a MAC (message authentication code) over and then
-      seal with the TPM.  On resume and once the measured environment
-      has been re-established, tboot will re-calculate the MAC and
-      verify it against the sealed value.  Tboot's policy determines
-      what happens if the verification fails.
+      provides tboot with a set of memory ranges (RAM and RESERVED_KERN
+      in the e820 table, but not any memory that BIOS might alter over
+      the S3 transition) that tboot will calculate a MAC (message
+      authentication code) over and then seal with the TPM. On resume
+      and once the measured environment has been re-established, tboot
+      will re-calculate the MAC and verify it against the sealed value.
+      Tboot's policy determines what happens if the verification fails.
+      Note that the c/s 194 of tboot which has the new MAC code supports
+      this.

  That's pretty much it for TXT support.

diff -r d2911aa1461d MAINTAINERS
--- a/MAINTAINERS	Thu Mar 04 09:37:53 2010 -0500
+++ b/MAINTAINERS	Wed Mar 10 08:18:48 2010 -0500
@@ -2891,6 +2891,17 @@ F:	Documentation/networking/README.ipw22
  F:	Documentation/networking/README.ipw2200
  F:	drivers/net/wireless/ipw2x00/ipw2200.*

+INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
+M:	Joseph Cihula <joseph.cihula@intel.com>
+M:	Shane Wang <shane.wang@intel.com>
+L:	tboot-devel@lists.sourceforge.net
+W:	http://tboot.sourceforge.net
+T:	Mercurial http://www.bughost.org/repos.hg/tboot.hg
+S:	Supported
+F:	Documentation/intel_txt.txt
+F:	include/linux/tboot.h
+F:	arch/x86/kernel/tboot.c
+
  INTEL WIRELESS WIMAX CONNECTION 2400
  M:	Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
  M:	linux-wimax@intel.com
diff -r d2911aa1461d arch/x86/include/asm/e820.h
--- a/arch/x86/include/asm/e820.h	Thu Mar 04 09:37:53 2010 -0500
+++ b/arch/x86/include/asm/e820.h	Wed Mar 10 08:18:48 2010 -0500
@@ -45,7 +45,12 @@
  #define E820_NVS	4
  #define E820_UNUSABLE	5

-/* reserved RAM used by kernel itself */
+/*
+ * reserved RAM used by kernel itself
+ * if CONFIG_INTEL_TXT is enabled, memory of this type will be
+ * included in the S3 integrity calculation and so should not include
+ * any memory that BIOS might alter over the S3 transition
+ */
  #define E820_RESERVED_KERN        128

  #ifndef __ASSEMBLY__
diff -r d2911aa1461d arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Thu Mar 04 09:37:53 2010 -0500
+++ b/arch/x86/kernel/tboot.c	Wed Mar 10 08:18:48 2010 -0500
@@ -130,6 +130,9 @@ static void add_mac_region(phys_addr_t s
  	struct tboot_mac_region *mr;
  	phys_addr_t end = start + size;

+	if (tboot->num_mac_regions >= MAX_TB_MAC_REGIONS)
+		panic("tboot: Too many MAC regions\n");
+
  	if (start && size) {
  		mr = &tboot->mac_regions[tboot->num_mac_regions++];
  		mr->start = round_down(start, PAGE_SIZE);
@@ -139,18 +142,17 @@ static void add_mac_region(phys_addr_t s

  static void __init tboot_setup_sleep(void)
  {
+	int i;
+
  	tboot->num_mac_regions = 0;

-	/* S3 resume code */
-	add_mac_region(acpi_wakeup_address, WAKEUP_SIZE);
+	for (i = 0; i < e820.nr_map; i++) {
+		if ((e820.map[i].type != E820_RAM)
+		 && (e820.map[i].type != E820_RESERVED_KERN))
+			continue;

-#ifdef CONFIG_X86_TRAMPOLINE
-	/* AP trampoline code */
-	add_mac_region(virt_to_phys(trampoline_base), TRAMPOLINE_SIZE);
-#endif
-
-	/* kernel code + data + bss */
-	add_mac_region(virt_to_phys(_text), _end - _text);
+		add_mac_region(e820.map[i].addr, e820.map[i].size);
+	}

  	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
  }


[-- Attachment #2: s3_memory_integrity.patch --]
[-- Type: text/plain, Size: 4688 bytes --]

v3: Based on a complexity analysis and tradeoff, we moved all MAC'ing into
tboot.

This patch adds support for S3 memory integrity protection within an Intel(R)
TXT launched kernel, for all kernel and userspace memory.  All RAM used by the
kernel and userspace, as indicated by memory ranges of type E820_RAM and
E820_RESERVED_KERN in the e820 table, will be integrity protected.

The MAINTAINERS file is also updated to reflect the maintainers of the
TXT-related code.

Signed-off-by: Shane Wang <shane.wang@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

diff -r d2911aa1461d Documentation/intel_txt.txt
--- a/Documentation/intel_txt.txt	Thu Mar 04 09:37:53 2010 -0500
+++ b/Documentation/intel_txt.txt	Wed Mar 10 08:18:48 2010 -0500
@@ -161,13 +161,15 @@ o  In order to put a system into any of 
       has been restored, it will restore the TPM PCRs and then
       transfer control back to the kernel's S3 resume vector.
       In order to preserve system integrity across S3, the kernel
-      provides tboot with a set of memory ranges (kernel
-      code/data/bss, S3 resume code, and AP trampoline) that tboot
-      will calculate a MAC (message authentication code) over and then
-      seal with the TPM.  On resume and once the measured environment
-      has been re-established, tboot will re-calculate the MAC and
-      verify it against the sealed value.  Tboot's policy determines
-      what happens if the verification fails.
+      provides tboot with a set of memory ranges (RAM and RESERVED_KERN
+      in the e820 table, but not any memory that BIOS might alter over
+      the S3 transition) that tboot will calculate a MAC (message
+      authentication code) over and then seal with the TPM. On resume
+      and once the measured environment has been re-established, tboot
+      will re-calculate the MAC and verify it against the sealed value.
+      Tboot's policy determines what happens if the verification fails.
+      Note that the c/s 194 of tboot which has the new MAC code supports
+      this.
 
 That's pretty much it for TXT support.
 
diff -r d2911aa1461d MAINTAINERS
--- a/MAINTAINERS	Thu Mar 04 09:37:53 2010 -0500
+++ b/MAINTAINERS	Wed Mar 10 08:18:48 2010 -0500
@@ -2891,6 +2891,17 @@ F:	Documentation/networking/README.ipw22
 F:	Documentation/networking/README.ipw2200
 F:	drivers/net/wireless/ipw2x00/ipw2200.*
 
+INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
+M:	Joseph Cihula <joseph.cihula@intel.com>
+M:	Shane Wang <shane.wang@intel.com>
+L:	tboot-devel@lists.sourceforge.net
+W:	http://tboot.sourceforge.net
+T:	Mercurial http://www.bughost.org/repos.hg/tboot.hg
+S:	Supported
+F:	Documentation/intel_txt.txt
+F:	include/linux/tboot.h
+F:	arch/x86/kernel/tboot.c
+
 INTEL WIRELESS WIMAX CONNECTION 2400
 M:	Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
 M:	linux-wimax@intel.com
diff -r d2911aa1461d arch/x86/include/asm/e820.h
--- a/arch/x86/include/asm/e820.h	Thu Mar 04 09:37:53 2010 -0500
+++ b/arch/x86/include/asm/e820.h	Wed Mar 10 08:18:48 2010 -0500
@@ -45,7 +45,12 @@
 #define E820_NVS	4
 #define E820_UNUSABLE	5
 
-/* reserved RAM used by kernel itself */
+/*
+ * reserved RAM used by kernel itself
+ * if CONFIG_INTEL_TXT is enabled, memory of this type will be
+ * included in the S3 integrity calculation and so should not include
+ * any memory that BIOS might alter over the S3 transition
+ */
 #define E820_RESERVED_KERN        128
 
 #ifndef __ASSEMBLY__
diff -r d2911aa1461d arch/x86/kernel/tboot.c
--- a/arch/x86/kernel/tboot.c	Thu Mar 04 09:37:53 2010 -0500
+++ b/arch/x86/kernel/tboot.c	Wed Mar 10 08:18:48 2010 -0500
@@ -130,6 +130,9 @@ static void add_mac_region(phys_addr_t s
 	struct tboot_mac_region *mr;
 	phys_addr_t end = start + size;
 
+	if (tboot->num_mac_regions >= MAX_TB_MAC_REGIONS)
+		panic("tboot: Too many MAC regions\n");
+
 	if (start && size) {
 		mr = &tboot->mac_regions[tboot->num_mac_regions++];
 		mr->start = round_down(start, PAGE_SIZE);
@@ -139,18 +142,17 @@ static void add_mac_region(phys_addr_t s
 
 static void __init tboot_setup_sleep(void)
 {
+	int i;
+
 	tboot->num_mac_regions = 0;
 
-	/* S3 resume code */
-	add_mac_region(acpi_wakeup_address, WAKEUP_SIZE);
+	for (i = 0; i < e820.nr_map; i++) {
+		if ((e820.map[i].type != E820_RAM)
+		 && (e820.map[i].type != E820_RESERVED_KERN))
+			continue;
 
-#ifdef CONFIG_X86_TRAMPOLINE
-	/* AP trampoline code */
-	add_mac_region(virt_to_phys(trampoline_base), TRAMPOLINE_SIZE);
-#endif
-
-	/* kernel code + data + bss */
-	add_mac_region(virt_to_phys(_text), _end - _text);
+		add_mac_region(e820.map[i].addr, e820.map[i].size);
+	}
 
 	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
 }

Re: [PATCH v3] intel_txt: add support for S3 memory integrity protection within Intel(R) TXT launched kernel

[Rafael J. Wysocki]

On Wednesday 10 March 2010, Shane Wang wrote:
> <compared with v2, this patch adds a check of array size in tboot.c, and a note 
> to specify which c/s of tboot supports this kind of MACing in intel_txt.txt>
> 
> v3: Based on a complexity analysis and tradeoff, we moved all MAC'ing into
> tboot.
> 
> This patch adds support for S3 memory integrity protection within an Intel(R)
> TXT launched kernel, for all kernel and userspace memory.  All RAM used by the
> kernel and userspace, as indicated by memory ranges of type E820_RAM and
> E820_RESERVED_KERN in the e820 table, will be integrity protected.
> 
> The MAINTAINERS file is also updated to reflect the maintainers of the
> TXT-related code.
> 
> Signed-off-by: Shane Wang <shane.wang@intel.com>
> Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

>   Documentation/intel_txt.txt |   16 +++++++++-------
>   MAINTAINERS                 |   11 +++++++++++
>   arch/x86/include/asm/e820.h |    7 ++++++-
>   arch/x86/kernel/tboot.c     |   20 +++++++++++---------
>   4 files changed, 37 insertions(+), 17 deletions(-)
> 
> diff -r d2911aa1461d Documentation/intel_txt.txt
> --- a/Documentation/intel_txt.txt	Thu Mar 04 09:37:53 2010 -0500
> +++ b/Documentation/intel_txt.txt	Wed Mar 10 08:18:48 2010 -0500
> @@ -161,13 +161,15 @@ o  In order to put a system into any of
>         has been restored, it will restore the TPM PCRs and then
>         transfer control back to the kernel's S3 resume vector.
>         In order to preserve system integrity across S3, the kernel
> -      provides tboot with a set of memory ranges (kernel
> -      code/data/bss, S3 resume code, and AP trampoline) that tboot
> -      will calculate a MAC (message authentication code) over and then
> -      seal with the TPM.  On resume and once the measured environment
> -      has been re-established, tboot will re-calculate the MAC and
> -      verify it against the sealed value.  Tboot's policy determines
> -      what happens if the verification fails.
> +      provides tboot with a set of memory ranges (RAM and RESERVED_KERN
> +      in the e820 table, but not any memory that BIOS might alter over
> +      the S3 transition) that tboot will calculate a MAC (message
> +      authentication code) over and then seal with the TPM. On resume
> +      and once the measured environment has been re-established, tboot
> +      will re-calculate the MAC and verify it against the sealed value.
> +      Tboot's policy determines what happens if the verification fails.
> +      Note that the c/s 194 of tboot which has the new MAC code supports
> +      this.
> 
>   That's pretty much it for TXT support.
> 
> diff -r d2911aa1461d MAINTAINERS
> --- a/MAINTAINERS	Thu Mar 04 09:37:53 2010 -0500
> +++ b/MAINTAINERS	Wed Mar 10 08:18:48 2010 -0500
> @@ -2891,6 +2891,17 @@ F:	Documentation/networking/README.ipw22
>   F:	Documentation/networking/README.ipw2200
>   F:	drivers/net/wireless/ipw2x00/ipw2200.*
> 
> +INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
> +M:	Joseph Cihula <joseph.cihula@intel.com>
> +M:	Shane Wang <shane.wang@intel.com>
> +L:	tboot-devel@lists.sourceforge.net
> +W:	http://tboot.sourceforge.net
> +T:	Mercurial http://www.bughost.org/repos.hg/tboot.hg
> +S:	Supported
> +F:	Documentation/intel_txt.txt
> +F:	include/linux/tboot.h
> +F:	arch/x86/kernel/tboot.c
> +
>   INTEL WIRELESS WIMAX CONNECTION 2400
>   M:	Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
>   M:	linux-wimax@intel.com
> diff -r d2911aa1461d arch/x86/include/asm/e820.h
> --- a/arch/x86/include/asm/e820.h	Thu Mar 04 09:37:53 2010 -0500
> +++ b/arch/x86/include/asm/e820.h	Wed Mar 10 08:18:48 2010 -0500
> @@ -45,7 +45,12 @@
>   #define E820_NVS	4
>   #define E820_UNUSABLE	5
> 
> -/* reserved RAM used by kernel itself */
> +/*
> + * reserved RAM used by kernel itself
> + * if CONFIG_INTEL_TXT is enabled, memory of this type will be
> + * included in the S3 integrity calculation and so should not include
> + * any memory that BIOS might alter over the S3 transition
> + */
>   #define E820_RESERVED_KERN        128
> 
>   #ifndef __ASSEMBLY__
> diff -r d2911aa1461d arch/x86/kernel/tboot.c
> --- a/arch/x86/kernel/tboot.c	Thu Mar 04 09:37:53 2010 -0500
> +++ b/arch/x86/kernel/tboot.c	Wed Mar 10 08:18:48 2010 -0500
> @@ -130,6 +130,9 @@ static void add_mac_region(phys_addr_t s
>   	struct tboot_mac_region *mr;
>   	phys_addr_t end = start + size;
> 
> +	if (tboot->num_mac_regions >= MAX_TB_MAC_REGIONS)
> +		panic("tboot: Too many MAC regions\n");
> +
>   	if (start && size) {
>   		mr = &tboot->mac_regions[tboot->num_mac_regions++];
>   		mr->start = round_down(start, PAGE_SIZE);
> @@ -139,18 +142,17 @@ static void add_mac_region(phys_addr_t s
> 
>   static void __init tboot_setup_sleep(void)
>   {
> +	int i;
> +
>   	tboot->num_mac_regions = 0;
> 
> -	/* S3 resume code */
> -	add_mac_region(acpi_wakeup_address, WAKEUP_SIZE);
> +	for (i = 0; i < e820.nr_map; i++) {
> +		if ((e820.map[i].type != E820_RAM)
> +		 && (e820.map[i].type != E820_RESERVED_KERN))
> +			continue;
> 
> -#ifdef CONFIG_X86_TRAMPOLINE
> -	/* AP trampoline code */
> -	add_mac_region(virt_to_phys(trampoline_base), TRAMPOLINE_SIZE);
> -#endif
> -
> -	/* kernel code + data + bss */
> -	add_mac_region(virt_to_phys(_text), _end - _text);
> +		add_mac_region(e820.map[i].addr, e820.map[i].size);
> +	}
> 
>   	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;
>   }
> 
> 

[tip:x86/txt] x86, tboot: Add support for S3 memory integrity protection

[tip-bot for Shane Wang]

Commit-ID:  4bd96a7a8185755b091233b16034c7436cbf57af
Gitweb:     http://git.kernel.org/tip/4bd96a7a8185755b091233b16034c7436cbf57af
Author:     Shane Wang <shane.wang@intel.com>
AuthorDate: Wed, 10 Mar 2010 14:36:10 +0800
Committer:  H. Peter Anvin <hpa@zytor.com>
CommitDate: Fri, 19 Mar 2010 13:39:58 -0700

x86, tboot: Add support for S3 memory integrity protection

This patch adds support for S3 memory integrity protection within an Intel(R)
TXT launched kernel, for all kernel and userspace memory.  All RAM used by the
kernel and userspace, as indicated by memory ranges of type E820_RAM and
E820_RESERVED_KERN in the e820 table, will be integrity protected.

The MAINTAINERS file is also updated to reflect the maintainers of the
TXT-related code.

All MACing is done in tboot, based on a complexity analysis and tradeoff.

v3: Compared with v2, this patch adds a check of array size in
tboot.c, and a note to specify which c/s of tboot supports this kind
of MACing in intel_txt.txt.

Signed-off-by: Shane Wang <shane.wang@intel.com>
LKML-Reference: <4B973DDA.6050902@intel.com>
Signed-off-by: Joseph Cihula <joseph.cihula@intel.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 Documentation/intel_txt.txt |   16 +++++++++-------
 MAINTAINERS                 |   11 +++++++++++
 arch/x86/include/asm/e820.h |    7 ++++++-
 arch/x86/kernel/tboot.c     |   20 +++++++++++---------
 4 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/Documentation/intel_txt.txt b/Documentation/intel_txt.txt
index f40a1f0..87c8990 100644
--- a/Documentation/intel_txt.txt
+++ b/Documentation/intel_txt.txt
@@ -161,13 +161,15 @@ o  In order to put a system into any of the sleep states after a TXT
       has been restored, it will restore the TPM PCRs and then
       transfer control back to the kernel's S3 resume vector.
       In order to preserve system integrity across S3, the kernel
-      provides tboot with a set of memory ranges (kernel
-      code/data/bss, S3 resume code, and AP trampoline) that tboot
-      will calculate a MAC (message authentication code) over and then
-      seal with the TPM.  On resume and once the measured environment
-      has been re-established, tboot will re-calculate the MAC and
-      verify it against the sealed value.  Tboot's policy determines
-      what happens if the verification fails.
+      provides tboot with a set of memory ranges (RAM and RESERVED_KERN
+      in the e820 table, but not any memory that BIOS might alter over
+      the S3 transition) that tboot will calculate a MAC (message
+      authentication code) over and then seal with the TPM. On resume
+      and once the measured environment has been re-established, tboot
+      will re-calculate the MAC and verify it against the sealed value.
+      Tboot's policy determines what happens if the verification fails.
+      Note that the c/s 194 of tboot which has the new MAC code supports
+      this.
 
 That's pretty much it for TXT support.
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 47cc449..d3072cb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2940,6 +2940,17 @@ S:	Odd Fixes
 F:	Documentation/networking/README.ipw2200
 F:	drivers/net/wireless/ipw2x00/ipw2200.*
 
+INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
+M:	Joseph Cihula <joseph.cihula@intel.com>
+M:	Shane Wang <shane.wang@intel.com>
+L:	tboot-devel@lists.sourceforge.net
+W:	http://tboot.sourceforge.net
+T:	Mercurial http://www.bughost.org/repos.hg/tboot.hg
+S:	Supported
+F:	Documentation/intel_txt.txt
+F:	include/linux/tboot.h
+F:	arch/x86/kernel/tboot.c
+
 INTEL WIRELESS WIMAX CONNECTION 2400
 M:	Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
 M:	linux-wimax@intel.com
diff --git a/arch/x86/include/asm/e820.h b/arch/x86/include/asm/e820.h
index 0e22296..ec8a52d 100644
--- a/arch/x86/include/asm/e820.h
+++ b/arch/x86/include/asm/e820.h
@@ -45,7 +45,12 @@
 #define E820_NVS	4
 #define E820_UNUSABLE	5
 
-/* reserved RAM used by kernel itself */
+/*
+ * reserved RAM used by kernel itself
+ * if CONFIG_INTEL_TXT is enabled, memory of this type will be
+ * included in the S3 integrity calculation and so should not include
+ * any memory that BIOS might alter over the S3 transition
+ */
 #define E820_RESERVED_KERN        128
 
 #ifndef __ASSEMBLY__
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 86c9f91..cc2c604 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -175,6 +175,9 @@ static void add_mac_region(phys_addr_t start, unsigned long size)
 	struct tboot_mac_region *mr;
 	phys_addr_t end = start + size;
 
+	if (tboot->num_mac_regions >= MAX_TB_MAC_REGIONS)
+		panic("tboot: Too many MAC regions\n");
+
 	if (start && size) {
 		mr = &tboot->mac_regions[tboot->num_mac_regions++];
 		mr->start = round_down(start, PAGE_SIZE);
@@ -184,18 +187,17 @@ static void add_mac_region(phys_addr_t start, unsigned long size)
 
 static int tboot_setup_sleep(void)
 {
+	int i;
+
 	tboot->num_mac_regions = 0;
 
-	/* S3 resume code */
-	add_mac_region(acpi_wakeup_address, WAKEUP_SIZE);
+	for (i = 0; i < e820.nr_map; i++) {
+		if ((e820.map[i].type != E820_RAM)
+		 && (e820.map[i].type != E820_RESERVED_KERN))
+			continue;
 
-#ifdef CONFIG_X86_TRAMPOLINE
-	/* AP trampoline code */
-	add_mac_region(virt_to_phys(trampoline_base), TRAMPOLINE_SIZE);
-#endif
-
-	/* kernel code + data + bss */
-	add_mac_region(virt_to_phys(_text), _end - _text);
+		add_mac_region(e820.map[i].addr, e820.map[i].size);
+	}
 
 	tboot->acpi_sinfo.kernel_s3_resume_vector = acpi_wakeup_address;