From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: KunWu Chan <kunwu.chan@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
"Liam R. Howlett" <liam@infradead.org>,
Vlastimil Babka <vbabka@kernel.org>,
Jann Horn <jannh@google.com>, Pedro Falcato <pfalcato@suse.de>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
sashiko-bot <sashiko-bot@kernel.org>,
stable@vger.kernel.org
Subject: Re: [PATCH] mm/mremap: account mm->locked_vm correctly for MREMAP_DONTUNMAP
Date: Mon, 31 Aug 2026 09:19:37 +0100 [thread overview]
Message-ID: <apU5ErQKpDNqEuW3@gremlin> (raw)
In-Reply-To: <CAN_Opa9CZUTUyvveTnVXK7Wy9Xq-9wc=QkASceLv5hTE-NiNnw@mail.gmail.com>
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
prev parent reply other threads:[~2026-08-31 8:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-28 11:20 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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=apU5ErQKpDNqEuW3@gremlin \
--to=ljs@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=jannh@google.com \
--cc=kunwu.chan@gmail.com \
--cc=liam@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=pfalcato@suse.de \
--cc=sashiko-bot@kernel.org \
--cc=stable@vger.kernel.org \
--cc=vbabka@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®