Re: [PATCH v2] x86/mm/cpa: Warn if set_memory_XXcrypted() fails

[kirill . shutemov]

On Fri, Oct 27, 2023 at 02:47:44PM -0700, Rick Edgecombe wrote:
> On TDX it is possible for the untrusted host to cause
> set_memory_encrypted() or set_memory_decrypted() to fail such that an
> error is returned and the resulting memory is shared. Callers need to take
> care to handle these errors to avoid returning decrypted (shared) memory to
> the page allocator, which could lead to functional or security issues.
> In terms of security, the problematic case is guest PTEs mapping the
> shared alias GFNs, since the VMM has control of the shared mapping in the
> EPT/NPT.
> 
> Such conversion errors may herald future system instability, but are
> temporarily survivable with proper handling in the caller. The kernel
> traditionally makes every effort to keep running, but it is expected that
> some coco guests may prefer to play it safe security-wise, and panic in
> this case. To accommodate both cases, warn when the arch breakouts for
> converting memory at the VMM layer return an error to CPA. Security focused
> users can rely on panic_on_warn to defend against bugs in the callers. Some
> VMMs are not known to behave in the troublesome way, so users that would
> like to terminate on any unusual behavior by the VMM around this will be
> covered as well.
> 
> Since the arch breakouts host the logic for handling coco implementation
> specific errors, an error returned from them means that the set_memory()
> call is out of options for handling the error internally. Make this the
> condition to warn about.
> 
> It is possible that very rarely these functions could fail due to guest
> memory pressure (in the case of failing to allocate a huge page when
> splitting a page table). Don't warn in this case because it is a lot less
> likely to indicate an attack by the host and it is not clear which
> set_memory() calls should get the same treatment. That corner should be
> addressed by future work that considers the more general problem and not
> just papers over a single set_memory() variant.
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> Suggested-by: Michael Kelley (LINUX) <mikelley@xxxxxxxxxxxxx>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>

Tha patch looks good:

Reviewed-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>

It intended to get upstream alongside with the caller fixes to leak memory
on failure, right? Maybe get it into one patchset?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov