[PATCH v2 0/1] mm/memory-failure: don't allow to unpoison hw corrupted page

[PATCH v2 0/1] mm/memory-failure: don't allow to unpoison hw corrupted page

[zhenwei pi]

v1 -> v2:
- this change gets protected by mf_mutex
- use -EOPNOTSUPP instead of -EPERM 

v1:
- check KPTE to avoid to unpoison hardware corrupted page

zhenwei pi (1):
  mm/memory-failure: don't allow to unpoison hw corrupted page

 mm/memory-failure.c | 9 +++++++++
 1 file changed, 9 insertions(+)

-- 
2.20.1

[PATCH v2 1/1] mm/memory-failure: don't allow to unpoison hw corrupted page

[zhenwei pi]

Currently unpoison_memory(unsigned long pfn) is designed for soft
poison(hwpoison-inject) only. Since 17fae1294ad9d, the KPTE gets
cleared on a x86 platform once hardware memory corrupts.

Unpoisoning a hardware corrupted page puts page back buddy only,
the kernel has a chance to access the page with *NOT PRESENT* KPTE.
This leads BUG during accessing on the corrupted KPTE.

Do not allow to unpoison hardware corrupted page in unpoison_memory() to
avoid BUG like this:

 Unpoison: Software-unpoisoned page 0x61234
 BUG: unable to handle page fault for address: ffff888061234000
 #PF: supervisor write access in kernel mode
 #PF: error_code(0x0002) - not-present page
 PGD 2c01067 P4D 2c01067 PUD 107267063 PMD 10382b063 PTE 800fffff9edcb062
 Oops: 0002 [#1] PREEMPT SMP NOPTI
 CPU: 4 PID: 26551 Comm: stress Kdump: loaded Tainted: G   M       OE     5.18.0.bm.1-amd64 #7
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ...
 RIP: 0010:clear_page_erms+0x7/0x10
 Code: ...
 RSP: 0000:ffffc90001107bc8 EFLAGS: 00010246
 RAX: 0000000000000000 RBX: 0000000000000901 RCX: 0000000000001000
 RDX: ffffea0001848d00 RSI: ffffea0001848d40 RDI: ffff888061234000
 RBP: ffffea0001848d00 R08: 0000000000000901 R09: 0000000000001276
 R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000001
 R13: 0000000000000000 R14: 0000000000140dca R15: 0000000000000001
 FS:  00007fd8b2333740(0000) GS:ffff88813fd00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: ffff888061234000 CR3: 00000001023d2005 CR4: 0000000000770ee0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
 PKRU: 55555554
 Call Trace:
  <TASK>
  prep_new_page+0x151/0x170
  get_page_from_freelist+0xca0/0xe20
  ? sysvec_apic_timer_interrupt+0xab/0xc0
  ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
  __alloc_pages+0x17e/0x340
  __folio_alloc+0x17/0x40
  vma_alloc_folio+0x84/0x280
  __handle_mm_fault+0x8d4/0xeb0
  handle_mm_fault+0xd5/0x2a0
  do_user_addr_fault+0x1d0/0x680
  ? kvm_read_and_reset_apf_flags+0x3b/0x50
  exc_page_fault+0x78/0x170
  asm_exc_page_fault+0x27/0x30

Fixes: 847ce401df392 ("HWPOISON: Add unpoisoning support")
Fixes: 17fae1294ad9d ("x86/{mce,mm}: Unmap the entire page if the whole page is affected and poisoned")
Cc: Naoya Horiguchi <naoya.horiguchi@nec.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 mm/memory-failure.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index b85661cbdc4a..da99a2b7ef35 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2090,6 +2090,7 @@ int unpoison_memory(unsigned long pfn)
 {
 	struct page *page;
 	struct page *p;
+	pte_t *kpte;
 	int ret = -EBUSY;
 	int freeit = 0;
 	static DEFINE_RATELIMIT_STATE(unpoison_rs, DEFAULT_RATELIMIT_INTERVAL,
@@ -2103,6 +2104,14 @@ int unpoison_memory(unsigned long pfn)
 
 	mutex_lock(&mf_mutex);
 
+	kpte = virt_to_kpte((unsigned long)page_to_virt(p));
+	if (kpte && !pte_present(*kpte)) {
+		unpoison_pr_info("Unpoison: Page was hardware poisoned %#lx\n",
+				 pfn, &unpoison_rs);
+		ret = -EOPNOTSUPP;
+		goto unlock_mutex;
+	}
+
 	if (!PageHWPoison(p)) {
 		unpoison_pr_info("Unpoison: Page was already unpoisoned %#lx\n",
 				 pfn, &unpoison_rs);
-- 
2.20.1

Re: [PATCH v2 1/1] mm/memory-failure: don't allow to unpoison hw corrupted page

[David Hildenbrand]

On 08.06.22 10:43, zhenwei pi wrote:
> Currently unpoison_memory(unsigned long pfn) is designed for soft
> poison(hwpoison-inject) only. Since 17fae1294ad9d, the KPTE gets
> cleared on a x86 platform once hardware memory corrupts.
> 
> Unpoisoning a hardware corrupted page puts page back buddy only,
> the kernel has a chance to access the page with *NOT PRESENT* KPTE.
> This leads BUG during accessing on the corrupted KPTE.
> 
> Do not allow to unpoison hardware corrupted page in unpoison_memory() to
> avoid BUG like this:
> 
>  Unpoison: Software-unpoisoned page 0x61234
>  BUG: unable to handle page fault for address: ffff888061234000
>  #PF: supervisor write access in kernel mode
>  #PF: error_code(0x0002) - not-present page
>  PGD 2c01067 P4D 2c01067 PUD 107267063 PMD 10382b063 PTE 800fffff9edcb062
>  Oops: 0002 [#1] PREEMPT SMP NOPTI
>  CPU: 4 PID: 26551 Comm: stress Kdump: loaded Tainted: G   M       OE     5.18.0.bm.1-amd64 #7
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ...
>  RIP: 0010:clear_page_erms+0x7/0x10
>  Code: ...
>  RSP: 0000:ffffc90001107bc8 EFLAGS: 00010246
>  RAX: 0000000000000000 RBX: 0000000000000901 RCX: 0000000000001000
>  RDX: ffffea0001848d00 RSI: ffffea0001848d40 RDI: ffff888061234000
>  RBP: ffffea0001848d00 R08: 0000000000000901 R09: 0000000000001276
>  R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000001
>  R13: 0000000000000000 R14: 0000000000140dca R15: 0000000000000001
>  FS:  00007fd8b2333740(0000) GS:ffff88813fd00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: ffff888061234000 CR3: 00000001023d2005 CR4: 0000000000770ee0
>  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  PKRU: 55555554
>  Call Trace:
>   <TASK>
>   prep_new_page+0x151/0x170
>   get_page_from_freelist+0xca0/0xe20
>   ? sysvec_apic_timer_interrupt+0xab/0xc0
>   ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
>   __alloc_pages+0x17e/0x340
>   __folio_alloc+0x17/0x40
>   vma_alloc_folio+0x84/0x280
>   __handle_mm_fault+0x8d4/0xeb0
>   handle_mm_fault+0xd5/0x2a0
>   do_user_addr_fault+0x1d0/0x680
>   ? kvm_read_and_reset_apf_flags+0x3b/0x50
>   exc_page_fault+0x78/0x170
>   asm_exc_page_fault+0x27/0x30
> 
> Fixes: 847ce401df392 ("HWPOISON: Add unpoisoning support")
> Fixes: 17fae1294ad9d ("x86/{mce,mm}: Unmap the entire page if the whole page is affected and poisoned")
> Cc: Naoya Horiguchi <naoya.horiguchi@nec.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  mm/memory-failure.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index b85661cbdc4a..da99a2b7ef35 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2090,6 +2090,7 @@ int unpoison_memory(unsigned long pfn)
>  {
>  	struct page *page;
>  	struct page *p;
> +	pte_t *kpte;
>  	int ret = -EBUSY;
>  	int freeit = 0;
>  	static DEFINE_RATELIMIT_STATE(unpoison_rs, DEFAULT_RATELIMIT_INTERVAL,
> @@ -2103,6 +2104,14 @@ int unpoison_memory(unsigned long pfn)
>  
>  	mutex_lock(&mf_mutex);
>  
> +	kpte = virt_to_kpte((unsigned long)page_to_virt(p));
> 
I'm curious whether virt_to_kpte is sane to use, especially, when having
the direct map map PMDs and not PTEs?

virt_to_kpte() only checks for pmd_none() -- but what if we have
pmd_large()?

Naive me would assume that calling virt_to_kpte() from generic code is
broken. Only mm/highmem.c uses it, however, 32bit most probably also
doesn't have large mappings in the page tables for the direct map.

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 1/1] mm/memory-failure: don't allow to unpoison hw corrupted page

[Miaohe Lin]

On 2022/6/8 16:43, zhenwei pi wrote:
> Currently unpoison_memory(unsigned long pfn) is designed for soft
> poison(hwpoison-inject) only. Since 17fae1294ad9d, the KPTE gets
> cleared on a x86 platform once hardware memory corrupts.
> 
> Unpoisoning a hardware corrupted page puts page back buddy only,
> the kernel has a chance to access the page with *NOT PRESENT* KPTE.
> This leads BUG during accessing on the corrupted KPTE.
> 
> Do not allow to unpoison hardware corrupted page in unpoison_memory() to
> avoid BUG like this:
> 
>  Unpoison: Software-unpoisoned page 0x61234
>  BUG: unable to handle page fault for address: ffff888061234000
>  #PF: supervisor write access in kernel mode
>  #PF: error_code(0x0002) - not-present page
>  PGD 2c01067 P4D 2c01067 PUD 107267063 PMD 10382b063 PTE 800fffff9edcb062
>  Oops: 0002 [#1] PREEMPT SMP NOPTI
>  CPU: 4 PID: 26551 Comm: stress Kdump: loaded Tainted: G   M       OE     5.18.0.bm.1-amd64 #7
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ...
>  RIP: 0010:clear_page_erms+0x7/0x10
>  Code: ...
>  RSP: 0000:ffffc90001107bc8 EFLAGS: 00010246
>  RAX: 0000000000000000 RBX: 0000000000000901 RCX: 0000000000001000
>  RDX: ffffea0001848d00 RSI: ffffea0001848d40 RDI: ffff888061234000
>  RBP: ffffea0001848d00 R08: 0000000000000901 R09: 0000000000001276
>  R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000001
>  R13: 0000000000000000 R14: 0000000000140dca R15: 0000000000000001
>  FS:  00007fd8b2333740(0000) GS:ffff88813fd00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: ffff888061234000 CR3: 00000001023d2005 CR4: 0000000000770ee0
>  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  PKRU: 55555554
>  Call Trace:
>   <TASK>
>   prep_new_page+0x151/0x170
>   get_page_from_freelist+0xca0/0xe20
>   ? sysvec_apic_timer_interrupt+0xab/0xc0
>   ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
>   __alloc_pages+0x17e/0x340
>   __folio_alloc+0x17/0x40
>   vma_alloc_folio+0x84/0x280
>   __handle_mm_fault+0x8d4/0xeb0
>   handle_mm_fault+0xd5/0x2a0
>   do_user_addr_fault+0x1d0/0x680
>   ? kvm_read_and_reset_apf_flags+0x3b/0x50
>   exc_page_fault+0x78/0x170
>   asm_exc_page_fault+0x27/0x30
> 
> Fixes: 847ce401df392 ("HWPOISON: Add unpoisoning support")
> Fixes: 17fae1294ad9d ("x86/{mce,mm}: Unmap the entire page if the whole page is affected and poisoned")
> Cc: Naoya Horiguchi <naoya.horiguchi@nec.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  mm/memory-failure.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index b85661cbdc4a..da99a2b7ef35 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2090,6 +2090,7 @@ int unpoison_memory(unsigned long pfn)
>  {
>  	struct page *page;
>  	struct page *p;
> +	pte_t *kpte;
>  	int ret = -EBUSY;
>  	int freeit = 0;
>  	static DEFINE_RATELIMIT_STATE(unpoison_rs, DEFAULT_RATELIMIT_INTERVAL,
> @@ -2103,6 +2104,14 @@ int unpoison_memory(unsigned long pfn)
>  
>  	mutex_lock(&mf_mutex);
>  
> +	kpte = virt_to_kpte((unsigned long)page_to_virt(p));
> +	if (kpte && !pte_present(*kpte)) {

Might we need to find a common way to avoid unpoisoning the hardware corrupted page?
Other architectures would consume the hardware corrupted data if the hardware corrupted
page is unpoisoned? Or this patch is enough as a first step?

Thanks!

> +		unpoison_pr_info("Unpoison: Page was hardware poisoned %#lx\n",
> +				 pfn, &unpoison_rs);
> +		ret = -EOPNOTSUPP;
> +		goto unlock_mutex;
> +	}
> +
>  	if (!PageHWPoison(p)) {
>  		unpoison_pr_info("Unpoison: Page was already unpoisoned %#lx\n",
>  				 pfn, &unpoison_rs);
> 

Re: Re: [PATCH v2 1/1] mm/memory-failure: don't allow to unpoison hw corrupted page

[zhenwei pi]

On 6/8/22 17:30, David Hildenbrand wrote:
> On 08.06.22 10:43, zhenwei pi wrote:
>> Currently unpoison_memory(unsigned long pfn) is designed for soft
>> poison(hwpoison-inject) only. Since 17fae1294ad9d, the KPTE gets
>> cleared on a x86 platform once hardware memory corrupts.
>>
>> Unpoisoning a hardware corrupted page puts page back buddy only,
>> the kernel has a chance to access the page with *NOT PRESENT* KPTE.
>> This leads BUG during accessing on the corrupted KPTE.
>>
>> Do not allow to unpoison hardware corrupted page in unpoison_memory() to
>> avoid BUG like this:
>>
>>   Unpoison: Software-unpoisoned page 0x61234
>>   BUG: unable to handle page fault for address: ffff888061234000
>>   #PF: supervisor write access in kernel mode
>>   #PF: error_code(0x0002) - not-present page
>>   PGD 2c01067 P4D 2c01067 PUD 107267063 PMD 10382b063 PTE 800fffff9edcb062
>>   Oops: 0002 [#1] PREEMPT SMP NOPTI
>>   CPU: 4 PID: 26551 Comm: stress Kdump: loaded Tainted: G   M       OE     5.18.0.bm.1-amd64 #7
>>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ...
>>   RIP: 0010:clear_page_erms+0x7/0x10
>>   Code: ...
>>   RSP: 0000:ffffc90001107bc8 EFLAGS: 00010246
>>   RAX: 0000000000000000 RBX: 0000000000000901 RCX: 0000000000001000
>>   RDX: ffffea0001848d00 RSI: ffffea0001848d40 RDI: ffff888061234000
>>   RBP: ffffea0001848d00 R08: 0000000000000901 R09: 0000000000001276
>>   R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000001
>>   R13: 0000000000000000 R14: 0000000000140dca R15: 0000000000000001
>>   FS:  00007fd8b2333740(0000) GS:ffff88813fd00000(0000) knlGS:0000000000000000
>>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>   CR2: ffff888061234000 CR3: 00000001023d2005 CR4: 0000000000770ee0
>>   DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>   DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>   PKRU: 55555554
>>   Call Trace:
>>    <TASK>
>>    prep_new_page+0x151/0x170
>>    get_page_from_freelist+0xca0/0xe20
>>    ? sysvec_apic_timer_interrupt+0xab/0xc0
>>    ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
>>    __alloc_pages+0x17e/0x340
>>    __folio_alloc+0x17/0x40
>>    vma_alloc_folio+0x84/0x280
>>    __handle_mm_fault+0x8d4/0xeb0
>>    handle_mm_fault+0xd5/0x2a0
>>    do_user_addr_fault+0x1d0/0x680
>>    ? kvm_read_and_reset_apf_flags+0x3b/0x50
>>    exc_page_fault+0x78/0x170
>>    asm_exc_page_fault+0x27/0x30
>>
>> Fixes: 847ce401df392 ("HWPOISON: Add unpoisoning support")
>> Fixes: 17fae1294ad9d ("x86/{mce,mm}: Unmap the entire page if the whole page is affected and poisoned")
>> Cc: Naoya Horiguchi <naoya.horiguchi@nec.com>
>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>> ---
>>   mm/memory-failure.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index b85661cbdc4a..da99a2b7ef35 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -2090,6 +2090,7 @@ int unpoison_memory(unsigned long pfn)
>>   {
>>   	struct page *page;
>>   	struct page *p;
>> +	pte_t *kpte;
>>   	int ret = -EBUSY;
>>   	int freeit = 0;
>>   	static DEFINE_RATELIMIT_STATE(unpoison_rs, DEFAULT_RATELIMIT_INTERVAL,
>> @@ -2103,6 +2104,14 @@ int unpoison_memory(unsigned long pfn)
>>   
>>   	mutex_lock(&mf_mutex);
>>   
>> +	kpte = virt_to_kpte((unsigned long)page_to_virt(p));
>>
> I'm curious whether virt_to_kpte is sane to use, especially, when having
> the direct map map PMDs and not PTEs?
> 
> virt_to_kpte() only checks for pmd_none() -- but what if we have
> pmd_large()?
> 
> Naive me would assume that calling virt_to_kpte() from generic code is
> broken. Only mm/highmem.c uses it, however, 32bit most probably also
> doesn't have large mappings in the page tables for the direct map.
> 

Hi,

I dived into this part and noticed that both pmd_off_k() and 
virt_to_kpte() are broken.

For example, on a x86 platform, if the CPU has feature 'pdpe1gb', the 
kernel prefers 1G map. (cat /proc/meminfo | grep DirectMap to show the 
current mapping)

static inline pmd_t *pmd_off_k(unsigned long va)
{
         return pmd_offset(pud_offset(p4d_offset(pgd_offset_k(va), va), 
va), va);
}

There is no pud_none() & pud_large()(of cause, we can't use pud_large() 
here) to test *PUD* valid or not.

So I'm going to do:
- in pmd_off_k(), use pud_none() and pud_bad() to test *PUD*, if failed, 
BUG().
- in virt_to_kpte(), use pmd_none() & pmd_bad() to test *PMD*, if 
failed, BUG().
- rework KPTE test in unpoison_memory(), walk page table instead of 
useing virt_to_kpte().

Do you have any suggestions?

-- 
zhenwei pi

Re: [PATCH v2 1/1] mm/memory-failure: don't allow to unpoison hw corrupted page

[David Hildenbrand]

On 10.06.22 07:06, zhenwei pi wrote:
> 
> 
> On 6/8/22 17:30, David Hildenbrand wrote:
>> On 08.06.22 10:43, zhenwei pi wrote:
>>> Currently unpoison_memory(unsigned long pfn) is designed for soft
>>> poison(hwpoison-inject) only. Since 17fae1294ad9d, the KPTE gets
>>> cleared on a x86 platform once hardware memory corrupts.
>>>
>>> Unpoisoning a hardware corrupted page puts page back buddy only,
>>> the kernel has a chance to access the page with *NOT PRESENT* KPTE.
>>> This leads BUG during accessing on the corrupted KPTE.
>>>
>>> Do not allow to unpoison hardware corrupted page in unpoison_memory() to
>>> avoid BUG like this:
>>>
>>>   Unpoison: Software-unpoisoned page 0x61234
>>>   BUG: unable to handle page fault for address: ffff888061234000
>>>   #PF: supervisor write access in kernel mode
>>>   #PF: error_code(0x0002) - not-present page
>>>   PGD 2c01067 P4D 2c01067 PUD 107267063 PMD 10382b063 PTE 800fffff9edcb062
>>>   Oops: 0002 [#1] PREEMPT SMP NOPTI
>>>   CPU: 4 PID: 26551 Comm: stress Kdump: loaded Tainted: G   M       OE     5.18.0.bm.1-amd64 #7
>>>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ...
>>>   RIP: 0010:clear_page_erms+0x7/0x10
>>>   Code: ...
>>>   RSP: 0000:ffffc90001107bc8 EFLAGS: 00010246
>>>   RAX: 0000000000000000 RBX: 0000000000000901 RCX: 0000000000001000
>>>   RDX: ffffea0001848d00 RSI: ffffea0001848d40 RDI: ffff888061234000
>>>   RBP: ffffea0001848d00 R08: 0000000000000901 R09: 0000000000001276
>>>   R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000001
>>>   R13: 0000000000000000 R14: 0000000000140dca R15: 0000000000000001
>>>   FS:  00007fd8b2333740(0000) GS:ffff88813fd00000(0000) knlGS:0000000000000000
>>>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>   CR2: ffff888061234000 CR3: 00000001023d2005 CR4: 0000000000770ee0
>>>   DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>>   DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>>   PKRU: 55555554
>>>   Call Trace:
>>>    <TASK>
>>>    prep_new_page+0x151/0x170
>>>    get_page_from_freelist+0xca0/0xe20
>>>    ? sysvec_apic_timer_interrupt+0xab/0xc0
>>>    ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
>>>    __alloc_pages+0x17e/0x340
>>>    __folio_alloc+0x17/0x40
>>>    vma_alloc_folio+0x84/0x280
>>>    __handle_mm_fault+0x8d4/0xeb0
>>>    handle_mm_fault+0xd5/0x2a0
>>>    do_user_addr_fault+0x1d0/0x680
>>>    ? kvm_read_and_reset_apf_flags+0x3b/0x50
>>>    exc_page_fault+0x78/0x170
>>>    asm_exc_page_fault+0x27/0x30
>>>
>>> Fixes: 847ce401df392 ("HWPOISON: Add unpoisoning support")
>>> Fixes: 17fae1294ad9d ("x86/{mce,mm}: Unmap the entire page if the whole page is affected and poisoned")
>>> Cc: Naoya Horiguchi <naoya.horiguchi@nec.com>
>>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>>> ---
>>>   mm/memory-failure.c | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>>> index b85661cbdc4a..da99a2b7ef35 100644
>>> --- a/mm/memory-failure.c
>>> +++ b/mm/memory-failure.c
>>> @@ -2090,6 +2090,7 @@ int unpoison_memory(unsigned long pfn)
>>>   {
>>>   	struct page *page;
>>>   	struct page *p;
>>> +	pte_t *kpte;
>>>   	int ret = -EBUSY;
>>>   	int freeit = 0;
>>>   	static DEFINE_RATELIMIT_STATE(unpoison_rs, DEFAULT_RATELIMIT_INTERVAL,
>>> @@ -2103,6 +2104,14 @@ int unpoison_memory(unsigned long pfn)
>>>   
>>>   	mutex_lock(&mf_mutex);
>>>   
>>> +	kpte = virt_to_kpte((unsigned long)page_to_virt(p));
>>>
>> I'm curious whether virt_to_kpte is sane to use, especially, when having
>> the direct map map PMDs and not PTEs?
>>
>> virt_to_kpte() only checks for pmd_none() -- but what if we have
>> pmd_large()?
>>
>> Naive me would assume that calling virt_to_kpte() from generic code is
>> broken. Only mm/highmem.c uses it, however, 32bit most probably also
>> doesn't have large mappings in the page tables for the direct map.
>>
> 
> Hi,
> 
> I dived into this part and noticed that both pmd_off_k() and 
> virt_to_kpte() are broken.
> 
> For example, on a x86 platform, if the CPU has feature 'pdpe1gb', the 
> kernel prefers 1G map. (cat /proc/meminfo | grep DirectMap to show the 
> current mapping)
> 
> static inline pmd_t *pmd_off_k(unsigned long va)
> {
>          return pmd_offset(pud_offset(p4d_offset(pgd_offset_k(va), va), 
> va), va);
> }
> 
> There is no pud_none() & pud_large()(of cause, we can't use pud_large() 
> here) to test *PUD* valid or not.
> 
> So I'm going to do:
> - in pmd_off_k(), use pud_none() and pud_bad() to test *PUD*, if failed, 
> BUG().
> - in virt_to_kpte(), use pmd_none() & pmd_bad() to test *PMD*, if 
> failed, BUG().
> - rework KPTE test in unpoison_memory(), walk page table instead of 
> useing virt_to_kpte().
> 
> Do you have any suggestions?
> 

TBH, I am not convinced that this is worth the trouble.


You have to win the lottery and get a MCE use a pure debugging interface
to clear a hwpoisoned page that was not previously poisoned via that
debugging interface. There are easier ways to crash the kernel if one is
explicitly looking for trouble on a debug kernel.


Further, this looks very x86 specific to me. The whole detection is
unreliable if the arch doesn't do this unmapping, and neither your patch
subject nor the new error message holds reliably.

Even further, what if the hw error triggered memory_failure() which set
SetPageHWPoison(), but that failed? We won't be unmapping anything,
still, you'd be able to clear that because it could look like a software
poisoned page, no?

Last but not least: what if some other arch requires yet some other
handling for hw poisoned pages that we are not covering here?


Instead of seeing more widespread use of virt_to_kpte() and friends I'd
much rather see all of that "generic" only-works-on-random-32bit-arch
stuff removed.


I am not even sure if we even *need* that unpoisoning complexity just
for some debugging/testing activity.

847ce401df39 ("HWPOISON: Add unpoisoning support") mentions: "The
unpoisoning interface is useful for stress testing tools to reclaim
poisoned pages (to prevent OOM)". Why can't these tools simply reboot
once we reach a certain #hwpoisoned pages? It's certainly a lot easier
then having kernel support for something that cannot happen in "real"
environments.


So if anything, I'd vote for removing that interface+complexity, or
alternatively *disabling* it once we have a hw triggered poisoning.

-- 
Thanks,

David / dhildenb