On 10/30/22 17:29, Peter Xu wrote:
> huge_pte_offset() is potentially a pgtable walker, looking up pte_t* for a
> hugetlb address.
>
> Normally, it's always safe to walk the pgtable as long as we're with the
> mmap lock held for either read or write, because that guarantees the
> pgtable pages will always be valid during the process.
>
> But it's not true for hugetlbfs: hugetlbfs has the pmd sharing feature, it
> means that even with mmap lock held, the PUD pgtable page can still go away
> from under us if pmd unsharing is possible during the walk.
>
> It's not always the case, e.g.:
>
> (1) If the mapping is private we're not prone to pmd sharing or
> unsharing, so it's okay.
>
> (2) If we're with the hugetlb vma lock held for either read/write, it's
> okay too because pmd unshare cannot happen at all.
>
> Document all these explicitly for huge_pte_offset(), because it's really
> not that obvious. This also tells all the callers on what it needs to
> guarantee huge_pte_offset() thread-safety.
>
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
> arch/arm64/mm/hugetlbpage.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 35e9a468d13e..0bf930c75d4b 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -329,6 +329,38 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma,
> return ptep;
> }
>
> +/*
> + * huge_pte_offset(): Walk the hugetlb pgtable until the last level PTE.
> + * Returns the pte_t* if found, or NULL if the address is not mapped.
> + *
> + * NOTE: since this function will walk all the pgtable pages (including not
> + * only high-level pgtable page, but also PUD that can be unshared
> + * concurrently for VM_SHARED), the caller of this function should be
> + * responsible of its thread safety. One can follow this rule:
> + *
> + * (1) For private mappings: pmd unsharing is not possible, so it'll
> + * always be safe if we're with the mmap sem for either read or write.
> + * This is normally always the case, so IOW we don't need to do
> + * anything special.
Not sure if it is worth calling out that we are safe if the process owning the
page table being walked is single threaded? Although, a pmd can be 'unshared'
due to an operation in another process, the primary is when the pmd is cleared
which only happens when the unshare is initiated by a thread of the process
owning the page tables being walked.
--
Mike Kravetz
> + *
> + * (2) For shared mappings: pmd unsharing is possible (so the PUD-ranged
> + * pgtable page can go away from under us! It can be done by a pmd
> + * unshare with a follow up munmap() on the other process), then we
> + * need either:
> + *
> + * (2.1) hugetlb vma lock read or write held, to make sure pmd unshare
> + * won't happen upon the range (it also makes sure the pte_t we
> + * read is the right and stable one), or,
> + *
> + * (2.2) RCU read lock, to make sure even pmd unsharing happened, the
> + * old shared PUD page won't get freed from under us, so even of
> + * the pteval can be obsolete, at least it's still always safe to
> + * access the pgtable page (e.g., de-referencing pte_t* would not
> + * cause use-after-free).
> + *
> + * PS: from the regard of (2.2), it's the same logic of fast-gup being safe
> + * for generic mm, as long as RCU is used to free any pgtable page.
> + */
> pte_t *huge_pte_offset(struct mm_struct *mm,
> unsigned long addr, unsigned long sz)
> {
> --
> 2.37.3
>
