Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] x86/sme: Explicitly map new EFI memmap table as encrypted
From:   Tom Lendacky <thomas.lendacky@amd.com>
To:     linux-kernel@vger.kernel.org, x86@kernel.org,
        linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        Darren Hart <dvhart@infradead.org>,
        Andy Shevchenko <andy@infradead.org>,
        Matt Fleming <matt@codeblueprint.co.uk>, stable@vger.kernel.org
References: <ebf1eb2940405438a09d51d121ec0d02c8755558.1634752931.git.thomas.lendacky@amd.com>
Message-ID: <9d9ca009-93c5-acc3-7445-d514c7878478@amd.com>
Date:   Thu, 21 Oct 2021 08:21:15 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
In-Reply-To: <ebf1eb2940405438a09d51d121ec0d02c8755558.1634752931.git.thomas.lendacky@amd.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bndXOU52OUhlMGtOTGdvV01MeGlzTGt6bWUwM1pXbkxIRGptOVU4aFNqTFE4?=
 =?utf-8?B?TnVBTFd5SG5ZWDdDQ001enU4VEdmU3RsZTJzeGErb2huMnlMcGYvRmRTbVZY?=
 =?utf-8?B?OXFYTHpiQkxEeVBnN0JGeUNDVzlVN2lyM1YxNUxUUmwvc2Z1Zmp1TURlSVZj?=
 =?utf-8?B?SWUzYmw4RGRkY2dMeHMycWtmZDdtN3lMYjhzc09XMGN1NnRSUVoyYmF4NXE4?=
 =?utf-8?B?NXB6SERaOFJkTzRQNy94VmIrYThiTGJOZmg5SzhvV3NNbWNWK0w3Vk9WRXAr?=
 =?utf-8?B?cCtMMkxhamsxcnJBVlZwOFFGUW0yMlRxR1RXeUVEUDB5eHY5WjJlMHREQzgv?=
 =?utf-8?B?UzZzNjNHajZxOHhxeEFoSkZ0dFVBdHdncDdjNmVISHVTOWtQWi9KMDZlc3k4?=
 =?utf-8?B?OU5haGRxNGp0VTA2Kzg4bnRGUi9scjM3dXFJTnNrbm1nV0tlMzVvSExPRTdZ?=
 =?utf-8?B?eG1uRVpkd3FBdk10NHN4ZWpmOHhBZEhzQ21HZExFeXhrY1Q3VEwwaXEwYXNT?=
 =?utf-8?B?MjRoYkVYZFNtdzZWY2xkSC91YU9EMGJjakcrWEIxWjVnOC9WNmZ1OHZaSjhl?=
 =?utf-8?B?TC9BV1ErKzZEZHlLQitWakNrQjZ3YzBrakltUlV3T0hVaVpvVjBGQjlQS3FD?=
 =?utf-8?B?LzUxcWYyaEpFQ09aZ1YzSjlsNWRWT1YvWWYrSXFldkZGZ3kvdFNFWmZKRWYy?=
 =?utf-8?B?Q0FBb29KaVJmQU5yS0J3bDRTd3JKUlhZMjhYYlZ3QTA4ZEhqRU41cVhZL0E3?=
 =?utf-8?B?Y3BuNU1NaW9LTUoxOWpPMDQwd3p6bEQyYVN0cDQ0b0crNWRTdkhpMVZmVXQy?=
 =?utf-8?B?K25XMWI3amJqOEQ5RnJ1bVdPRndrcWV1ZUcvbVk1MDVXeXZQRnpOcXY4QjRp?=
 =?utf-8?B?NW85Z2xKbFJSTm5iZFJISThtTk5XYVJLZitsYS84N2hVcHdlNnRQdWtzcGM5?=
 =?utf-8?B?dWpHMzFNNHJGSVZJSFRQRW9UZE1aWHJEcnV6SW9GK0UvclpQczBqZE4va3ha?=
 =?utf-8?B?ZGxQazFFWHllall2WlhFaE9ZRUdnZTl0U09yTHp5QS9QQlQzc0grVmlXeFBs?=
 =?utf-8?B?a0ppc1NsaWJ6dklaT2xQbGlNakM1cmszcHY3L1BnTmFTTENiOWRHR3JpdWg3?=
 =?utf-8?B?dGJ4NmQxOUt6MXE1QkIrUHJOdXpDS25XeHZCUjAyY1YwejgxODgzRFF3Z3B6?=
 =?utf-8?B?RTdHaVNMellQUXEwRlVTTVY1cjBLODBPWlVhRGRtaU1VWXBYS3VodzdpQVVV?=
 =?utf-8?B?WTJST2NkZGZ4S1M1SlRvMU0xa2FJYnBXSEZxcEpySGRWMU5EcE1jSHh2ZFJH?=
 =?utf-8?B?aEpFWGtoTi8rdHl3QkJkTFUwZ2h2dk5hdzMwa01sVlZVUnZJbXlwZjMzTHBi?=
 =?utf-8?B?NkZVelFWZnhZbHJGN0REbUZvVEVuY0lYdUU4WUM4cjlyVGVZT0l6R2Iyb0dR?=
 =?utf-8?B?VmRJWVNKUVVOdFE3a2FjWmpkc3J1cU5Bb2dRVFpUV2dIWHViOUJSb3lxeDhm?=
 =?utf-8?B?T2VCTmZVYVdnd1B6ZkRmRGNMNG5uMUtBZFJxVTVjNFlZNHE4eGhiSzV4eEF4?=
 =?utf-8?B?ODc2ajA2OHR6UzdBTnF5U1p3b3ZCa2d5WlgvY0k1M3liMm56RGJISzZkaDVQ?=
 =?utf-8?B?RUhpZ3dZQjY1RUFLdGFqQVFtOVFkOXFzRVZXTkhITGJFdFdyejdGT1EzNTR1?=
 =?utf-8?B?OXFlSjUxaHZvOW40VWQwVWdJbzVGRFA0anRkY3Zaa0tTVWlXaWVTcWVuVmkz?=
 =?utf-8?B?YmVmV0NSdE12OVY2VnFzdzlnblJOVEcyMVpHNHY5blMrYjF2TmJQSUtXR2p3?=
 =?utf-8?B?cU50dE1aQmZCMG82UXZUT1V0THVnZ0hVN1FSRjN0N2s3bHptNUROeWJ4bnZ4?=
 =?utf-8?B?VFJ2N3EyV2pEUGFFMTJrTm4wTzlBL2xobWo3YjBqZ21NS0JyRXhzREFXc2l1?=
 =?utf-8?Q?guOBkgzITMU=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 840bea1c-df49-44a7-18dd-08d99495aaf1
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Oct 2021 13:21:19.3360
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: tlendack@amd.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5072
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/20/21 1:02 PM, Tom Lendacky wrote:
> Reserving memory using efi_mem_reserve() calls into the x86
> efi_arch_mem_reserve() function. This function will insert a new EFI
> memory descriptor into the EFI memory map representing the area of
> memory to be reserved and marking it as EFI runtime memory. As part
> of adding this new entry, a new EFI memory map is allocated and mapped.
> The mapping is where a problem can occur. This new memory map is mapped
> using early_memremap() and generally mapped encrypted, unless the new
> memory for the mapping happens to come from an area of memory that is
> marked as EFI_BOOT_SERVICES_DATA memory. In this case, the new memory will
> be mapped unencrypted. However, during replacement of the old memory map,
> efi_mem_type() is disabled, so the new memory map will now be long-term
> mapped encrypted (in efi.memmap), resulting in the map containing invalid
> data and causing the kernel boot to crash.
> 
> Since it is known that the area will be mapped encrypted going forward,
> explicitly map the new memory map as encrypted using early_memremap_prot().
> 
> Cc: <stable@vger.kernel.org> # 4.14.x
> Fixes: 8f716c9b5feb ("x86/mm: Add support to access boot related data in the clear")
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>   arch/x86/platform/efi/quirks.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index b15ebfe40a73..b0b848d6933a 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -277,7 +277,8 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
>   		return;
>   	}
>   
> -	new = early_memremap(data.phys_map, data.size);
> +	new = early_memremap_prot(data.phys_map, data.size,
> +				  pgprot_val(pgprot_encrypted(FIXMAP_PAGE_NORMAL)));

I should really have a comment above this as to why this version of the 
early_memremap is being used.

Let me add that (and maybe work on the commit message a bit) and submit a 
v2. But I'll hold off for a bit in case any discussion comes about.

Thanks,
Tom

>   	if (!new) {
>   		pr_err("Failed to map new boot services memmap\n");
>   		return;
>