Thread (81 messages) 81 messages, 3 authors, 2d ago

Re: [PATCH v5 24/38] mm/vma: enforce that mm-managed mappings may not set VMA_IO_BIT

flat view

From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
Date: 2026-10-12 18:49:21
Also in: bpf, dri-devel, fuse-devel, kvm, kvm-riscv, kvmarm, linux-arch, linux-doc, linux-fbdev, linux-fsdevel, linux-mm, linux-perf-users, linux-rdma, linux-riscv, linux-s390, linux-scsi, linux-sound, linux-trace-kernel, linux-usb, lkml, selinux, sparclinux

On Mon, Oct 12, 2026 at 08:39:35PM +0200, David Hildenbrand (Arm) wrote:
On 10/12/26 20:35, Lorenzo Stoakes (ARM) wrote:
quoted
On Mon, Oct 12, 2026 at 08:25:54PM +0200, David Hildenbrand (Arm) wrote:
quoted
On 10/9/26 20:04, Lorenzo Stoakes (ARM) wrote:
quoted
It makes no sense for a mapping whose contents the kernel does not own to
specify that the range is MMIO.

Prior to this patch, all in-tree drivers which did so have been updated
such that they are marked as not being mm-managed. The check WARNs and
fails the mmap for any out-of-tree driver that still sets VMA_IO_BIT
without a kernel mapping.

No functional change intended for in-tree code.

Reviewed-by: Zi Yan <ziy@nvidia.com>
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/vma.c | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/mm/vma.c b/mm/vma.c
index 4f09fb98a2fd..51bdbe08bb02 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2796,6 +2796,12 @@ static int mmap_validate_vma_flags(const vma_flags_t *flags)
 		return -EINVAL;
 #endif

+	if (!vma_flags_test_any(flags, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT)) {
+		/* mm-managed mappings may not set VMA_IO_BIT. */
+		if (WARN_ON_ONCE(vma_flags_test(flags, VMA_IO_BIT)))
+			return -EINVAL;
We do have a helper (vma_flags_is_mm_managed()) but we're not using it because
of the VM_IO situation ...
That gets removed.
quoted
That's confusing ...

Could we do something like testing vma_flags_is_mm_managed(), but removing the
VM_IO flag from the checked flags?
I mean we do that in a subsequent commit, I think even the next one?

quoted
Then we can add a comment about VM_IO in the future getting remove from
vma_flags_is_mm_managed() ...
Yeah, but to do that the code would now be something like:

	vma_flags_t masked = flags;

	/*
	  * vma_flags_is_mm_managed() tests VMA_IO_BIT, which will be removed
	 * in the next commit.
	 */
	vma_flags_clear(&flags, VMA_IO_BIT);
	if (!vma_flags_is_mm_managed(masked)) {
		...
	}

Is that really worth it?

I don't think there's really too much confusion intra-patch about that is there?
I would do it as above (- the comment, as that is the next patch indeed) and
only remove the vma_flags_clear() in the next patch.

But in any case, you should spell out that this is only temporary in the patch
description :)
And then there's a 'masked' variable that is unused?

You need to have two vma_flags_t variables to be able both clear VMA_IO_BIT
and use vma_flags_is_mm_managed().

So the change would have to be

+ 	vma_flags_t masked = *flags;
+
+ 	vma_flags_clear(&flags, VMA_IO_BIT);
+ 	if (!vma_flags_is_mm_managed(&masked)) {
+ 		...
+ 	}

Then:

- 	vma_flags_t masked = *flags;
-
- 	vma_flags_clear(&flags, VMA_IO_BIT);
- 	if (!vma_flags_is_mm_managed(&masked)) {
+ 	if (!vma_flags_is_mm_managed(flags)) {
 		...
 	}

Which doesn't really seem worth it for somebody who is confused between two
patches mid-series.

But if you feel strongly I can make the change like that?

--
Cheers, 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