Re: [PATCH 24/39] mm/vma: enforce that only kernel-owned mappings may set VMA_IO_BIT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Denial of Service

Pre-existing issues:
- [High] Memory Leak
- [High] Use-After-Free / Double Free
--

commit 12d56e2f89cf46c2631f51c17242ae01f2acd00b
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

mm/vma: enforce that only kernel-owned mappings may set VMA_IO_BIT

This patch updates mmap_validate_vma_flags to warn and fail if a mapping sets
VMA_IO_BIT without being marked as kernel-owned.

> diff --git a/mm/vma.c b/mm/vma.c
> index cb0c4c6257567..5996757d5aaf4 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2772,6 +2772,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)) {
> +		/* Only kernel-owned mappings may set VMA_IO_BIT. */
> +		if (WARN_ON_ONCE(vma_flags_test(flags, VMA_IO_BIT)))

[Severity: Medium]
Can this warning be triggered by an unprivileged user intentionally mapping
a device exposed by a buggy or out-of-tree driver?

If a user interacts with a buggy driver that sets VMA_IO_BIT improperly,
triggering this warning will crash the system if panic_on_warn=1 is
configured. Is there a way to validate this without the warning to
prevent a local denial of service?

> +			return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does returning an error here expose a
resource leak in call_mmap_prepare()?

If __mmap_region() handles an mmap call for a driver that implements the
.mmap_prepare hook, the driver may replace the mapping file:

mm/vma.c:call_mmap_prepare() {
    ...
	err = mmap_prepare_validate(&prev_desc, desc);
	if (err)
		return err;

	/* Update fields permitted to be changed. */
	map->pgoff = desc->pgoff;
	if (desc->vm_file != map->file) {
...
		map->file = desc->vm_file;
		map->file_doesnt_need_get = true;
	}
    ...
}

If mmap_prepare_validate() fails because of this new VMA_IO_BIT check returning
an error, call_mmap_prepare() returns early. The state updates to map->file and
map->file_doesnt_need_get are bypassed.

When __mmap_region() aborts, it evaluates map.file_doesnt_need_get to decide
whether to clean up the new file:

mm/vma.c:__mmap_region() {
...
	if (map.file_doesnt_need_get)
		fput(map.file);
...
}

Since map.file_doesnt_need_get evaluates to false, is the reference to the
newly allocated file permanently lost?


[Severity: High]
This is also a pre-existing issue, but can this failure path also lead to a
double-free when mmap_file() fails?

If a driver's .mmap_prepare hook successfully replaces the mapping file,
call_mmap_prepare() sets map->file_doesnt_need_get = true. In
__mmap_new_file_vma(), the get_file() call is skipped.

If mmap_file() then fails (either because the driver's .mmap hook returns an
error, or this validation fails), the error path unconditionally drops the
reference:

mm/vma.c:__mmap_new_file_vma() {
...
	error = mmap_file(map->file, vma, vmg);
	if (error) {
...
		fput(vma->vm_file);
		vma->vm_file = NULL;
		return error;
	}
...
}

The error propagates back to __mmap_region(), which executes the abort_munmap
block:

mm/vma.c:__mmap_region() {
...
abort_munmap:
...
	if (map.file_doesnt_need_get)
		fput(map.file);
...
}

Because map.file_doesnt_need_get is still true, it calls fput() on the exact
same file pointer, leading to a double-free. Does this new validation step
make this sequence more reachable by unprivileged users interacting with
out-of-tree drivers?

> +	}
> +
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce22b@kernel.org?part=24