Re: [PATCH v2] mm/madvise: zap markers from guard regions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm/madvise: zap_range() reclaims PTE tables needed by
  madvise_guard_install()
--

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

mm/madvise: zap markers from guard regions

This commit introduces a zap_range() helper and uses it in
madvise_guard_install() with ZAP_FLAG_DROP_MARKER so that existing PTE
markers are dropped when zapping a range prior to installing guard markers.

> diff --git a/mm/madvise.c b/mm/madvise.c
> index 0214efd2a014f..4f77285832079 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -850,6 +850,19 @@ static int madvise_free_single_vma(struct madvise_behavior *madv_behavior)
>  	return 0;
>  }
>  
> +static void zap_range(struct madvise_behavior *madv_behavior,
> +		      zap_flags_t flags)
> +{
> +	const struct madvise_behavior_range *range = &madv_behavior->range;
> +	struct zap_details details = {
> +		.reclaim_pt = true,
> +		.zap_flags = flags,
> +	};
> +
> +	zap_vma_range_batched(madv_behavior->tlb, madv_behavior->vma,
> +			      range->start, range->end - range->start, &details);
> +}

[ ... ]

> @@ -1217,9 +1224,11 @@ static long madvise_guard_install(struct madvise_behavior *madv_behavior)
>  
>  		/*
>  		 * OK some of the range have non-guard pages mapped, zap
> -		 * them. This leaves existing guard pages in place.
> +		 * them. This also drops any PTE markers in the range,
> +		 * including existing guard markers, which we reinstall on
> +		 * the next attempt.
>  		 */
> -		zap_vma_range(vma, range->start, range->end - range->start);
> +		zap_range(madv_behavior, ZAP_FLAG_DROP_MARKER);
>  	}

[Severity: Medium]
Should zap_range() avoid setting details.reclaim_pt = true when called from
madvise_guard_install()?

When a guard installation range spans at least a full PMD and contains any
mapped page, swap entry, or non-guard marker, zap_range() with
ZAP_FLAG_DROP_MARKER clears every PTE in the PMD and reclaims the empty PTE
page table via pte_free_tlb().  On the very next iteration of the loop,
madvise_guard_install() walks the same range to install PTE_MARKER_GUARD
entries and walk_pmd_range() has to allocate a replacement PTE page table via
__pte_alloc() while the old table is still queued in madv_behavior->tlb.

Besides the unnecessary page table teardown and reallocation, __pte_alloc()
on the retry pass can fail with -ENOMEM under tight memory or memcg limits
even when the PTE page tables were already allocated before the zap, leaving
any pre-existing guard markers in the zapped range removed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-disallow-uffd-guard-install-v2-1-9d0063c70542@kernel.org?part=1