Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Mon, 30 Oct 2023 11:27:14 +0300
From:   kirill.shutemov@linux.intel.com
To:     Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc:     x86@kernel.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com, luto@kernel.org,
        peterz@infradead.org, elena.reshetova@intel.com,
        isaku.yamahata@intel.com, seanjc@google.com,
        Michael Kelley <mikelley@microsoft.com>,
        thomas.lendacky@amd.com, decui@microsoft.com,
        sathyanarayanan.kuppuswamy@linux.intel.com,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] x86/mm/cpa: Warn if set_memory_XXcrypted() fails
Message-ID: <20231030082714.pbma2bg2p354cuft@box.shutemov.name>
References: <20231027214744.1742056-1-rick.p.edgecombe@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231027214744.1742056-1-rick.p.edgecombe@intel.com>
Precedence: bulk

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@amd.com>
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Suggested-by: Michael Kelley (LINUX) <mikelley@microsoft.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

Tha patch looks good:

Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

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