linux-kernel - Re: [PATCH v7 3/6] mce: fix set_mce_nospec to always unmap the whole page

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 13 Apr 2022 23:41:18 +0000
From:   Jane Chu <jane.chu@...cle.com>
To:     Borislav Petkov <bp@...en8.de>
CC:     "david@...morbit.com" <david@...morbit.com>,
        "djwong@...nel.org" <djwong@...nel.org>,
        "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
        "hch@...radead.org" <hch@...radead.org>,
        "vishal.l.verma@...el.com" <vishal.l.verma@...el.com>,
        "dave.jiang@...el.com" <dave.jiang@...el.com>,
        "agk@...hat.com" <agk@...hat.com>,
        "snitzer@...hat.com" <snitzer@...hat.com>,
        "dm-devel@...hat.com" <dm-devel@...hat.com>,
        "ira.weiny@...el.com" <ira.weiny@...el.com>,
        "willy@...radead.org" <willy@...radead.org>,
        "vgoyal@...hat.com" <vgoyal@...hat.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "nvdimm@...ts.linux.dev" <nvdimm@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-xfs@...r.kernel.org" <linux-xfs@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "dave.hansen@...el.com" <dave.hansen@...el.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "luto@...nel.org" <luto@...nel.org>
Subject: Re: [PATCH v7 3/6] mce: fix set_mce_nospec to always unmap the whole
 page

On 4/12/2022 3:07 AM, Borislav Petkov wrote:
> On Tue, Apr 05, 2022 at 01:47:44PM -0600, Jane Chu wrote:
>> The set_memory_uc() approach doesn't work well in all cases.
>> For example, when "The VMM unmapped the bad page from guest
>> physical space and passed the machine check to the guest."
>> "The guest gets virtual #MC on an access to that page.
>>   When the guest tries to do set_memory_uc() and instructs
>>   cpa_flush() to do clean caches that results in taking another
>>   fault / exception perhaps because the VMM unmapped the page
>>   from the guest."
> 
> I presume this is quoting someone...

Yes, will mention Dan's name next time.

> 
>> Since the driver has special knowledge to handle NP or UC,
>> let's mark the poisoned page with NP and let driver handle it
> 
> s/let's mark/mark/
> 

Okay.

>> when it comes down to repair.
>>
>> Please refer to discussions here for more details.
>> https://lore.kernel.org/all/CAPcyv4hrXPb1tASBZUg-GgdVs0OOFKXMXLiHmktg_kFi7YBMyQ@mail.gmail.com/
>>
>> Now since poisoned page is marked as not-present, in order to
>> avoid writing to a 'np' page and trigger kernel Oops, also fix
> 
> You can write it out: "non-present page..."

Okay.

> 
>> pmem_do_write().
>>
>> Fixes: 284ce4011ba6 ("x86/memory_failure: Introduce {set, clear}_mce_nospec()")
>> Signed-off-by: Jane Chu <jane.chu@...cle.com>
>> ---
>>   arch/x86/kernel/cpu/mce/core.c |  6 +++---
>>   arch/x86/mm/pat/set_memory.c   | 18 ++++++------------
>>   drivers/nvdimm/pmem.c          | 31 +++++++------------------------
>>   include/linux/set_memory.h     |  4 ++--
>>   4 files changed, 18 insertions(+), 41 deletions(-)
> 
> For such mixed subsystem patches we probably should talk about who picks
> them up, eventually...

Hmm, maintainers' action item?

> 
>> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
>> index 981496e6bc0e..fa67bb9d1afe 100644
>> --- a/arch/x86/kernel/cpu/mce/core.c
>> +++ b/arch/x86/kernel/cpu/mce/core.c
>> @@ -579,7 +579,7 @@ static int uc_decode_notifier(struct notifier_block *nb, unsigned long val,
>>   
>>   	pfn = mce->addr >> PAGE_SHIFT;
>>   	if (!memory_failure(pfn, 0)) {
>> -		set_mce_nospec(pfn, whole_page(mce));
>> +		set_mce_nospec(pfn);
>>   		mce->kflags |= MCE_HANDLED_UC;
>>   	}
>>   
>> @@ -1316,7 +1316,7 @@ static void kill_me_maybe(struct callback_head *cb)
>>   
>>   	ret = memory_failure(p->mce_addr >> PAGE_SHIFT, flags);
>>   	if (!ret) {
>> -		set_mce_nospec(p->mce_addr >> PAGE_SHIFT, p->mce_whole_page);
>> +		set_mce_nospec(p->mce_addr >> PAGE_SHIFT);
>>   		sync_core();
>>   		return;
>>   	}
>> @@ -1342,7 +1342,7 @@ static void kill_me_never(struct callback_head *cb)
>>   	p->mce_count = 0;
>>   	pr_err("Kernel accessed poison in user space at %llx\n", p->mce_addr);
>>   	if (!memory_failure(p->mce_addr >> PAGE_SHIFT, 0))
>> -		set_mce_nospec(p->mce_addr >> PAGE_SHIFT, p->mce_whole_page);
>> +		set_mce_nospec(p->mce_addr >> PAGE_SHIFT);
>>   }
> 
> Both that ->mce_whole_page and whole_page() look unused after this.
> 

Yes, indeed, Dan also noticed, will remove while_page() in a follow-on 
patch.

thanks,
-jane