linux-kernel - Re: [PATCH -next 1/1] mm: hugetlb_vmemmap: Fix WARN_ON in vmemmap_remap

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-prev] [day] [month] [year] [list]

Date:   Tue, 1 Nov 2022 09:56:01 +0000
From:   Catalin Marinas <catalin.marinas@....com>
To:     mawupeng <mawupeng1@...wei.com>
Cc:     muchun.song@...ux.dev, songmuchun@...edance.com,
        anshuman.khandual@....com, akpm@...ux-foundation.org,
        mike.kravetz@...cle.com, mhocko@...e.com, osalvador@...e.de,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -next 1/1] mm: hugetlb_vmemmap: Fix WARN_ON in
 vmemmap_remap_pte

On Sat, Oct 29, 2022 at 09:55:15AM +0800, mawupeng wrote:
> On 2022/10/26 11:01, mawupeng wrote:
> > On 2022/10/25 14:36, Muchun Song wrote:
> >>> On Oct 25, 2022, at 09:42, Wupeng Ma <mawupeng1@...wei.com> wrote:
> >>>
> >>> From: Ma Wupeng <mawupeng1@...wei.com>
> >>>
> >>> Commit f41f2ed43ca5 ("mm: hugetlb: free the vmemmap pages associated with
> >>> each HugeTLB page") add vmemmap_remap_pte to remap the tail pages as
> >>> read-only to catch illegal write operation to the tail page.
> >>>
> >>> However this will lead to WARN_ON in arm64 in __check_racy_pte_update()
> >>
> >> Thanks for your finding this issue.
> >>
> >>> since this may lead to dirty state cleaned. This check is introduced by
> >>> commit 2f4b829c625e ("arm64: Add support for hardware updates of the
> >>> access and dirty pte bits") and the initial check is as follow:
> >>>
> >>>  BUG_ON(pte_write(*ptep) && !pte_dirty(pte));
> >>>
> >>> Since we do need to mark this pte as read-only to catch illegal write
> >>> operation to the tail pages, use set_pte  to replace set_pte_at to bypass
> >>> this check.
[...]
> IMHO, arm64 or other archs do some work on the dirty bit and rdonly bit in
> pte in commit 2f4b829c625e ("arm64: Add support for hardware updates of the
> access and dirty pte bits").
> 
> Maybe we can use pte_wrprotect() to mark this pte read-only? It will add 
> PTE_DIRTY bit for the new pte entry compare to the old one.
> 
> Here is the diff:
> 
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index ba2a2596fb4e..24a230895316 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -244,8 +244,7 @@ static void vmemmap_remap_pte(pte_t *pte, unsigned long addr,
>          * Remap the tail pages as read-only to catch illegal write operation
>          * to the tail pages.
>          */
> -       pgprot_t pgprot = PAGE_KERNEL_RO;
> -       pte_t entry = mk_pte(walk->reuse_page, pgprot);
> +       pte_t entry = pte_wrprotect(mk_pte(walk->reuse_page, PAGE_KERNEL));

This may silence the warning but we plan to add another to detect a
change in the pfn without going through a break-before-make sequence.

-- 
Catalin