Re: [PATCH 1/6] mm/mremap: fix hugetlb reservation leak on partial move

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Fri, Oct 30, 2026 at 02:42:46PM +0000, Josef Bacik wrote:
> Moving a part of a private hugetlb mapping with mremap() leaks the huge
> page reservations of the parts that stay. They are not returned when the
> mapping is unmapped, when the process exits or when the pool is resized.
>
> fixup_hugetlb_reservations() takes the reservation map away from the old
> VMA while that VMA still spans its whole range, because the moved part is
> only split off by the unmap at the end of the move. hugetlb_vm_op_close()
> then finds no map in the parts that remain and returns nothing for them:
>
>     mmap() of 4 pages, MAP_PRIVATE | MAP_HUGETLB    HugePages_Rsvd: 4
>     mremap() of the second page to a new address    HugePages_Rsvd: 4
>     munmap() of all of it                           HugePages_Rsvd: 3
>
> The source is unmapped in this case, so the number of VMAs ends up where it
> did before and the check against sysctl_max_map_count stays as it is.
>
> Fix by splitting a hugetlb VMA in prep_move_vma() before a part of it is
> moved, as commit 597e4b9ab48a ("mm/mremap: fix locked_vm leak by splitting
> VMA for MREMAP_DONTUNMAP") does for an mlock()'d VMA. A backport needs that
> commit, which is tagged for stable itself.

Ack, that one goes back to 5.7 in theory, this one 5.16 so all good.

>
> Fixes: 550a7d60bd5e ("mm, hugepages: add mremap() support for hugepage backed vma")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Some nits below but LGTM, so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  mm/mremap.c | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 0d06d7f37f19..cf746bafdee4 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1038,7 +1038,7 @@ static void vrm_stat_account(struct vma_remap_struct *vrm,
>  }
>
>  static bool __check_map_count_against_split(struct mm_struct *mm,
> -					    bool pre_split,
> +					    bool source_stays,
>  					    bool before_unmaps)
>  {
>  	const int sys_map_count = get_sysctl_max_map_count();
> @@ -1092,14 +1092,15 @@ static bool __check_map_count_against_split(struct mm_struct *mm,
>  	 */
>  	map_count += 2;
>
> -	/* If pre-split, the -1 observed above doesn't apply. */
> -	if (pre_split)
> +	/* If the split off source stays, the -1 observed above doesn't apply. */
> +	if (source_stays)
>  		map_count++;
>
>  	return map_count <= sys_map_count;
>  }
>
> -static bool needs_pre_split(struct vma_remap_struct *vrm)
> +/* Is the source range split off its VMA, to then stay mapped? */

NIT, but slightly awkward wording. Honestly I think we can just do without a
comment here, the name self-documents, and the hugetlb 'stuff' is documented in
the comment below.

> +static bool split_source_stays(struct vma_remap_struct *vrm)
>  {
>  	/*
>  	 * An MREMAP_DONTUNMAP of a mlock()'d VMA needs to unlock the
> @@ -1109,11 +1110,23 @@ static bool needs_pre_split(struct vma_remap_struct *vrm)
>  		vma_test(vrm->vma, VMA_LOCKED_BIT);
>  }
>
> +static bool needs_pre_split(struct vma_remap_struct *vrm)
> +{
> +	/*
> +	 * fixup_hugetlb_reservations() takes the reservations away from the
> +	 * source VMA.  The parts of it that do not move have to keep theirs.
> +	 */

If only we could swear in comments... :)

> +	if (vma_is_hugetlb(vrm->vma))
> +		return true;
> +
> +	return split_source_stays(vrm);
> +}
> +
>  /* Do we violate the map count limit if we split VMAs when moving the VMA? */
>  static bool check_map_count_against_split(struct vma_remap_struct *vrm)
>  {
>  	return __check_map_count_against_split(current->mm,
> -		needs_pre_split(vrm), /*before_unmaps=*/false);
> +		split_source_stays(vrm), /*before_unmaps=*/false);
>  }
>
>  /* Do we violate the map count limit if we split VMAs prior to early unmaps? */
> @@ -1162,10 +1175,7 @@ static unsigned long prep_move_vma(struct vma_remap_struct *vrm)
>  	if (err)
>  		return err;
>
> -	/*
> -	 * To account mlock()'d pages correctly in the MREMAP_DONTUNMAP
> -	 * case perform any split ahead of time for an mlock()'d VMA.
> -	 */
> +	/* Perform any split ahead of time if the source VMA has to be distinct. */

Nice reduction in comment size too :)

>  	if (needs_pre_split(vrm)) {
>  		VMA_ITERATOR(vmi, vma->vm_mm, old_addr);
>
>
> --
> 2.55.0
>

--
Cheers, Lorenzo