Subject: Re: [PATCH v1 3/3] x86/mm: Encrypt the initrd earlier for BSP
 microcode update
To: Borislav Petkov <bp@alien8.de>
Cc: x86@kernel.org, Brijesh Singh <brijesh.singh@amd.com>,
        linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Thomas Gleixner <tglx@linutronix.de>
References: <20171207233342.29646.12858.stgit@tlendack-t1.amdoffice.net>
 <20171207233410.29646.27816.stgit@tlendack-t1.amdoffice.net>
 <20171221144925.76ge77xraapgzm36@pd.tnic>
From: Tom Lendacky <thomas.lendacky@amd.com>
Message-ID: <cf742b88-49ac-7420-ebee-3523c267bde4@amd.com>
Date: Thu, 21 Dec 2017 10:48:29 -0600
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: <20171221144925.76ge77xraapgzm36@pd.tnic>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Dec 2017 16:48:35.1673 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 1ecec159-c511-424b-4589-08d54892adc7
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR12MB1141
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3436
Lines: 86

On 12/21/2017 8:49 AM, Borislav Petkov wrote:
> On Thu, Dec 07, 2017 at 05:34:10PM -0600, Tom Lendacky wrote:
>> Currently the BSP microcode update code examines the initrd very early
>> in the boot process.  If SME is active, the initrd is treated as being
>> encrypted but it has not been encrypted (in place) yet.  Update the
>> early boot code that encrypts the kernel to also encrypt the initrd so
>> that early BSP microcode updates work.
> 
> ...
> 
>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>> index 1f0efb8..60df247 100644
>> --- a/arch/x86/mm/mem_encrypt.c
>> +++ b/arch/x86/mm/mem_encrypt.c
>> @@ -731,11 +731,12 @@ static unsigned long __init sme_pgtable_calc(unsigned long len)
>>  	return total;
>>  }
>>  
>> -void __init sme_encrypt_kernel(void)
>> +void __init sme_encrypt_kernel(struct boot_params *bp)
>>  {
>>  	unsigned long workarea_start, workarea_end, workarea_len;
>>  	unsigned long execute_start, execute_end, execute_len;
>>  	unsigned long kernel_start, kernel_end, kernel_len;
>> +	unsigned long initrd_start, initrd_end, initrd_len;
>>  	unsigned long pgtable_area_len;
>>  	unsigned long decrypted_base;
>>  	pgd_t *pgd;
>> @@ -744,14 +745,15 @@ void __init sme_encrypt_kernel(void)
>>  		return;
>>  
>>  	/*
>> -	 * Prepare for encrypting the kernel by building new pagetables with
>> -	 * the necessary attributes needed to encrypt the kernel in place.
>> +	 * Prepare for encrypting the kernel and initrd by building new
>> +	 * pagetables with the necessary attributes needed to encrypt the
>> +	 * kernel in place.
>>  	 *
>>  	 *   One range of virtual addresses will map the memory occupied
>> -	 *   by the kernel as encrypted.
>> +	 *   by the kernel and initrd as encrypted.
>>  	 *
>>  	 *   Another range of virtual addresses will map the memory occupied
>> -	 *   by the kernel as decrypted and write-protected.
>> +	 *   by the kernel and initrd as decrypted and write-protected.
>>  	 *
>>  	 *     The use of write-protect attribute will prevent any of the
>>  	 *     memory from being cached.
>> @@ -762,6 +764,20 @@ void __init sme_encrypt_kernel(void)
>>  	kernel_end = ALIGN(__pa_symbol(_end), PMD_PAGE_SIZE);
>>  	kernel_len = kernel_end - kernel_start;
>>  
>> +	initrd_start = 0;
>> +	initrd_end = 0;
>> +	initrd_len = 0;
>> +#ifdef CONFIG_BLK_DEV_INITRD
>> +	initrd_len = (unsigned long)bp->hdr.ramdisk_size |
>> +		     ((unsigned long)bp->ext_ramdisk_size << 32);
>> +	if (initrd_len) {
>> +		initrd_start = (unsigned long)bp->hdr.ramdisk_image |
>> +			       ((unsigned long)bp->ext_ramdisk_image << 32);
>> +		initrd_end = PAGE_ALIGN(initrd_start + initrd_len);
>> +		initrd_len = initrd_end - initrd_start;
>> +	}
>> +#endif
> 
> In a prepatch, pls make get_ramdisk_image() and get_ramdisk_size() from
> arch/x86/kernel/setup.c accessible to this code too. Also, add dummies
> for the !CONFIG_BLK_DEV_INITRD case so that you can simply call them
> here, regardless of the CONFIG_BLK_DEV_INITRD setting.
> 
> Then you won't need boot_params ptr either and that would simplify the
> diff a bit.

This is very early in the boot and the boot parameters have not been
copied to boot_params yet, so I need the pointer.  And since the routines
in arch/x86/kernel/setup.c also use boot_params, those would have to be
modified to accept a pointer rather than automatically using boot_params.
I'm not sure it's worth all that.

Thanks,
Tom

> 
> Thx.
>