2019-09-23 04:27:53

by Qiujun Huang

[permalink] [raw]
Subject: [PATCH 3/3] mm:fix gup_pud_range

__get_user_pages_fast try to walk the page table but the
hugepage pte is replace by hwpoison swap entry by mca path.
...
[15798.177437] mce: Uncorrected hardware memory error in
user-access at 224f1761c0
[15798.180171] MCE 0x224f176: Killing pal_main:6784 due to
hardware memory corruption
[15798.180176] MCE 0x224f176: Killing qemu-system-x86:167336
due to hardware memory corruption
...
[15798.180206] BUG: unable to handle kernel
[15798.180226] paging request at ffff891200003000
[15798.180236] IP: [<ffffffff8106edae>] gup_pud_range+
0x13e/0x1e0
...

We need to skip the hwpoison entry in gup_pud_range.

Signed-off-by: Qiujun Huang <[email protected]>
---
mm/gup.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index 98f13ab..6157ed9 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2230,6 +2230,8 @@ static int gup_pud_range(p4d_t p4d, unsigned long addr, unsigned long end,
next = pud_addr_end(addr, end);
if (pud_none(pud))
return 0;
+ if (unlikely(!pud_present(pud)))
+ return 0;
if (unlikely(pud_huge(pud))) {
if (!gup_huge_pud(pud, pudp, addr, next, flags,
pages, nr))
--
1.8.3.1


2019-09-23 13:37:01

by Qiujun Huang

[permalink] [raw]
Subject: Re: [PATCH 3/3] mm:fix gup_pud_range

On Sat, Sep 21, 2019 at 9:19 AM John Hubbard <[email protected]> wrote:
>
> On 9/20/19 5:33 PM, Qiujun Huang wrote:
> >> On 9/20/19 8:51 AM, Qiujun Huang wrote:
> ...
> >> It would be nice if this spelled out a little more clearly what's
> >> wrong. I think you and Aneesh are saying that the entry is really
> >> a swap entry, created by the MCE response to a bad page?
> > do_machine_check->
> > do_memory_failure->
> > memory_failure->
> > hwpoison_user_mappings
> > will updated PUD level PTE entry as a swap entry.
> >
> > static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
> > unsigned long address, void *arg)
> > {
> > ...
> > if (PageHWPoison(page) && !(flags & TTU_IGNORE_HWPOISON)) {
> > pteval = swp_entry_to_pte(make_hwpoison_entry(subpage));
>
> OK, that helps. Let's add something approximately like this to the
> commit description:
>
> do_machine_check()
> do_memory_failure()
> memory_failure()
> hw_poison_user_mappings()
> try_to_unmap()
> pteval = swp_entry_to_pte(make_hwpoison_entry(subpage));
>
> ...and now we have a swap entry that indicates that the page entry
> refers to a bad (and poisoned) page of memory, but gup_fast() at this
> level of the page table was ignoring swap entries, and incorrectly
> assuming that "!pxd_none() == valid and present".
>
> And this was not just a poisoned page problem, but a generaly swap entry
> problem. So, any swap entry type (device memory migration, numa migration,
> or just regular swapping) could lead to the same problem.
>
> Fix this by checking for pxd_present(), instead of pxd_none().
>
>
> > ...
> >>
> >>>
> >>> Signed-off-by: Qiujun Huang <[email protected]>
> >>> ---
> >>> mm/gup.c | 2 ++
> >>> 1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/mm/gup.c b/mm/gup.c
> >>> index 98f13ab..6157ed9 100644
> >>> --- a/mm/gup.c
> >>> +++ b/mm/gup.c
> >>> @@ -2230,6 +2230,8 @@ static int gup_pud_range(p4d_t p4d, unsigned long addr, unsigned long end,
> >>> next = pud_addr_end(addr, end);
> >>> if (pud_none(pud))
> >>> return 0;
> >>> + if (unlikely(!pud_present(pud)))
> >>> + return 0;
> >>
> >> If the MCE hwpoison behavior puts in swap entries, then it seems like all
> >> page table walkers would need to check for p*d_present(), and maybe at all
> >> levels too, right?
> > I think so
> >>
>
> Should those changes be part of this fix, do you think?

Yes, please.Thanks
>
> thanks,
> --
> John Hubbard
> NVIDIA