linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: "Liam R . Howlett" <Liam.Howlett@oracle.com>,
	Jann Horn <jannh@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/7] mm/mremap: correctly handle partial mremap() of VMA starting at 0
Date: Wed, 5 Mar 2025 12:50:25 +0100	[thread overview]
Message-ID: <0a4b2608-2dc2-42fa-b30c-1c1a62b56bbf@suse.cz> (raw)
In-Reply-To: <195c3956c70a142b12465e09b4aa5e33a898b789.1740911247.git.lorenzo.stoakes@oracle.com>

On 3/3/25 12:08, Lorenzo Stoakes wrote:
> Consider the case of a a partial mremap() (that results in a VMA split) of
> an accountable VMA (i.e. which has the VM_ACCOUNT flag set) whose start
> address is zero, with the MREMAP_MAYMOVE flag specified and a scenario
> where a move does in fact occur:
> 
>        addr  end
>         |     |
>         v     v
>     |-------------|
>     |     vma     |
>     |-------------|
>     0
> 
> This move is affected by unmapping the range [addr, end). In order to
> prevent an incorrect decrement of accounted memory which has already been
> determined, the mremap() code in move_vma() clears VM_ACCOUNT from the VMA
> prior to doing so, before reestablishing it in each of the VMAs post-split:
> 
>     addr  end
>      |     |
>      v     v
>  |---|     |---|
>  | A |     | B |
>  |---|     |---|
> 
> Commit 6b73cff239e5 ("mm: change munmap splitting order and move_vma()")
> changed this logic such as to determine whether there is a need to do so by
> establishing account_start and account_end and, in the instance where such
> an operation is required, assigning them to vma->vm_start and vma->vm_end.
> 
> Later the code checks if the operation is required for 'A' referenced above
> thusly:
> 
> 	if (account_start) {
> 		...
> 	}
> 
> However, if the VMA described above has vma->vm_start == 0, which is now
> assigned to account_start, this branch will not be executed.
> 
> As a result, the VMA 'A' above will remain stripped of its VM_ACCOUNT flag,
> incorrectly.
> 
> The fix is to simply convert these variables to booleans and set them as
> required.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Fixes: 6b73cff239e5 ("mm: change munmap splitting order and move_vma()")
> Cc: stable@vger.kernel.org

Looks like the variables would be more accurately named account_prev and
account_next? But maybe it's refactored later anyway. Keeps the diff to
stable backport smaller.

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>



  parent reply	other threads:[~2025-03-05 18:59 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-03 11:08 [PATCH 0/7] refactor mremap and fix bug Lorenzo Stoakes
2025-03-03 11:08 ` [PATCH 1/7] mm/mremap: correctly handle partial mremap() of VMA starting at 0 Lorenzo Stoakes
2025-03-03 13:26   ` Harry Yoo
2025-03-03 16:09   ` Liam R. Howlett
2025-03-05 11:50   ` Vlastimil Babka [this message]
2025-03-05 19:46     ` Lorenzo Stoakes
2025-03-03 11:08 ` [PATCH 2/7] mm/mremap: refactor mremap() system call implementation Lorenzo Stoakes
2025-03-03 17:12   ` Liam R. Howlett
2025-03-03 17:20     ` Lorenzo Stoakes
2025-03-03 18:50       ` Liam R. Howlett
2025-03-05  1:47   ` Harry Yoo
2025-03-05 11:23     ` Lorenzo Stoakes
2025-03-03 11:08 ` [PATCH 3/7] mm/mremap: introduce and use vma_remap_struct threaded state Lorenzo Stoakes
2025-03-05 18:52   ` Liam R. Howlett
2025-03-05 19:43     ` Lorenzo Stoakes
2025-03-05 19:55       ` Liam R. Howlett
2025-03-03 11:08 ` [PATCH 4/7] mm/mremap: initial refactor of move_vma() Lorenzo Stoakes
2025-03-04 21:45   ` Yosry Ahmed
2025-03-04 23:15     ` Andrew Morton
2025-03-05 19:20   ` Liam R. Howlett
2025-03-05 20:01     ` Lorenzo Stoakes
2025-03-03 11:08 ` [PATCH 5/7] mm/mremap: complete " Lorenzo Stoakes
2025-03-03 11:08 ` [PATCH 6/7] mm/mremap: refactor move_page_tables(), abstracting state Lorenzo Stoakes
2025-03-03 11:08 ` [PATCH 7/7] mm/mremap: thread state through move page table operation Lorenzo Stoakes
2025-03-03 23:17   ` kernel test robot
2025-03-03 23:30   ` kernel test robot
2025-03-04  0:05     ` Andrew Morton
2025-03-04  0:12       ` Andrew Morton
2025-03-04  5:40         ` Lorenzo Stoakes

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=0a4b2608-2dc2-42fa-b30c-1c1a62b56bbf@suse.cz \
    --to=vbabka@suse.cz \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    /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