intel_txt: add s3 userspace memory integrity verification

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

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

arch/x86/kernel/entry_64.S +14 -0

+
+	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);

arch/x86/kernel/entry_64.S +16 -0

+	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;
+

arch/x86/kernel/entry_64.S +15 -0

+					|| (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);

arch/x86/kernel/entry_64.S +5 -0

 	}
 
 	tboot_shutdown(acpi_shutdown_map[sleep_state]);
+}
+
+static void tboot_sx_resume(void)
+{
+	vmac_t mac;

arch/x86/kernel/entry_64.S +14 -0

+
+	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);

arch/x86/kernel/entry_64.S +16 -0

+	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;
+

arch/x86/kernel/entry_64.S +15 -0

+					|| (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);

arch/x86/kernel/entry_64.S +5 -0

 	}
 
 	tboot_shutdown(acpi_shutdown_map[sleep_state]);
+}
+
+static void tboot_sx_resume(void)
+{
+	vmac_t mac;

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

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

From: Pavel Machek [mailto:[email protected]]
Sent: Friday, December 04, 2009 12:29 AM

Quoted 2 lines

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

It does not work.

Quoted 2 lines

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.

Quoted 9 lines

arch/x86/kernel/tboot.c +9 -0

+void tboot_do_suspend_lowlevel(void)
+{
+   int ret = -1;
+
+   if (!tboot_enabled()) {
+           do_suspend_lowlevel();
+           return;
+   }
+

arch/x86/kernel/tboot.c void tboot_sleep(u8 sleep_state, u32 pm1 · line 517

Quoted 4 lines

arch/x86/kernel/tboot.c +4 -0

+   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

Quoted 6 lines

Quoted 5 lines

arch/x86/kernel/tboot.c +5 -0

+
+   ret = tboot_pre_stack_switch();
+   if (!ret) {
+           tboot_switch_stack_call(tboot_do_suspend_lowlevel_call,
+                                   (u64)new_stack_ptr);

Quoted 3 lines

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

Quoted 6 lines

Quoted 5 lines

arch/x86/kernel/tboot.c +5 -0

+
+   ret = tboot_pre_stack_switch();
+   if (!ret) {
+           tboot_switch_stack_call(tboot_do_suspend_lowlevel_call,
+                                   (u64)new_stack_ptr);

Quoted 3 lines

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

From: Pavel Machek [mailto:[email protected]]
Sent: Friday, December 04, 2009 12:29 AM

Quoted 2 lines

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

It does not work.

Quoted 2 lines

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.

Quoted 9 lines

arch/x86/kernel/tboot.c +9 -0

+void tboot_do_suspend_lowlevel(void)
+{
+   int ret = -1;
+
+   if (!tboot_enabled()) {
+           do_suspend_lowlevel();
+           return;
+   }
+

arch/x86/kernel/tboot.c void tboot_sleep(u8 sleep_state, u32 pm1 · line 517

Quoted 4 lines

arch/x86/kernel/tboot.c +4 -0

+   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