linux-kernel - Re: [PATCH 04/47] LoongArch: Set _PAGE_DIRTY only if _PAGE_WRITE is set in {pmd,pte}

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 17 Nov 2022 21:59:49 +0800
From:   Xi Ruoyao <xry111@...111.site>
To:     Huacai Chen <chenhuacai@...ngson.cn>,
        Huacai Chen <chenhuacai@...nel.org>
Cc:     loongarch@...ts.linux.dev, Xuefeng Li <lixuefeng@...ngson.cn>,
        Guo Ren <guoren@...nel.org>, Xuerui Wang <kernel@...0n.name>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        "David S . Miller" <davem@...emloft.net>,
        sparclinux@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org, Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH 04/47] LoongArch: Set _PAGE_DIRTY only if _PAGE_WRITE is
 set in {pmd,pte}_mkdirty()

Hi Huacai,

On Thu, 2022-11-17 at 12:25 +0800, Huacai Chen wrote:
> Now {pmd,pte}_mkdirty() set _PAGE_DIRTY bit unconditionally, this causes
> random segmentation fault after commit 0ccf7f168e17bb7e ("mm/thp: carry
> over dirty bit when thp splits on pmd").

Hmm, the pte_mkdirty call is already removed in commit 624a2c94f5b7a081
("Partly revert \"mm/thp: carry over dirty bit when thp splits on
pmd\"").

Not sure if this issue is related to some random segfaults I've observed
recently though.  My last kernel build contains 0ccf7f168e17bb7e but
does not contain 624a2c94f5b7a081.

> 
> The reason is: when fork(), parent process use pmd_wrprotect() to clear
> huge page's _PAGE_WRITE and _PAGE_DIRTY (for COW); then pte_mkdirty() set
> _PAGE_DIRTY as well as _PAGE_MODIFIED while splitting dirty huge pages;
> once _PAGE_DIRTY is set, there will be no tlb modify exception so the COW
> machanism fails; and at last memory corruption occurred between parent
> and child processes.
> 
> So, we should set _PAGE_DIRTY only when _PAGE_WRITE is set in {pmd,pte}_
> mkdirty().
> 
> Cc: stable@...r.kernel.org
> Cc: Peter Xu <peterx@...hat.com>
> Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
> ---
> Note: CC sparc maillist because they have similar issues.
>  
>  arch/loongarch/include/asm/pgtable.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
> index 946704bee599..debbe116f105 100644
> --- a/arch/loongarch/include/asm/pgtable.h
> +++ b/arch/loongarch/include/asm/pgtable.h
> @@ -349,7 +349,9 @@ static inline pte_t pte_mkclean(pte_t pte)
>  
>  static inline pte_t pte_mkdirty(pte_t pte)
>  {
> -       pte_val(pte) |= (_PAGE_DIRTY | _PAGE_MODIFIED);
> +       pte_val(pte) |= _PAGE_MODIFIED;
> +       if (pte_val(pte) & _PAGE_WRITE)
> +               pte_val(pte) |= _PAGE_DIRTY;
>         return pte;
>  }
>  
> @@ -478,7 +480,9 @@ static inline pmd_t pmd_mkclean(pmd_t pmd)
>  
>  static inline pmd_t pmd_mkdirty(pmd_t pmd)
>  {
> -       pmd_val(pmd) |= (_PAGE_DIRTY | _PAGE_MODIFIED);
> +       pmd_val(pmd) |= _PAGE_MODIFIED;
> +       if (pmd_val(pmd) & _PAGE_WRITE)
> +               pmd_val(pmd) |= _PAGE_DIRTY;
>         return pmd;
>  }
>  

-- 
Xi Ruoyao <xry111@...111.site>
School of Aerospace Science and Technology, Xidian University