lkml.org 
[lkml]   [2022]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] mm/hugetlb: avoid corrupting page->mapping in hugetlb_mcopy_atomic_pte
    On Wed, Jul 13, 2022 at 4:36 PM Mike Kravetz <mike.kravetz@oracle.com> wrote:
    >
    > On 07/13/22 15:46, Axel Rasmussen wrote:
    > > I think there is a small mistake in this patch.
    > >
    > > Consider the non-minor-fault case. We have this block:
    > >
    > > /* Add shared, newly allocated pages to the page cache. */
    > > if (vm_shared && !is_continue) {
    > > /* ... */
    > > }
    > >
    > > In here, we've added the newly allocated page to the page cache, and
    > > we've set this page_in_pagecache flag to true. But we *do not* setup
    > > rmap for this page in this block. I think in this case, the patch will
    > > cause us to do the wrong thing: we should hugepage_add_new_anon_rmap()
    > > further down, but with this patch we dup instead.
    >
    > I am not sure I follow. The patch from Miaohe Lin would not change any
    > behavior in the 'if (vm_shared && !is_continue)' case. In this case
    > both vm_shared and page_in_pagecache are true.
    >
    > IIUC, the patch would address the case where !vm_shared && is_continue.

    Ah, you're right, my interpretation of the various flags got mixed up
    somewhere along the way.

    page_in_pagecache is equivalent to vm_shared in this function,
    *except* when we have is_continue. Given that, I think this patch is
    correct in the vm_shared case (no behavior change). In case of
    !vm_shared && is_continue, I agree the patch is a correction to the
    previous behavior.

    >
    > On 07/12/22 21:05, Miaohe Lin wrote:
    > > In MCOPY_ATOMIC_CONTINUE case with a non-shared VMA, pages in the page
    > > cache are installed in the ptes. But hugepage_add_new_anon_rmap is called
    > > for them mistakenly because they're not vm_shared. This will corrupt the
    > > page->mapping used by page cache code.
    > >
    > > Fixes: f619147104c8 ("userfaultfd: add UFFDIO_CONTINUE ioctl")
    > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
    > > ---
    > > mm/hugetlb.c | 2 +-
    > > 1 file changed, 1 insertion(+), 1 deletion(-)
    > >
    > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
    > > index 8d379e03f672..b232e1508e49 100644
    > > --- a/mm/hugetlb.c
    > > +++ b/mm/hugetlb.c
    > > @@ -6038,7 +6038,7 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,
    > > if (!huge_pte_none_mostly(huge_ptep_get(dst_pte)))
    > > goto out_release_unlock;
    > >
    > > - if (vm_shared) {
    > > + if (page_in_pagecache) {
    > > page_dup_file_rmap(page, true);
    > > } else {
    > > ClearHPageRestoreReserve(page);
    >
    > --
    > Mike Kravetz

    \
     
     \ /
      Last update: 2022-07-14 02:21    [W:2.492 / U:0.800 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site