On 4/26/24 7:53 AM, Zi Yan wrote:
Hi Zi (and Ryan)!
lockless pgtable walker could see the migration entry pmd in this state
and start interpretting the fields as if it were present, leading to
BadThings (TM). GUP-fast appears to be one such lockless pgtable walker.
Could you please explain how bad things might happen ?
See 2 places where pmdp_get_lockless() is called in gup.c, without the PTL.
These could both return the swap pte for which pmd_mkinvalid() has been called.
In both cases, this would lead to the pmd_present() check eroneously returning
true, eventually causing incorrect interpretation of the pte fields. e.g.:
gup_pmd_range()
pmd_t pmd = pmdp_get_lockless(pmdp);
gup_huge_pmd(pmd, ...)
page = nth_page(pmd_page(orig), (addr & ~PMD_MASK) >> PAGE_SHIFT);
page is guff.
Let me know what you think!
Add JohnH to check GUP code.
Ryan is correct about this behavior.
By the way, remember that gup is not the only lockless page table
walker: there is also the CPU hardware itself, which inconveniently
refuses to bother with taking page table locks. :)
So if we have code that can make a non-present PTE appear to be present
to any of these page walkers, whether software or hardware, it's a
definitely Not Good and will lead directly to bugs.
Since I had to study this patch and discussion a bit in order to
respond, I'll go ahead and also reply to the original patch with review
comments.
thanks,
--
John Hubbard
NVIDIA
