Re: [PATCH v2 08/40] mm: add mmap action for discontiguous kernel page mapping

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] `discontig_kernel_map_page()` silently maps entire compound pages instead of single pages and maps incorrect memory when passed a tail page.
- [Medium] API design causes an unavoidable memory leak if a driver allocates state in `ops->init()` and a subsequent failure occurs.
- [Low] Missing NULL check for the `ops` struct before dereferencing it in validation code.
--

commit d5639a50ca1090febd85053c26c1f0a170b51deb
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Date:   Mon Sep 14 14:57:28 2026 +0000

    mm: add mmap action for discontiguous kernel page mapping

This commit introduces a new mmap action allowing drivers to map discontiguous 
kernel pages. It adds discontig_kernel_page_ops for defining the mapping 
operation, including initialization and page retrieval logic. Helpers are 
provided to map single pages, ranges, or compound pages.

> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a1f2d375cf7d8..2a92193ac6a5e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
[ ... ]
> @@ -4647,10 +4647,55 @@ static inline void mmap_action_map_kernel_pages_full(struct vm_area_desc *desc,
> +static inline void
> +discontig_kernel_map_page(struct discontig_kernel_page_state *state,
> +			  struct page *page)
> +{
> +	struct folio *folio = page_folio(page);
> +
> +	if (folio_test_large(folio)) {
> +		VM_WARN_ON_ONCE(page != folio_page(folio, 0));
> +		state->action = DISCONTIG_KERNEL_PAGE_MAP_COMPOUND_PAGE;
> +		state->__folio = folio;
> +		state->__nr_pages = min(state->nr_pages_remain,
> +					folio_nr_pages(folio));

[Severity: High]
If a driver passes a tail page here intending to map just that specific page, 
does this unintentionally escalate the mapping to the entire compound page?

The API discards the specific page pointer and sets up a folio mapping
instead. Even though there is a warning when passing a tail page, the code
proceeds to map starting from the head page in apply_discontig_action(),
which could expose unrequested kernel memory to userspace.

> diff --git a/mm/memory.c b/mm/memory.c
> index 448342883e9da..42f0845172471 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
[ ... ]
> +int map_discontig_kernel_pages_prepare(struct vm_area_desc *desc)
> +{
> +	const struct mmap_action *action = &desc->action;
> +	const struct discontig_kernel_page_ops *ops =
> +		action->map_kernel_discontig.ops;
> +
> +	/* At minimum need to be able to get pages. */
> +	if (WARN_ON_ONCE(!ops->get))

[Severity: Low]
Is it possible for the ops pointer to be NULL here?

If a driver erroneously sets up a discontiguous mapping action but passes a
NULL ops pointer, dereferencing ops->get will cause a panic before the
intended validation warning can trigger.

> +		return -EINVAL;
> +
> +	__map_kernel_pages_prepare(desc);
> +	return 0;
> +}
> +
> +static int apply_discontig_action(struct vm_area_struct *vma,
> +				  struct discontig_kernel_page_state *state)
> +{
> +	unsigned long nr_pages = state->__nr_pages;
> +	unsigned long addr = state->addr;
> +	unsigned long i;
[ ... ]
> +	/* Compound folio - have to iterate through each page. */
> +	for (i = 0; i < nr_pages; i++, addr += PAGE_SIZE) {
> +		struct page *page = folio_page(state->__folio, i);

[Severity: High]
As mentioned above, if a tail page was passed to discontig_kernel_map_page(),
this loop always starts mapping from the head page (i = 0) of the folio.

Could this map the wrong memory into userspace if the caller specifically
requested a tail page?

> +		int err;
> +
> +		err = insert_page(vma, addr, page, vma->vm_page_prot,
> +				  /*mkwrite=*/false);
> +		if (err)
> +			return err;
> +	}
> +	return 0;
> +}
> +
> +int map_discontig_kernel_pages_complete(struct vm_area_struct *vma,
> +					struct mmap_action *action)
> +{
> +	const struct discontig_kernel_page_ops *ops =
> +		action->map_kernel_discontig.ops;
> +	struct discontig_kernel_page_state state = {
> +		.start = vma->vm_start,
> +		.end = vma->vm_end,
> +		.addr = vma->vm_start,
> +		.pgoff = vma->vm_pgoff,
> +		.nr_pages_mapped = 0,
> +		.nr_pages_remain = vma_pages(vma),
> +		.vm_private_data = vma->vm_private_data,
> +		.private = action->map_kernel_discontig.init_private,
> +	};
> +	int err = 0;
> +
> +	if (ops->init)
> +		err = ops->init(vma->vm_private_data, &state.private);
> +	if (err)
> +		return err;
> +
> +	do {
> +		unsigned long end, pgoff_end;
> +		unsigned long nr_pages;
> +
> +		/* Default to abort. */
> +		state.action = DISCONTIG_KERNEL_PAGE_ABORT;
> +		err = ops->get(&state);
> +		if (err || state.action == DISCONTIG_KERNEL_PAGE_ABORT)
> +			return err;

[Severity: Medium]
Does this error path leak memory if a driver allocated state in ops->init()?

The commit message mentions that any state established by ops->init() is
released in vm_ops->close. However, if an error occurs here (or below in
apply_discontig_action), the function returns immediately.

Since state is a local variable on the stack, and the driver's close callback
only receives the vma (which has no access to state.private), it appears any
memory assigned to *private would be permanently leaked.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-b4-mmap-prepare-vma-flag-sanify-v2-0-7d9781ed5361@kernel.org?part=8