Re: [PATCH] mm/mremap: populate calculated fields in vrm state correctly

[Xueyuan Chen]

> Commit d5c8aec0542e ("mm/mremap: initial refactor of move_vma()")
> introduced the vma_remap_struct helper to pass state sanely through
> mremap() logic.
> 
> Part of this state is vrm->delta - equal to the absolute difference between
> vrm->old_len and vrm->new_len (the old and new lengths of the VMA,
> respectively).
> 
> This value is used to correctly charge accounted VMAs.
> 
> When shrinking a VMA, mremap_to() sets vrm->old_len equal to vrm->new_len
> having performed the shrink of the VMA first, prior to any potential
> move (e.g. MREMAP_FIXED | MREMAP_MAYMOVE specified).
> 
> However it does not recalculate vrm->delta, which results in incorrectly
> overcharging the VMA.
> 
> The charge leaks to Committed_AS globally, meaning a mischievous user can
> cause allocations to fail across the system in strict overcommit mode.
> 
> Resolve the issue by abstracting the update of vrm->delta to
> vrm_populate_calculated() and invoking this from mremap_to() when
> vrm->old_len is changed.
> 
> Additionally, update vrm->remap_type whose value depends on
> vrm->delta. The field is not read again after the shrink, but it is the
> logically correct thing to do and protects against future changes which
> break this.
> 
> Fixes: d5c8aec0542e ("mm/mremap: initial refactor of move_vma()")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/all/20261017233527.797191-1-nickolai@xxxxxxxxxxxxx/
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> ---
>  mm/mremap.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 2e1ee879f70f..3f645cdc8060 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -887,12 +887,6 @@ unsigned long move_page_tables(struct pagetable_move_control *pmc)
>  	return pmc_progress(pmc);
>  }
>  
> -/* Set vrm->delta to the difference in VMA size specified by user. */
> -static void vrm_set_delta(struct vma_remap_struct *vrm)
> -{
> -	vrm->delta = abs_diff(vrm->old_len, vrm->new_len);
> -}
> -
>  /* Determine what kind of remap this is - shrink, expand or no resize at all. */
>  static enum mremap_type vrm_remap_type(struct vma_remap_struct *vrm)
>  {
> @@ -905,6 +899,13 @@ static enum mremap_type vrm_remap_type(struct vma_remap_struct *vrm)
>  	return MREMAP_EXPAND;
>  }
>  
> +/* Populate calculated fields. */
> +static void vrm_populate_calculated(struct vma_remap_struct *vrm)
> +{
> +	vrm->delta = abs_diff(vrm->old_len, vrm->new_len);
> +	vrm->remap_type = vrm_remap_type(vrm);
> +}

Hi Lorenzo,

Good idea to update remap_type for here too.

Reviewed-by: Xueyuan Chen <xueyuan.chen21@xxxxxxxxx>

-- 
Thanks,
Xueyuan