2022-06-08 09:52:54

by zhenwei pi

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

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 <[email protected]>
Signed-off-by: zhenwei pi <[email protected]>
---
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


2022-06-08 10:30:14

by David Hildenbrand

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

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 <[email protected]>
> Signed-off-by: zhenwei pi <[email protected]>
> ---
> 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

2022-06-10 05:50:40

by zhenwei pi

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



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 <[email protected]>
>> Signed-off-by: zhenwei pi <[email protected]>
>> ---
>> 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

2022-06-10 09:37:10

by David Hildenbrand

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

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 <[email protected]>
>>> Signed-off-by: zhenwei pi <[email protected]>
>>> ---
>>> 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