mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm/mremap: account mm->locked_vm correctly for MREMAP_DONTUNMAP
@ 2026-08-28 11:20 Lorenzo Stoakes (ARM)
  2026-08-28 13:39 ` Vlastimil Babka (SUSE)
  2026-08-29 14:37 ` KunWu Chan
  0 siblings, 2 replies; 4+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-28 11:20 UTC (permalink / raw)
  To: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
	Pedro Falcato
  Cc: linux-mm, linux-kernel, sashiko-bot, Kunwu Chan, stable,
	Lorenzo Stoakes (ARM)

When a VMA is mremap()'d with MREMAP_DONTUNMAP set, that results in the VMA
being copied, but the source VMA not being unmapped.

If the VMA is mlock()'d this is a legal operation, though the source VMA
has its VMA_LOCKED_BIT cleared.

However this is done in dontunmap_complete(), after mm->locked_vm was
incremented via vrm_stat_account(), resulting in double-counting.

Worse, this is not even corrected when source VMA is unmapped, due to
the VMA_LOCKED_BIT flag having been cleared.

This all works fine in the usual mremap() case (without MREMAP_DONTUNMAP),
as the source VMA is unmapped with VMA_LOCKED_BIT intact, at which time
mm->locked_vm is decremented accordingly.

Resolve the issue by invoking vrm_stat_account() only after
dontunmap_complete() has run.

Note that MREMAP_DONTUNMAP requires old_len == new_len, so no need to
account for a delta in size in this case.

The bug was introduced by commit b714ccb02a76 ("mm/mremap: complete
refactor of move_vma()") which incorrectly reordered the accounting and the
clearing of the VMA_LOCKED_BIT flag.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260825-fix-mremap-dontunmap-pgoff-v1-1-39a40b2c98b3@kernel.org
Reported-by: Kunwu Chan <kunwu.chan@gmail.com>
Closes: https://lore.kernel.org/all/20260828094823.594279-1-kunwu.chan@linux.dev/
Fixes: b714ccb02a76 ("mm/mremap: complete refactor of move_vma()")
Cc: stable@vger.kernel.org
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/mremap.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index 2b4b523a86b8..7c368440fafe 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1355,12 +1355,11 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
 		if (vma_is_anonymous(vma) && !vma->vm_file)
 			vma_set_pgoff(vma, pgoff_unfaulted);
 	}
-
-	/* Because we won't unmap we don't need to touch locked_vm. */
 }
 
 static unsigned long move_vma(struct vma_remap_struct *vrm)
 {
+	const bool is_dontunmap = vrm->flags & MREMAP_DONTUNMAP;
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *new_vma;
 	unsigned long hiwater_vm;
@@ -1401,10 +1400,10 @@ static unsigned long move_vma(struct vma_remap_struct *vrm)
 	 */
 	hiwater_vm = mm->hiwater_vm;
 
-	vrm_stat_account(vrm, vrm->new_len);
-	if (unlikely(!err && (vrm->flags & MREMAP_DONTUNMAP)))
+	if (unlikely(is_dontunmap && !err))
 		dontunmap_complete(vrm, new_vma);
-	else
+	vrm_stat_account(vrm, vrm->new_len);
+	if (!is_dontunmap || err)
 		unmap_source_vma(vrm);
 
 	mm->hiwater_vm = hiwater_vm;

---
base-commit: aeddb4d52acfcc5ce5e988acd48f2906fe966ca3
change-id: 20260828-mremap-fix-locked-vm-b8991ffc4181

Best regards,
-- 
Lorenzo Stoakes (ARM) <ljs@kernel.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/mremap: account mm->locked_vm correctly for MREMAP_DONTUNMAP
  2026-08-28 11:20 [PATCH] mm/mremap: account mm->locked_vm correctly for MREMAP_DONTUNMAP Lorenzo Stoakes (ARM)
@ 2026-08-28 13:39 ` Vlastimil Babka (SUSE)
  2026-08-29 14:37 ` KunWu Chan
  1 sibling, 0 replies; 4+ messages in thread
From: Vlastimil Babka (SUSE) @ 2026-08-28 13:39 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM),
	Andrew Morton, Liam R. Howlett, Jann Horn, Pedro Falcato
  Cc: linux-mm, linux-kernel, sashiko-bot, Kunwu Chan, stable

On 8/28/26 13:20, Lorenzo Stoakes (ARM) wrote:
> When a VMA is mremap()'d with MREMAP_DONTUNMAP set, that results in the VMA
> being copied, but the source VMA not being unmapped.
> 
> If the VMA is mlock()'d this is a legal operation, though the source VMA
> has its VMA_LOCKED_BIT cleared.
> 
> However this is done in dontunmap_complete(), after mm->locked_vm was
> incremented via vrm_stat_account(), resulting in double-counting.
> 
> Worse, this is not even corrected when source VMA is unmapped, due to
> the VMA_LOCKED_BIT flag having been cleared.
> 
> This all works fine in the usual mremap() case (without MREMAP_DONTUNMAP),
> as the source VMA is unmapped with VMA_LOCKED_BIT intact, at which time
> mm->locked_vm is decremented accordingly.
> 
> Resolve the issue by invoking vrm_stat_account() only after
> dontunmap_complete() has run.
> 
> Note that MREMAP_DONTUNMAP requires old_len == new_len, so no need to
> account for a delta in size in this case.
> 
> The bug was introduced by commit b714ccb02a76 ("mm/mremap: complete
> refactor of move_vma()") which incorrectly reordered the accounting and the
> clearing of the VMA_LOCKED_BIT flag.
> 
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260825-fix-mremap-dontunmap-pgoff-v1-1-39a40b2c98b3@kernel.org
> Reported-by: Kunwu Chan <kunwu.chan@gmail.com>
> Closes: https://lore.kernel.org/all/20260828094823.594279-1-kunwu.chan@linux.dev/
> Fixes: b714ccb02a76 ("mm/mremap: complete refactor of move_vma()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/mremap.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 2b4b523a86b8..7c368440fafe 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1355,12 +1355,11 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
>  		if (vma_is_anonymous(vma) && !vma->vm_file)
>  			vma_set_pgoff(vma, pgoff_unfaulted);
>  	}
> -
> -	/* Because we won't unmap we don't need to touch locked_vm. */
>  }
>  
>  static unsigned long move_vma(struct vma_remap_struct *vrm)
>  {
> +	const bool is_dontunmap = vrm->flags & MREMAP_DONTUNMAP;
>  	struct mm_struct *mm = current->mm;
>  	struct vm_area_struct *new_vma;
>  	unsigned long hiwater_vm;
> @@ -1401,10 +1400,10 @@ static unsigned long move_vma(struct vma_remap_struct *vrm)
>  	 */
>  	hiwater_vm = mm->hiwater_vm;
>  
> -	vrm_stat_account(vrm, vrm->new_len);
> -	if (unlikely(!err && (vrm->flags & MREMAP_DONTUNMAP)))
> +	if (unlikely(is_dontunmap && !err))
>  		dontunmap_complete(vrm, new_vma);
> -	else
> +	vrm_stat_account(vrm, vrm->new_len);
> +	if (!is_dontunmap || err)
>  		unmap_source_vma(vrm);
>  
>  	mm->hiwater_vm = hiwater_vm;
> 
> ---
> base-commit: aeddb4d52acfcc5ce5e988acd48f2906fe966ca3
> change-id: 20260828-mremap-fix-locked-vm-b8991ffc4181
> 
> Best regards,


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/mremap: account mm->locked_vm correctly for MREMAP_DONTUNMAP
  2026-08-28 11:20 [PATCH] mm/mremap: account mm->locked_vm correctly for MREMAP_DONTUNMAP Lorenzo Stoakes (ARM)
  2026-08-28 13:39 ` Vlastimil Babka (SUSE)
@ 2026-08-29 14:37 ` KunWu Chan
  2026-08-31  8:19   ` Lorenzo Stoakes (ARM)
  1 sibling, 1 reply; 4+ messages in thread
From: KunWu Chan @ 2026-08-29 14:37 UTC (permalink / raw)
  To: Lorenzo Stoakes (ARM)
  Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
	Pedro Falcato, linux-mm, linux-kernel, sashiko-bot, stable

Hi Lorenzo,

Thanks for the fix. I agree this is a very clean solution.

On Fri, Aug 28, 2026 at 7:20 PM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
>
> When a VMA is mremap()'d with MREMAP_DONTUNMAP set, that results in the VMA
> being copied, but the source VMA not being unmapped.
>
> If the VMA is mlock()'d this is a legal operation, though the source VMA
> has its VMA_LOCKED_BIT cleared.
>
> However this is done in dontunmap_complete(), after mm->locked_vm was
> incremented via vrm_stat_account(), resulting in double-counting.
>
> Worse, this is not even corrected when source VMA is unmapped, due to
> the VMA_LOCKED_BIT flag having been cleared.
>
> This all works fine in the usual mremap() case (without MREMAP_DONTUNMAP),
> as the source VMA is unmapped with VMA_LOCKED_BIT intact, at which time
> mm->locked_vm is decremented accordingly.
>
> Resolve the issue by invoking vrm_stat_account() only after
> dontunmap_complete() has run.
>
> Note that MREMAP_DONTUNMAP requires old_len == new_len, so no need to
> account for a delta in size in this case.
>
> The bug was introduced by commit b714ccb02a76 ("mm/mremap: complete
> refactor of move_vma()") which incorrectly reordered the accounting and the
> clearing of the VMA_LOCKED_BIT flag.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260825-fix-mremap-dontunmap-pgoff-v1-1-39a40b2c98b3@kernel.org
> Reported-by: Kunwu Chan <kunwu.chan@gmail.com>
> Closes: https://lore.kernel.org/all/20260828094823.594279-1-kunwu.chan@linux.dev/
> Fixes: b714ccb02a76 ("mm/mremap: complete refactor of move_vma()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
>  mm/mremap.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 2b4b523a86b8..7c368440fafe 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1355,12 +1355,11 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
>                 if (vma_is_anonymous(vma) && !vma->vm_file)
>                         vma_set_pgoff(vma, pgoff_unfaulted);
>         }
> -
> -       /* Because we won't unmap we don't need to touch locked_vm. */
>  }
>
>  static unsigned long move_vma(struct vma_remap_struct *vrm)
>  {
> +       const bool is_dontunmap = vrm->flags & MREMAP_DONTUNMAP;
>         struct mm_struct *mm = current->mm;
>         struct vm_area_struct *new_vma;
>         unsigned long hiwater_vm;
> @@ -1401,10 +1400,10 @@ static unsigned long move_vma(struct vma_remap_struct *vrm)
>          */
>         hiwater_vm = mm->hiwater_vm;
>
> -       vrm_stat_account(vrm, vrm->new_len);
> -       if (unlikely(!err && (vrm->flags & MREMAP_DONTUNMAP)))
> +       if (unlikely(is_dontunmap && !err))
>                 dontunmap_complete(vrm, new_vma);
> -       else
> +       vrm_stat_account(vrm, vrm->new_len);
> +       if (!is_dontunmap || err)
>                 unmap_source_vma(vrm);
>
>         mm->hiwater_vm = hiwater_vm;
>
> ---
> base-commit: aeddb4d52acfcc5ce5e988acd48f2906fe966ca3
> change-id: 20260828-mremap-fix-locked-vm-b8991ffc4181
>
> Best regards,
> --
> Lorenzo Stoakes (ARM) <ljs@kernel.org>
>

I tested the patch with the following tests [1]:
1: test-vmlck.c — Andrew Morton's test for basic locked_vm accounting
with mlock()/MREMAP_DONTUNMAP.
2: test_mlock_onfault_nofault.c — MLOCK_ONFAULT with no pages faulted in.
3: test_mlock_onfault_fault.c — MLOCK_ONFAULT with pages faulted in.

All three tests passed with the patch applied.
Tested-by:  Kunwu Chan <kunwu.chan@gmail.com>

The fix also looks correct to me.
Reviewed-by:  Kunwu Chan <kunwu.chan@gmail.com>

[1] https://github.com/KunWuChan/vmlck_test

Thanks again!

Best,
Kunwu

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/mremap: account mm->locked_vm correctly for MREMAP_DONTUNMAP
  2026-08-29 14:37 ` KunWu Chan
@ 2026-08-31  8:19   ` Lorenzo Stoakes (ARM)
  0 siblings, 0 replies; 4+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-31  8:19 UTC (permalink / raw)
  To: KunWu Chan
  Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
	Pedro Falcato, linux-mm, linux-kernel, sashiko-bot, stable

On Sat, Aug 29, 2026 at 10:37:11PM +0800, KunWu Chan wrote:
> Hi Lorenzo,
>
> Thanks for the fix. I agree this is a very clean solution.
>
> On Fri, Aug 28, 2026 at 7:20 PM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
> >
> > When a VMA is mremap()'d with MREMAP_DONTUNMAP set, that results in the VMA
> > being copied, but the source VMA not being unmapped.
> >
> > If the VMA is mlock()'d this is a legal operation, though the source VMA
> > has its VMA_LOCKED_BIT cleared.
> >
> > However this is done in dontunmap_complete(), after mm->locked_vm was
> > incremented via vrm_stat_account(), resulting in double-counting.
> >
> > Worse, this is not even corrected when source VMA is unmapped, due to
> > the VMA_LOCKED_BIT flag having been cleared.
> >
> > This all works fine in the usual mremap() case (without MREMAP_DONTUNMAP),
> > as the source VMA is unmapped with VMA_LOCKED_BIT intact, at which time
> > mm->locked_vm is decremented accordingly.
> >
> > Resolve the issue by invoking vrm_stat_account() only after
> > dontunmap_complete() has run.
> >
> > Note that MREMAP_DONTUNMAP requires old_len == new_len, so no need to
> > account for a delta in size in this case.
> >
> > The bug was introduced by commit b714ccb02a76 ("mm/mremap: complete
> > refactor of move_vma()") which incorrectly reordered the accounting and the
> > clearing of the VMA_LOCKED_BIT flag.
> >
> > Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> > Closes: https://sashiko.dev/#/patchset/20260825-fix-mremap-dontunmap-pgoff-v1-1-39a40b2c98b3@kernel.org
> > Reported-by: Kunwu Chan <kunwu.chan@gmail.com>
> > Closes: https://lore.kernel.org/all/20260828094823.594279-1-kunwu.chan@linux.dev/
> > Fixes: b714ccb02a76 ("mm/mremap: complete refactor of move_vma()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  mm/mremap.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/mremap.c b/mm/mremap.c
> > index 2b4b523a86b8..7c368440fafe 100644
> > --- a/mm/mremap.c
> > +++ b/mm/mremap.c
> > @@ -1355,12 +1355,11 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
> >                 if (vma_is_anonymous(vma) && !vma->vm_file)
> >                         vma_set_pgoff(vma, pgoff_unfaulted);
> >         }
> > -
> > -       /* Because we won't unmap we don't need to touch locked_vm. */
> >  }
> >
> >  static unsigned long move_vma(struct vma_remap_struct *vrm)
> >  {
> > +       const bool is_dontunmap = vrm->flags & MREMAP_DONTUNMAP;
> >         struct mm_struct *mm = current->mm;
> >         struct vm_area_struct *new_vma;
> >         unsigned long hiwater_vm;
> > @@ -1401,10 +1400,10 @@ static unsigned long move_vma(struct vma_remap_struct *vrm)
> >          */
> >         hiwater_vm = mm->hiwater_vm;
> >
> > -       vrm_stat_account(vrm, vrm->new_len);
> > -       if (unlikely(!err && (vrm->flags & MREMAP_DONTUNMAP)))
> > +       if (unlikely(is_dontunmap && !err))
> >                 dontunmap_complete(vrm, new_vma);
> > -       else
> > +       vrm_stat_account(vrm, vrm->new_len);
> > +       if (!is_dontunmap || err)
> >                 unmap_source_vma(vrm);
> >
> >         mm->hiwater_vm = hiwater_vm;
> >
> > ---
> > base-commit: aeddb4d52acfcc5ce5e988acd48f2906fe966ca3
> > change-id: 20260828-mremap-fix-locked-vm-b8991ffc4181
> >
> > Best regards,
> > --
> > Lorenzo Stoakes (ARM) <ljs@kernel.org>
> >
>
> I tested the patch with the following tests [1]:
> 1: test-vmlck.c — Andrew Morton's test for basic locked_vm accounting
> with mlock()/MREMAP_DONTUNMAP.
> 2: test_mlock_onfault_nofault.c — MLOCK_ONFAULT with no pages faulted in.
> 3: test_mlock_onfault_fault.c — MLOCK_ONFAULT with pages faulted in.
>
> All three tests passed with the patch applied.
> Tested-by:  Kunwu Chan <kunwu.chan@gmail.com>
>
> The fix also looks correct to me.
> Reviewed-by:  Kunwu Chan <kunwu.chan@gmail.com>

Great thanks! :)

>
> [1] https://github.com/KunWuChan/vmlck_test
>
> Thanks again!
>
> Best,
> Kunwu

--
Cheers, Lorenzo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-31  8:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-28 11:20 [PATCH] mm/mremap: account mm->locked_vm correctly for MREMAP_DONTUNMAP Lorenzo Stoakes (ARM)
2026-08-28 13:39 ` Vlastimil Babka (SUSE)
2026-08-29 14:37 ` KunWu Chan
2026-08-31  8:19   ` Lorenzo Stoakes (ARM)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®