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

[Edgecombe, Rick P]

On Fri, 2023-10-27 at 14:47 -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.

x86 maintainers,

If you don't want this patch yet but are ok with the general approach,
could you share? I didn't want to start fixing up the callers until
this was settled. If you can share you are ok with the approach I can
start in the meantime.

Thanks,

Rick