linux-kernel - Re: [PATCH 2/2] KVM: TDX: Do not clear poisoned pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bc492cb2-1d30-4a30-9eb9-d48b09cd29a9@intel.com>
Date: Wed, 25 Jun 2025 19:25:39 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Vishal Annapurve <vannapurve@...gle.com>
CC: Tony Luck <tony.luck@...el.com>, <pbonzini@...hat.com>,
	<seanjc@...gle.com>, Borislav Petkov <bp@...en8.de>, Thomas Gleixner
	<tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Dave Hansen
	<dave.hansen@...ux.intel.com>, <x86@...nel.org>, H Peter Anvin
	<hpa@...or.com>, <linux-edac@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
	<rick.p.edgecombe@...el.com>, <kirill.shutemov@...ux.intel.com>,
	<kai.huang@...el.com>, <reinette.chatre@...el.com>, <xiaoyao.li@...el.com>,
	<tony.lindgren@...ux.intel.com>, <binbin.wu@...ux.intel.com>,
	<isaku.yamahata@...el.com>, <yan.y.zhao@...el.com>, <chao.gao@...el.com>,
	Dave Hansen <dave.hansen@...el.com>
Subject: Re: [PATCH 2/2] KVM: TDX: Do not clear poisoned pages

On 25/06/2025 17:33, Vishal Annapurve wrote:
> On Wed, Jun 18, 2025 at 7:58 AM Dave Hansen <dave.hansen@...el.com> wrote:
>>
>> On 6/18/25 05:08, Adrian Hunter wrote:
>>> --- a/arch/x86/kvm/vmx/tdx.c
>>> +++ b/arch/x86/kvm/vmx/tdx.c
>>> @@ -282,10 +282,10 @@ static void tdx_clear_page(struct page *page)
>>>       void *dest = page_to_virt(page);
>>>       unsigned long i;
>>>
>>> -     /*
>>> -      * The page could have been poisoned.  MOVDIR64B also clears
>>> -      * the poison bit so the kernel can safely use the page again.
>>> -      */
>>> +     /* Machine check handler may have poisoned the page */
>>> +     if (PageHWPoison(page))
>>> +             return;
> 
> IIUC, even if movdir64b stores contents on hwpoisoned pages, it's not
> going to cause any trouble.

No.  PageHWPoison(page) means the page should not be touched.  It must
be freed back to the allocator where it will never be allocated again.

> 
> This check should be (unlikely(PageHWPoison(page)) and even better

'unlikely' would be fine

> probably should be omitted altogether if there are no side effects of
> direct store to hwpoisoned pages.
> 
>>
>> I think the old comment needs to stay in some form.
>>
>> There are two kinds of poisons here: One from an integrity mismatch and
>> the other because the hardware decided the memory is bad. MOVDIR64B
>> clears the integrity one, but not the hardware one obviously.
> 
> To ensure I understand correctly, Am I correct in saying: movdir64b
> clearing the integrity poison is just hardware clearing the poison
> bit, software will still treat that page as poisoned?

Typically an integrity violation would have caused a machine check
and the machine check handler would have marked the page
SetPageHWPoison(page).

So we really end up with only 2 cases:
1. page is fine and PageHWPoison(page) is false
2. page may have had an integrity violation or a hardware error
(we can't tell which), and PageHWPoison(page) is true