Thread (63 messages) 63 messages, 5 authors, 19h ago

Re: [PATCH 12/13] mm/mprotect: convert mprotect code to use vma_flags_t

From: Lorenzo Stoakes <ljs@kernel.org>
Date: 2026-07-11 17:04:43
Also in: dri-devel, intel-gfx, intel-xe, linux-arm-msm, linux-fbdev, linux-fsdevel, linux-mips, linux-mm, linux-samsung-soc, linux-sound, linux-tegra, lkml, nouveau, virtualization, xen-devel

On Wed, Jul 08, 2026 at 10:16:15PM -0400, Zi Yan wrote:
On Thu Jul 2, 2026 at 11:53 AM EDT, Lorenzo Stoakes wrote:
quoted
On Thu, Jul 02, 2026 at 12:09:17AM +0800, Lance Yang wrote:
quoted
On Mon, Jun 29, 2026 at 08:25:35PM +0100, Lorenzo Stoakes wrote:
quoted
Replace use of the legacy vm_flags_t flags with vma_flags_t values
throughout the mprotect logic.

Note that we retain the legacy vm_flags_t bit shifting code in
do_mprotect_key(), deferring a vma_flags_t approach to this for the time
being.

Additionally update comments to reflect the changes to be consistent.

No functional change intended.

Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
---
mm/mprotect.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 9cbf932b028c..c9504b2a2525 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -40,7 +40,7 @@
static bool maybe_change_pte_writable(struct vm_area_struct *vma, pte_t pte)
{
-	if (WARN_ON_ONCE(!(vma->vm_flags & VM_WRITE)))
+	if (WARN_ON_ONCE(!vma_test(vma, VMA_WRITE_BIT)))
		return false;

	/* Don't touch entries that are not even readable. */
@@ -97,7 +97,7 @@ static bool can_change_shared_pte_writable(struct vm_area_struct *vma,
bool can_change_pte_writable(struct vm_area_struct *vma, unsigned long addr,
			     pte_t pte)
{
-	if (!(vma->vm_flags & VM_SHARED))
+	if (!vma_test(vma, VMA_SHARED_BIT))
		return can_change_private_pte_writable(vma, addr, pte);

	return can_change_shared_pte_writable(vma, pte);
@@ -194,7 +194,7 @@ static __always_inline void set_write_prot_commit_flush_ptes(struct vm_area_stru
{
	bool set_write;

-	if (vma->vm_flags & VM_SHARED) {
+	if (vma_test(vma, VMA_SHARED_BIT)) {
		set_write = can_change_shared_pte_writable(vma, ptent);
		prot_commit_flush_ptes(vma, addr, ptep, oldpte, ptent, nr_ptes,
				       /* idx = */ 0, set_write, tlb);
@@ -811,8 +811,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
		vm_unacct_memory(nrpages);

	/*
-	 * Private VM_LOCKED VMA becoming writable: trigger COW to avoid major
-	 * fault on access.
+	 * Private VMA_LOCKED_BIT VMA becoming writable: trigger COW to avoid
+	 * major fault on access.
	 */
	if (vma_flags_test(&new_vma_flags, VMA_WRITE_BIT) &&
	    vma_flags_test(&old_vma_flags, VMA_LOCKED_BIT) &&
@@ -886,7 +886,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
			goto out;
		start = vma->vm_start;
		error = -EINVAL;
-		if (!(vma->vm_flags & VM_GROWSDOWN))
+		if (!vma_test(vma, VMA_GROWSDOWN_BIT))
			goto out;
	} else {
		if (vma->vm_start > start)
@@ -894,7 +894,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
		if (unlikely(grows & PROT_GROWSUP)) {
			end = vma->vm_end;
			error = -EINVAL;
-			if (!(vma->vm_flags & VM_GROWSUP))
+			if (!vma_test(vma, VMA_GROWSUP_BIT))
IIUC, should this be

if (!vma_test_single_mask(vma, VMA_GROWSUP))

instead?

#elif defined(CONFIG_PARISC)
#define VM_GROWSUP	INIT_VM_FLAG(GROWSUP)
...
#ifndef VM_GROWSUP
#define VM_GROWSUP	VM_NONE
...

VM_GROWSUP is only defined as GROWSUP on parisc and becomes VM_NONE
elsewhere. But VMA_GROWSUP_BIT is the raw ARCH_1 bit, which is also used
for other arch-specific VMA flags:

	DECLARE_VMA_BIT_ALIAS(SAO, ARCH_1),		/* Strong Access Ordering (powerpc) */
	DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1),		/* parisc */
	DECLARE_VMA_BIT_ALIAS(SPARC_ADI, ARCH_1),	/* sparc64 */
	DECLARE_VMA_BIT_ALIAS(ARM64_BTI, ARCH_1),	/* arm64 */
	DECLARE_VMA_BIT_ALIAS(ARCH_CLEAR, ARCH_1),	/* sparc64, arm64 */
	DECLARE_VMA_BIT_ALIAS(MAPPED_COPY, ARCH_1),	/* !CONFIG_MMU */

Other vma_test() changes look fine to me: just fixed INIT_VM_FLAG()
masks matching their VMA_*_BIT :)
Thanks you're right, will fix!

Again I swear I ran claude on all of this so it's failing me here :)
Is it better to add something like below to avoid misuse these mutually
exclusive bit aliases?

An example for VMA_GROWSUP_BIT:

#if defined(CONFIG_PARISC)
DECLARE_VMA_BIT_ALIAS(GROWSUP, ARCH_1),		/* parisc */
#else
/* make VMA_GROWSUP_BIT a build bug on */
#endif
Nah we can't...
Hmm, these VMA_*_BIT are enum items, so the above might not be possible.
...because of this but also this is the canonical definition and we want to use
it for the VMA_xxx form.
An alternative is to only define them for the corresponding config and
you will get build errors when trying to use them directly and the
config is not enabled. Otherwise, misuses like "vma_test(vma,
VMA_GROWSUP_BIT)" is harder to uncover.
Yeah good idea, I may add a new DECLARE_INTERNAL_VMA_BIT() that gives you
__VMA_xxx_BIT, so nobody makes the obvious mistake and we force uses of safe
VMA_xxx form. Hmm :)

--
Best Regards,
Yan, Zi
Thanks, Lorenzo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help