Re: [PATCH 3/4] mm/madvise: skip zone device folios in cold/pageout PMD range

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

Pre-existing issues:
- [Critical] Direct assignment to *ptep bypasses set_pte_at(), causing hypervisor faults on paravirtualized architectures.
- [Critical] madvise_cold_or_pageout_pte_range() blindly calls pmd_folio() on PMD entries without checking if the VMA allows raw PFN mappings, leading to invalid folio pointer dereferences.
--

--- Patch [3]: [PATCH 3/4] mm/madvise: skip zone device folios in cold/pageout PMD range ---
commit f53274ca71e03d22eaac643cdc7488b95adca203
Author: Gregory Price <[email protected]>

mm/madvise: skip zone device folios in cold/pageout PMD range

This patch modifies madvise_cold_or_pageout_pte_range() to explicitly
filter out ZONE_DEVICE folios. It uses folio_is_zone_device() to prevent
aging or paging out device memory that might bypass existing VMA and PMD
checks.

> diff --git a/mm/madvise.c b/mm/madvise.c
> index 07a21ca31bad4..ffd6a68320a86 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -395,6 +395,9 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
>  
>  		folio = pmd_folio(orig_pmd);
>  

[Severity: Critical]
This is a pre-existing issue, but does this code dereference an invalid
folio pointer?

The madvise_cold_or_pageout_pte_range() function handles MADV_COLD and
MADV_PAGEOUT hints, and the can_madv_lru_vma() filter rejects VM_PFNMAP but
allows VM_MIXEDMAP VMAs.

Device drivers can map raw PFNs (not backed by a struct page) as huge PMDs
into VM_MIXEDMAP VMAs using vmf_insert_pfn_pmd().

When encountering such a PMD, pmd_trans_huge() is true, but pmd_folio()
translates the raw PFN into a garbage struct folio pointer.

Could this cause a kernel panic due to an out-of-bounds read when
folio_is_zone_device() dereferences folio->flags on the invalid pointer?

> +		if (folio_is_zone_device(folio))
> +			goto huge_unlock;
> +
>  		/* Do not interfere with other mappings of this folio */
>  		if (folio_maybe_mapped_shared(folio))
>  			goto huge_unlock;

[ ... ]

While reviewing mm/madvise.c, I also noticed another pre-existing regression
in guard_install_set_pte():

> static int guard_install_set_pte(unsigned long addr, unsigned long next,
> 				 pte_t *ptep, struct mm_walk *walk)
> {
> 	unsigned long *nr_pages = (unsigned long *)walk->private;
> 
> 	/* Simply install a PTE marker, this causes segfault on access. */
> 	*ptep = make_pte_marker(PTE_MARKER_GUARD);

[Severity: Critical]
This is a pre-existing issue, but does this direct assignment bypass
architecture-specific hooks?

By assigning directly to *ptep instead of using set_pte_at(), this skips
hooks required by paravirtualized architectures.

On architectures like Xen PV where guest page tables are mapped read-only by
the hypervisor, will this direct write trigger a hypervisor fault and crash
the kernel if an unprivileged user calls madvise(..., MADV_GUARD_INSTALL) on
a valid anonymous VMA?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3