linux-kernel - [PATCH 0/4] mm: Fix pmd_trans_unstable() call sites on retry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri,  2 Jun 2023 19:05:48 -0400
From:   Peter Xu <peterx@...hat.com>
To:     linux-kernel@...r.kernel.org, linux-mm@...ck.org
Cc:     David Hildenbrand <david@...hat.com>,
        Alistair Popple <apopple@...dia.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        "Kirill A . Shutemov" <kirill@...temov.name>,
        Johannes Weiner <hannes@...xchg.org>,
        John Hubbard <jhubbard@...dia.com>,
        Naoya Horiguchi <naoya.horiguchi@....com>, peterx@...hat.com,
        Muhammad Usama Anjum <usama.anjum@...labora.com>,
        Hugh Dickins <hughd@...gle.com>,
        Mike Rapoport <rppt@...nel.org>
Subject: [PATCH 0/4] mm: Fix pmd_trans_unstable() call sites on retry

When hit pmd_trans_unstable() under mmap read lock, it means we raced with
something else.  Per the comment above the helper, we can definitely treat
it as some pmd (none?) but the 100% correct way is always retry, and I
don't think it should race again in most cases.

Not taking care of that retry can mean different things on different
paths.

For example, for smaps it means inaccurate accountings when we skip those
raced regions, but it's fine anyway because the accounting is not for 100%
accurate.

I think it's broken for pagemap OTOH, because we have the pagemap buffer
linear to the VA we're scanning, it means if we skip some region the follow
up scans can fill in the wrong slots, I think.  It means the pagemap
results returned to userapp will be wrong when very unlucky.

This reminded me that I should have a look at all call sites of
pmd_trans_unstable(), some of them are alright but I do see many of them
may still be better to give another shot when hit.

This series tries to resolve all call sites for it on that retry attempt.

I really don't know whether I missed something, even if not, whether it
matters a lot to anyone.  Still, _if_ I'm correct may worth consider
fixing.  Happy to be prove wrong.  Then Muhammad should know how to code
his.

The patchset is only smoke tested, nothing wrong I see.

Please have a look, thanks.

Peter Xu (4):
  mm/mprotect: Retry on pmd_trans_unstable()
  mm/migrate: Unify and retry an unstable pmd when hit
  mm: Warn for unstable pmd in move_page_tables()
  mm: Make most walk page paths with pmd_trans_unstable() to retry

 fs/proc/task_mmu.c  | 17 +++++++++++++----
 mm/madvise.c        |  8 ++++++--
 mm/memcontrol.c     |  8 ++++++--
 mm/memory-failure.c |  4 +++-
 mm/mempolicy.c      |  4 +++-
 mm/migrate_device.c |  9 ++++-----
 mm/mprotect.c       | 20 +++++++++++---------
 mm/mremap.c         |  4 ++--
 8 files changed, 48 insertions(+), 26 deletions(-)

-- 
2.40.1