linux-kernel - Re: [RESEND PATCH v3 4/5] mm/mprotect: do not flush on permission promotion

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

Message-Id: <0FFB7FA7-4F08-47EB-8AAE-400867E96DE4@gmail.com>
Date:   Fri, 11 Mar 2022 14:45:57 -0800
From:   Nadav Amit <nadav.amit@...il.com>
To:     linux-mm@...ck.org
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <will@...nel.org>, Yu Zhao <yuzhao@...gle.com>,
        Nick Piggin <npiggin@...il.com>, x86@...nel.org
Subject: Re: [RESEND PATCH v3 4/5] mm/mprotect: do not flush on permission
 promotion


> On Mar 11, 2022, at 11:07 AM, Nadav Amit <nadav.amit@...il.com> wrote:
> 
> From: Nadav Amit <namit@...are.com>
> 
> Currently, using mprotect() to unprotect a memory region or uffd to
> unprotect a memory region causes a TLB flush. At least on x86, as
> protection is promoted, no TLB flush is needed.
> 
> Add an arch-specific pte_may_need_flush() which tells whether a TLB
> flush is needed based on the old PTE and the new one. Implement an x86
> pte_may_need_flush().
> 
> For x86, besides the simple logic that PTE protection promotion or
> changes of software bits does require a flush, also add logic that
> considers the dirty-bit. If the dirty-bit is clear and write-protect is
> set, no TLB flush is needed, as x86 updates the dirty-bit atomically
> on write, and if the bit is clear, the PTE is reread.
> 
> 

[snip]

> + */
> +static inline bool pte_needs_flush(pte_t oldpte, pte_t newpte)
> +{
> +	/* !PRESENT -> * ; no need for flush */
> +	if (!pte_present(oldpte))
> +		return false;
> +
> +	/* PRESENT -> !PRESENT ; needs flush */
> +	if (!pte_present(newpte))
> +		return true;
> +
> +	/* PFN changed ; needs flush */
> +	if (pte_pfn(oldpte) != pte_pfn(newpte))
> +		return true;
> +
> +	return pte_flags_need_flush(pte_flags(oldpte), pte_flags(newpte),
> +				    _PAGE_ACCESSED);
> +}


Looking again at this patch, I think the PRESENT -> !PRESENT is
not needed:

1. It might trigger unnecessary flushes for PROT_NONE (or NUMA)
2. Real PRESENT -> !PRESENT is already checked by
   pte_flags_need_flush().


Let me know if I am missing something. Otherwise I will change it
for v4.