• Andrea Arcangeli's avatar
    mm: thp: fix MADV_DONTNEED vs migrate_misplaced_transhuge_page race condition · 4291e97c
    Andrea Arcangeli authored
    [ Upstream commit d7c33934 ]
    
    Patch series "migrate_misplaced_transhuge_page race conditions".
    
    Aaron found a new instance of the THP MADV_DONTNEED race against
    pmdp_clear_flush* variants, that was apparently left unfixed.
    
    While looking into the race found by Aaron, I may have found two more
    issues in migrate_misplaced_transhuge_page.
    
    These race conditions would not cause kernel instability, but they'd
    corrupt userland data or leave data non zero after MADV_DONTNEED.
    
    I did only minor testing, and I don't expect to be able to reproduce this
    (especially the lack of ->invalidate_range before migrate_page_copy,
    requires the latest iommu hardware or infiniband to reproduce).  The last
    patch is noop for x86 and it needs further review from maintainers of
    archs that implement flush_cache_range() (not in CC yet).
    
    To avoid confusion, it's not the first patch that introduces the bug fixed
    in the second patch, even before removing the
    pmdp_huge_clear_flush_notify, that _notify suffix was called after
    migrate_page_copy already run.
    
    This patch (of 3):
    
    This is a corollary of ced10803 ("thp: fix MADV_DONTNEED vs.  numa
    balancing race"), 58ceeb6b ("thp: fix MADV_DONTNEED vs.  MADV_FREE
    race") and 5b7abeae ("thp: fix MADV_DONTNEED vs clear soft dirty
    race).
    
    When the above three fixes where posted Dave asked
    https://lkml.kernel.org/r/929b3844-aec2-0111-fef7-8002f9d4e2b9@intel.com
    but apparently this was missed.
    
    The pmdp_clear_flush* in migrate_misplaced_transhuge_page() was introduced
    in a54a407f ("mm: Close races between THP migration and PMD numa
    clearing").
    
    The important part of such commit is only the part where the page lock is
    not released until the first do_huge_pmd_numa_page() finished disarming
    the pagenuma/protnone.
    
    The addition of pmdp_clear_flush() wasn't beneficial to such commit and
    there's no commentary about such an addition either.
    
    I guess the pmdp_clear_flush() in such commit was added just in case for
    safety, but it ended up introducing the MADV_DONTNEED race condition found
    by Aaron.
    
    At that point in time nobody thought of such kind of MADV_DONTNEED race
    conditions yet (they were fixed later) so the code may have looked more
    robust by adding the pmdp_clear_flush().
    
    This specific race condition won't destabilize the kernel, but it can
    confuse userland because after MADV_DONTNEED the memory won't be zeroed
    out.
    
    This also optimizes the code and removes a superfluous TLB flush.
    
    [akpm@linux-foundation.org: reflow comment to 80 cols, fix grammar and typo (beacuse)]
    Link: http://lkml.kernel.org/r/20181013002430.698-2-aarcange@redhat.comSigned-off-by: default avatarAndrea Arcangeli <aarcange@redhat.com>
    Reported-by: default avatarAaron Tomlin <atomlin@redhat.com>
    Acked-by: default avatarMel Gorman <mgorman@suse.de>
    Acked-by: default avatarKirill A. Shutemov <kirill.shutemov@linux.intel.com>
    Cc: Jerome Glisse <jglisse@redhat.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
    4291e97c
migrate.c 76.9 KB