Messages in this thread | | | Subject | Re: [PATCH] Revert "mm/gup: check page posion status for coredump." | From | David Hildenbrand <> | Date | Wed, 5 May 2021 15:55:42 +0200 |
| |
On 05.05.21 15:54, Michal Hocko wrote: > From: Michal Hocko <mhocko@suse.com> > > While reviewing http://lkml.kernel.org/r/20210429122519.15183-4-david@redhat.com > I have crossed d3378e86d182 ("mm/gup: check page posion status for > coredump.") and noticed that this patch is broken in two ways. First it > doesn't really prevent hwpoison pages from being dumped because hwpoison > pages can be marked asynchornously at any time after the check. > Secondly, and more importantly, the patch introduces a ref count leak > because get_dump_page takes a reference on the page which is not > releases. > > It also seems that the patch was merged incorrectly because there were > follow up changes not included as well as discussions on how to address > the underlying problem http://lkml.kernel.org/r/57ac524c-b49a-99ec-c1e4-ef5027bfb61b@redhat.com > > Therefore revert the original patch. > > Fixes: d3378e86d182 ("mm/gup: check page posion status for coredump.") > Signed-off-by: Michal Hocko <mhocko@suse.com> > --- > mm/gup.c | 4 ---- > mm/internal.h | 20 -------------------- > 2 files changed, 24 deletions(-) > > diff --git a/mm/gup.c b/mm/gup.c > index 71e546e279fc..a33abe9048ed 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -1592,10 +1592,6 @@ struct page *get_dump_page(unsigned long addr) > FOLL_FORCE | FOLL_DUMP | FOLL_GET); > if (locked) > mmap_read_unlock(mm); > - > - if (ret == 1 && is_page_poisoned(page)) > - return NULL; > - > return (ret == 1) ? page : NULL; > } > #endif /* CONFIG_ELF_CORE */ > diff --git a/mm/internal.h b/mm/internal.h > index ef5f336f59bd..43c4a2f8d4cc 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -96,26 +96,6 @@ static inline void set_page_refcounted(struct page *page) > set_page_count(page, 1); > } > > -/* > - * When kernel touch the user page, the user page may be have been marked > - * poison but still mapped in user space, if without this page, the kernel > - * can guarantee the data integrity and operation success, the kernel is > - * better to check the posion status and avoid touching it, be good not to > - * panic, coredump for process fatal signal is a sample case matching this > - * scenario. Or if kernel can't guarantee the data integrity, it's better > - * not to call this function, let kernel touch the poison page and get to > - * panic. > - */ > -static inline bool is_page_poisoned(struct page *page) > -{ > - if (PageHWPoison(page)) > - return true; > - else if (PageHuge(page) && PageHWPoison(compound_head(page))) > - return true; > - > - return false; > -} > - > extern unsigned long highest_memmap_pfn; > > /* >
Reviewed-by: David Hildenbrand <david@redhat.com>
-- Thanks,
David / dhildenb
| |