linux-kernel - Re: [RFC PATCH v2 15/32] x86: Add support for changing memory encryption attribute in early boot

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 27 Mar 2017 10:07:00 -0500
From:   Brijesh Singh <brijesh.singh@....com>
To:     Borislav Petkov <bp@...e.de>
CC:     <brijesh.singh@....com>, <simon.guinot@...uanux.org>,
        <linux-efi@...r.kernel.org>, <kvm@...r.kernel.org>,
        <rkrcmar@...hat.com>, <matt@...eblueprint.co.uk>,
        <linux-pci@...r.kernel.org>, <linus.walleij@...aro.org>,
        <gary.hook@....com>, <linux-mm@...ck.org>,
        <paul.gortmaker@...driver.com>, <hpa@...or.com>, <cl@...ux.com>,
        <dan.j.williams@...el.com>, <aarcange@...hat.com>,
        <sfr@...b.auug.org.au>, <andriy.shevchenko@...ux.intel.com>,
        <herbert@...dor.apana.org.au>, <bhe@...hat.com>,
        <xemul@...allels.com>, <joro@...tes.org>, <x86@...nel.org>,
        <peterz@...radead.org>, <piotr.luc@...el.com>, <mingo@...hat.com>,
        <msalter@...hat.com>, <ross.zwisler@...ux.intel.com>,
        <dyoung@...hat.com>, <thomas.lendacky@....com>, <jroedel@...e.de>,
        <keescook@...omium.org>, <arnd@...db.de>, <toshi.kani@....com>,
        <mathieu.desnoyers@...icios.com>, <luto@...nel.org>,
        <devel@...uxdriverproject.org>, <bhelgaas@...gle.com>,
        <tglx@...utronix.de>, <mchehab@...nel.org>,
        <iamjoonsoo.kim@....com>, <labbott@...oraproject.org>,
        <tony.luck@...el.com>, <alexandre.bounine@....com>,
        <kuleshovmail@...il.com>, <linux-kernel@...r.kernel.org>,
        <mcgrof@...nel.org>, <mst@...hat.com>,
        <linux-crypto@...r.kernel.org>, <tj@...nel.org>,
        <pbonzini@...hat.com>, <akpm@...ux-foundation.org>,
        <davem@...emloft.net>
Subject: Re: [RFC PATCH v2 15/32] x86: Add support for changing memory
 encryption attribute in early boot

Hi Boris,

On 03/24/2017 12:12 PM, Borislav Petkov wrote:
>>  }
>>
>> +static inline int __init early_set_memory_decrypted(void *addr,
>> +						    unsigned long size)
>> +{
>> +	return 1;
> 	^^^^^^^^
>
> return 1 when !CONFIG_AMD_MEM_ENCRYPT ?
>
> The non-early variants return 0.
>

I will fix it and use the same return value.

>> +}
>> +
>> +static inline int __init early_set_memory_encrypted(void *addr,
>> +						    unsigned long size)
>> +{
>> +	return 1;
>> +}
>> +
>>  #define __sme_pa		__pa

>> +	unsigned long pfn, npages;
>> +	unsigned long addr = (unsigned long)vaddr & PAGE_MASK;
>> +
>> +	/* We are going to change the physical page attribute from C=1 to C=0.
>> +	 * Flush the caches to ensure that all the data with C=1 is flushed to
>> +	 * memory. Any caching of the vaddr after function returns will
>> +	 * use C=0.
>> +	 */
>
> Kernel comments style is:
>
> 	/*
> 	 * A sentence ending with a full-stop.
> 	 * Another sentence. ...
> 	 * More sentences. ...
> 	 */
>

I will update to use kernel comment style.


>> +	clflush_cache_range(vaddr, size);
>> +
>> +	npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> +	pfn = slow_virt_to_phys((void *)addr) >> PAGE_SHIFT;
>> +
>> +	return kernel_map_pages_in_pgd(init_mm.pgd, pfn, addr, npages,
>> +					flags & ~sme_me_mask);
>> +
>> +}
>> +
>> +int __init early_set_memory_decrypted(void *vaddr, unsigned long size)
>> +{
>> +	unsigned long flags = get_pte_flags((unsigned long)vaddr);
>
> So this does lookup_address()...
>
>> +	return early_set_memory_enc_dec(vaddr, size, flags & ~sme_me_mask);
>
> ... and this does it too in slow_virt_to_phys(). So you do it twice per
> vaddr.
>
> So why don't you define a __slow_virt_to_phys() helper - notice
> the "__" - which returns flags in its second parameter and which
> slow_virt_to_phys() calls with a NULL second parameter in the other
> cases?
>

I will look into creating a helper function. thanks

-Brijesh