Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Insufficient Validation of nr_pages (Integer Overflow and Zero-Page Hang) --
commit 0017a54bcba2043c3f552b308a32a39be664f463 Author: Lorenzo Stoakes (ARM) <[email protected]> mm: add mmap action for discontiguous kernel page mapping The existing kernel page mapping mmap actions allow for partial and full mapping of an array of struct page pointers. However some drivers require the mapping of discontiguous ranges. Permit this by providing discontig_kernel_page_ops which allows a driver to specify how the operation should begin and how batches of pages should be retrieved. > diff --git a/mm/memory.c b/mm/memory.c > index dc38cad1dcb7e..8c9675451d4ba 100644 > --- a/mm/memory.c > +++ b/mm/memory.c [ ... ] > int map_kernel_pages_prepare(struct vm_area_desc *desc) > { > const struct mmap_action *action = &desc->action; > const unsigned long addr = action->map_kernel.start; > unsigned long nr_pages, end; > > - if (!vma_desc_test(desc, VMA_MIXEDMAP_BIT)) { > - VM_WARN_ON_ONCE(mmap_read_trylock(desc->mm)); > - VM_WARN_ON_ONCE(vma_desc_test(desc, VMA_PFNMAP_BIT)); > - vma_desc_set_flags(desc, VMA_MIXEDMAP_BIT); > - } > + __map_kernel_pages_prepare(desc); > > nr_pages = action->map_kernel.nr_pages; > end = addr + PAGE_SIZE * nr_pages; > if (!range_in_vma_desc(desc, addr, end)) > return -EFAULT; [Severity: High] Can the end calculation overflow here if nr_pages is exceptionally large? If a driver supplies an overly large nr_pages, PAGE_SIZE * nr_pages could wrap to a small value. Because range_is_subset() lacks an inner_start <= inner_end validation, this wrapped end value would incorrectly pass the range_in_vma_desc() bounds check. This could lead to inserting PTEs far out of VMA bounds and corrupting page tables. [ ... ] > +int map_discontig_kernel_pages_complete(struct vm_area_struct *vma, > + struct mmap_action *action) > +{ [ ... ] > + 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; > + nr_pages = state.__nr_pages; > + > + end = state.addr + PAGE_SIZE * nr_pages; > + if (end > vma->vm_end) > + return -EINVAL; [Severity: High] Does this bounds check handle a huge nr_pages value correctly? Similar to the check in map_kernel_pages_prepare(), if nr_pages is huge, end will wrap to a small value. This bypasses the end > vma->vm_end check, potentially allowing page table corruption. > + pgoff_end = state.pgoff + nr_pages; > + > + err = apply_discontig_action(vma, &state); > + if (err) > + return err; > + > + state.addr = end; > + state.pgoff = pgoff_end; > + state.nr_pages_mapped += nr_pages; > + state.nr_pages_remain -= nr_pages; > + } while (state.addr < vma->vm_end); [Severity: High] Could this result in an infinite loop if nr_pages is 0? If a driver provides 0 pages (for instance, via discontig_kernel_map_page_range(..., 0)), end will equal state.addr. state.addr is then updated with a 0 increment, causing the while (state.addr < vma->vm_end) loop to execute infinitely and hang the kernel. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce...@kernel.org?part=8
