RE: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently

Michael Kelley (LINUX)
2022-11-18T02:55:49Z

From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> On 11/16/22 10:41 AM, Michael Kelley wrote:
> > Current code in sme_postprocess_startup() decrypts the bss_decrypted
> > section when sme_me_mask is non-zero. But code in
> > mem_encrypt_free_decrytped_mem() re-encrypts the unused portion based
> > on CC_ATTR_MEM_ENCRYPT. In a Hyper-V guest VM using vTOM, these
> > conditions are not equivalent as sme_me_mask is always zero when
> > using vTOM. Consequently, mem_encrypt_free_decrypted_mem() attempts
> > to re-encrypt memory that was never decrypted.
> >
> > Fix this in mem_encrypt_free_decrypted_mem() by conditioning the
> > re-encryption on the same test for non-zero sme_me_mask. Hyper-V
> > guests using vTOM don't need the bss_decrypted section to be
> > decrypted, so skipping the decryption/re-encryption doesn't cause
> > a problem.
> >
>
> Do you think it needs Fixes tag?
>

At least for my purposes, it doesn't. The original assumption that non-zero
sme_me_mask and CC_ATTR_MEM_ENCRYPT are equivalent was valid until
this patch series where Hyper-V guests are reporting CC_ATTR_MEM_ENCRYPT
as "true" but sme_me_mask is zero. This patch series won't be backported,
so the old assumption remains valid for older kernels. There's no benefit in
backporting the change.

But I had not thought about TDX. In the TDX case, it appears that
sme_postprocess_startup() will not decrypt the bss_decrypted section.
The corresponding mem_encrypt_free_decrypted_mem() is a no-op unless
CONFIG_AMD_MEM_ENCRYPT is set. But maybe if someone builds a
kernel image that supports both TDX and AMD encryption, it could break
at runtime on a TDX system. I would also note that on a TDX system
without CONFIG_AMD_MEM_ENCRYPT, the unused memory in the
bss_decrypted section never gets freed.

But check my logic. :-) I'm not averse to adding the Fixes: tag if there's a
scenario for TDX where doing the backport will solve a real problem.

And thanks for reviewing the code!

Michael