| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 4 Dec 2009 17:07:51 +0800
From: "Wang, Shane" <shane.wang@...el.com>
To: Pavel Machek <pavel@....cz>, "Rafael J. Wysocki" <rjw@...k.pl>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
"Cihula, Joseph" <joseph.cihula@...el.com>,
"arjan@...ux.intel.com" <arjan@...ux.intel.com>,
"andi@...stfloor.org" <andi@...stfloor.org>,
"chrisw@...s-sol.org" <chrisw@...s-sol.org>,
"jmorris@...ei.org" <jmorris@...ei.org>,
"jbeulich@...ell.com" <jbeulich@...ell.com>,
"peterm@...hat.com" <peterm@...hat.com>
Subject: RE: [PATCH] intel_txt: add s3 userspace memory integrity
verification
(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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists