stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 6.4 0/8] 6.4.3-rc1 review
@ 2023-07-09 11:14 Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 1/8] mm: disable CONFIG_PER_VMA_LOCK until its fixed Greg Kroah-Hartman
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable
  Cc: Greg Kroah-Hartman, patches, linux-kernel, torvalds, akpm, linux,
	shuah, patches, lkft-triage, pavel, jonathanh, f.fainelli,
	sudipm.mukherjee, srw, rwarsow, conor

This is the start of the stable review cycle for the 6.4.3 release.
There are 8 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Tue, 11 Jul 2023 11:13:33 +0000.
Anything received after that time might be too late.

The whole patch series can be found in one patch at:
	https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.4.3-rc1.gz
or in the git tree and branch at:
	git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.4.y
and the diffstat can be found below.

thanks,

greg k-h

-------------
Pseudo-Shortlog of commits:

Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Linux 6.4.3-rc1

Suren Baghdasaryan <surenb@google.com>
    fork: lock VMAs of the parent process when forking, again

Suren Baghdasaryan <surenb@google.com>
    fork: lock VMAs of the parent process when forking

Liu Shixin <liushixin2@huawei.com>
    bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page

Peter Collingbourne <pcc@google.com>
    mm: call arch_swap_restore() from do_swap_page()

Hugh Dickins <hughd@google.com>
    mm: lock newly mapped VMA with corrected ordering

Suren Baghdasaryan <surenb@google.com>
    mm: lock newly mapped VMA which can be modified after it becomes visible

Suren Baghdasaryan <surenb@google.com>
    mm: lock a vma before stack expansion

Suren Baghdasaryan <surenb@google.com>
    mm: disable CONFIG_PER_VMA_LOCK until its fixed


-------------

Diffstat:

 Makefile                     | 4 ++--
 include/linux/bootmem_info.h | 2 ++
 kernel/fork.c                | 7 +++++++
 mm/Kconfig                   | 3 ++-
 mm/memory.c                  | 7 +++++++
 mm/mmap.c                    | 6 ++++++
 6 files changed, 26 insertions(+), 3 deletions(-)



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

* [PATCH 6.4 1/8] mm: disable CONFIG_PER_VMA_LOCK until its fixed
  2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
@ 2023-07-09 11:14 ` Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 2/8] mm: lock a vma before stack expansion Greg Kroah-Hartman
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable
  Cc: Greg Kroah-Hartman, patches, Jiri Slaby, Jacob Young,
	Suren Baghdasaryan, David Hildenbrand, Holger Hoffstätte,
	Andrew Morton

From: Suren Baghdasaryan <surenb@google.com>

commit f96c48670319d685d18d50819ed0c1ef751ed2ac upstream.

A memory corruption was reported in [1] with bisection pointing to the
patch [2] enabling per-VMA locks for x86.  Disable per-VMA locks config to
prevent this issue until the fix is confirmed.  This is expected to be a
temporary measure.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=217624
[2] https://lore.kernel.org/all/20230227173632.3292573-30-surenb@google.com

Link: https://lkml.kernel.org/r/20230706011400.2949242-3-surenb@google.com
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Closes: https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/
Reported-by: Jacob Young <jacobly.alt@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217624
Fixes: 0bff0aaea03e ("x86/mm: try VMA lock-based page fault handling first")
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Holger Hoffstätte <holger@applied-asynchrony.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/Kconfig |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1198,8 +1198,9 @@ config ARCH_SUPPORTS_PER_VMA_LOCK
        def_bool n
 
 config PER_VMA_LOCK
-	def_bool y
+	bool "Enable per-vma locking during page fault handling."
 	depends on ARCH_SUPPORTS_PER_VMA_LOCK && MMU && SMP
+	depends on BROKEN
 	help
 	  Allow per-vma locking during page fault handling.
 



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

* [PATCH 6.4 2/8] mm: lock a vma before stack expansion
  2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 1/8] mm: disable CONFIG_PER_VMA_LOCK until its fixed Greg Kroah-Hartman
@ 2023-07-09 11:14 ` Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 3/8] mm: lock newly mapped VMA which can be modified after it becomes visible Greg Kroah-Hartman
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable; +Cc: Greg Kroah-Hartman, patches, Suren Baghdasaryan, Linus Torvalds

From: Suren Baghdasaryan <surenb@google.com>

commit c137381f71aec755fbf47cd4e9bd4dce752c054c upstream.

With recent changes necessitating mmap_lock to be held for write while
expanding a stack, per-VMA locks should follow the same rules and be
write-locked to prevent page faults into the VMA being expanded. Add
the necessary locking.

Cc: stable@vger.kernel.org
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/mmap.c |    4 ++++
 1 file changed, 4 insertions(+)

--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1975,6 +1975,8 @@ static int expand_upwards(struct vm_area
 		return -ENOMEM;
 	}
 
+	/* Lock the VMA before expanding to prevent concurrent page faults */
+	vma_start_write(vma);
 	/*
 	 * vma->vm_start/vm_end cannot change under us because the caller
 	 * is required to hold the mmap_lock in read mode.  We need the
@@ -2062,6 +2064,8 @@ int expand_downwards(struct vm_area_stru
 		return -ENOMEM;
 	}
 
+	/* Lock the VMA before expanding to prevent concurrent page faults */
+	vma_start_write(vma);
 	/*
 	 * vma->vm_start/vm_end cannot change under us because the caller
 	 * is required to hold the mmap_lock in read mode.  We need the



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

* [PATCH 6.4 3/8] mm: lock newly mapped VMA which can be modified after it becomes visible
  2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 1/8] mm: disable CONFIG_PER_VMA_LOCK until its fixed Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 2/8] mm: lock a vma before stack expansion Greg Kroah-Hartman
@ 2023-07-09 11:14 ` Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 4/8] mm: lock newly mapped VMA with corrected ordering Greg Kroah-Hartman
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable; +Cc: Greg Kroah-Hartman, patches, Suren Baghdasaryan, Linus Torvalds

From: Suren Baghdasaryan <surenb@google.com>

commit 33313a747e81af9f31d0d45de78c9397fa3655eb upstream.

mmap_region adds a newly created VMA into VMA tree and might modify it
afterwards before dropping the mmap_lock.  This poses a problem for page
faults handled under per-VMA locks because they don't take the mmap_lock
and can stumble on this VMA while it's still being modified.  Currently
this does not pose a problem since post-addition modifications are done
only for file-backed VMAs, which are not handled under per-VMA lock.
However, once support for handling file-backed page faults with per-VMA
locks is added, this will become a race.

Fix this by write-locking the VMA before inserting it into the VMA tree.
Other places where a new VMA is added into VMA tree do not modify it
after the insertion, so do not need the same locking.

Cc: stable@vger.kernel.org
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/mmap.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2804,6 +2804,8 @@ cannot_expand:
 	if (vma->vm_file)
 		i_mmap_lock_write(vma->vm_file->f_mapping);
 
+	/* Lock the VMA since it is modified after insertion into VMA tree */
+	vma_start_write(vma);
 	vma_iter_store(&vmi, vma);
 	mm->map_count++;
 	if (vma->vm_file) {



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

* [PATCH 6.4 4/8] mm: lock newly mapped VMA with corrected ordering
  2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
                   ` (2 preceding siblings ...)
  2023-07-09 11:14 ` [PATCH 6.4 3/8] mm: lock newly mapped VMA which can be modified after it becomes visible Greg Kroah-Hartman
@ 2023-07-09 11:14 ` Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 5/8] mm: call arch_swap_restore() from do_swap_page() Greg Kroah-Hartman
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable
  Cc: Greg Kroah-Hartman, patches, Hugh Dickins, Suren Baghdasaryan,
	Linus Torvalds

From: Hugh Dickins <hughd@google.com>

commit 1c7873e3364570ec89343ff4877e0f27a7b21a61 upstream.

Lockdep is certainly right to complain about

  (&vma->vm_lock->lock){++++}-{3:3}, at: vma_start_write+0x2d/0x3f
                 but task is already holding lock:
  (&mapping->i_mmap_rwsem){+.+.}-{3:3}, at: mmap_region+0x4dc/0x6db

Invert those to the usual ordering.

Fixes: 33313a747e81 ("mm: lock newly mapped VMA which can be modified after it becomes visible")
Cc: stable@vger.kernel.org
Signed-off-by: Hugh Dickins <hughd@google.com>
Tested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/mmap.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2801,11 +2801,11 @@ cannot_expand:
 	if (vma_iter_prealloc(&vmi))
 		goto close_and_free_vma;
 
+	/* Lock the VMA since it is modified after insertion into VMA tree */
+	vma_start_write(vma);
 	if (vma->vm_file)
 		i_mmap_lock_write(vma->vm_file->f_mapping);
 
-	/* Lock the VMA since it is modified after insertion into VMA tree */
-	vma_start_write(vma);
 	vma_iter_store(&vmi, vma);
 	mm->map_count++;
 	if (vma->vm_file) {



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

* [PATCH 6.4 5/8] mm: call arch_swap_restore() from do_swap_page()
  2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
                   ` (3 preceding siblings ...)
  2023-07-09 11:14 ` [PATCH 6.4 4/8] mm: lock newly mapped VMA with corrected ordering Greg Kroah-Hartman
@ 2023-07-09 11:14 ` Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 6/8] bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page Greg Kroah-Hartman
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable
  Cc: Greg Kroah-Hartman, patches, Peter Collingbourne, Qun-wei Lin,
	David Hildenbrand, Huang, Ying, Steven Price, Catalin Marinas,
	Andrew Morton

From: Peter Collingbourne <pcc@google.com>

commit 6dca4ac6fc91fd41ea4d6c4511838d37f4e0eab2 upstream.

Commit c145e0b47c77 ("mm: streamline COW logic in do_swap_page()") moved
the call to swap_free() before the call to set_pte_at(), which meant that
the MTE tags could end up being freed before set_pte_at() had a chance to
restore them.  Fix it by adding a call to the arch_swap_restore() hook
before the call to swap_free().

Link: https://lkml.kernel.org/r/20230523004312.1807357-2-pcc@google.com
Link: https://linux-review.googlesource.com/id/I6470efa669e8bd2f841049b8c61020c510678965
Fixes: c145e0b47c77 ("mm: streamline COW logic in do_swap_page()")
Signed-off-by: Peter Collingbourne <pcc@google.com>
Reported-by: Qun-wei Lin <Qun-wei.Lin@mediatek.com>
Closes: https://lore.kernel.org/all/5050805753ac469e8d727c797c2218a9d780d434.camel@mediatek.com/
Acked-by: David Hildenbrand <david@redhat.com>
Acked-by: "Huang, Ying" <ying.huang@intel.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: <stable@vger.kernel.org>	[6.1+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/memory.c |    7 +++++++
 1 file changed, 7 insertions(+)

--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3933,6 +3933,13 @@ vm_fault_t do_swap_page(struct vm_fault
 	}
 
 	/*
+	 * Some architectures may have to restore extra metadata to the page
+	 * when reading from swap. This metadata may be indexed by swap entry
+	 * so this must be called before swap_free().
+	 */
+	arch_swap_restore(entry, folio);
+
+	/*
 	 * Remove the swap entry and conditionally try to free up the swapcache.
 	 * We're already holding a reference on the page but haven't mapped it
 	 * yet.



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

* [PATCH 6.4 6/8] bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page
  2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
                   ` (4 preceding siblings ...)
  2023-07-09 11:14 ` [PATCH 6.4 5/8] mm: call arch_swap_restore() from do_swap_page() Greg Kroah-Hartman
@ 2023-07-09 11:14 ` Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking Greg Kroah-Hartman
  2023-07-09 11:14 ` [PATCH 6.4 8/8] fork: lock VMAs of the parent process when forking, again Greg Kroah-Hartman
  7 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable
  Cc: Greg Kroah-Hartman, patches, Liu Shixin, Muchun Song,
	Matthew Wilcox, Mike Kravetz, Oscar Salvador, Andrew Morton

From: Liu Shixin <liushixin2@huawei.com>

commit 028725e73375a1ff080bbdf9fb503306d0116f28 upstream.

commit dd0ff4d12dd2 ("bootmem: remove the vmemmap pages from kmemleak in
put_page_bootmem") fix an overlaps existing problem of kmemleak.  But the
problem still existed when HAVE_BOOTMEM_INFO_NODE is disabled, because in
this case, free_bootmem_page() will call free_reserved_page() directly.

Fix the problem by adding kmemleak_free_part() in free_bootmem_page() when
HAVE_BOOTMEM_INFO_NODE is disabled.

Link: https://lkml.kernel.org/r/20230704101942.2819426-1-liushixin2@huawei.com
Fixes: f41f2ed43ca5 ("mm: hugetlb: free the vmemmap pages associated with each HugeTLB page")
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Acked-by: Muchun Song <songmuchun@bytedance.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/bootmem_info.h |    2 ++
 1 file changed, 2 insertions(+)

--- a/include/linux/bootmem_info.h
+++ b/include/linux/bootmem_info.h
@@ -3,6 +3,7 @@
 #define __LINUX_BOOTMEM_INFO_H
 
 #include <linux/mm.h>
+#include <linux/kmemleak.h>
 
 /*
  * Types for free bootmem stored in page->lru.next. These have to be in
@@ -59,6 +60,7 @@ static inline void get_page_bootmem(unsi
 
 static inline void free_bootmem_page(struct page *page)
 {
+	kmemleak_free_part(page_to_virt(page), PAGE_SIZE);
 	free_reserved_page(page);
 }
 #endif



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

* [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking
  2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
                   ` (5 preceding siblings ...)
  2023-07-09 11:14 ` [PATCH 6.4 6/8] bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page Greg Kroah-Hartman
@ 2023-07-09 11:14 ` Greg Kroah-Hartman
  2023-07-09 12:39   ` Thorsten Leemhuis
  2023-07-09 11:14 ` [PATCH 6.4 8/8] fork: lock VMAs of the parent process when forking, again Greg Kroah-Hartman
  7 siblings, 1 reply; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable
  Cc: Greg Kroah-Hartman, patches, Suren Baghdasaryan,
	David Hildenbrand, Jiri Slaby, Holger Hoffstätte,
	Jacob Young, Liam R. Howlett, Andrew Morton

From: Suren Baghdasaryan <surenb@google.com>

commit 2b4f3b4987b56365b981f44a7e843efa5b6619b9 upstream.

Patch series "Avoid memory corruption caused by per-VMA locks", v4.

A memory corruption was reported in [1] with bisection pointing to the
patch [2] enabling per-VMA locks for x86.  Based on the reproducer
provided in [1] we suspect this is caused by the lack of VMA locking while
forking a child process.

Patch 1/2 in the series implements proper VMA locking during fork.  I
tested the fix locally using the reproducer and was unable to reproduce
the memory corruption problem.

This fix can potentially regress some fork-heavy workloads.  Kernel build
time did not show noticeable regression on a 56-core machine while a
stress test mapping 10000 VMAs and forking 5000 times in a tight loop
shows ~7% regression.  If such fork time regression is unacceptable,
disabling CONFIG_PER_VMA_LOCK should restore its performance.  Further
optimizations are possible if this regression proves to be problematic.

Patch 2/2 disables per-VMA locks until the fix is tested and verified.


This patch (of 2):

When forking a child process, parent write-protects an anonymous page and
COW-shares it with the child being forked using copy_present_pte().
Parent's TLB is flushed right before we drop the parent's mmap_lock in
dup_mmap().  If we get a write-fault before that TLB flush in the parent,
and we end up replacing that anonymous page in the parent process in
do_wp_page() (because, COW-shared with the child), this might lead to some
stale writable TLB entries targeting the wrong (old) page.  Similar issue
happened in the past with userfaultfd (see flush_tlb_page() call inside
do_wp_page()).

Lock VMAs of the parent process when forking a child, which prevents
concurrent page faults during fork operation and avoids this issue.  This
fix can potentially regress some fork-heavy workloads.  Kernel build time
did not show noticeable regression on a 56-core machine while a stress
test mapping 10000 VMAs and forking 5000 times in a tight loop shows ~7%
regression.  If such fork time regression is unacceptable, disabling
CONFIG_PER_VMA_LOCK should restore its performance.  Further optimizations
are possible if this regression proves to be problematic.

Link: https://lkml.kernel.org/r/20230706011400.2949242-1-surenb@google.com
Link: https://lkml.kernel.org/r/20230706011400.2949242-2-surenb@google.com
Fixes: 0bff0aaea03e ("x86/mm: try VMA lock-based page fault handling first")
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Suggested-by: David Hildenbrand <david@redhat.com>
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Closes: https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/
Reported-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Closes: https://lore.kernel.org/all/b198d649-f4bf-b971-31d0-e8433ec2a34c@applied-asynchrony.com/
Reported-by: Jacob Young <jacobly.alt@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=3D217624
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Acked-by: David Hildenbrand <david@redhat.com>
Tested-by: Holger Hoffsttte <holger@applied-asynchrony.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 kernel/fork.c |    6 ++++++
 1 file changed, 6 insertions(+)

--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -662,6 +662,12 @@ static __latent_entropy int dup_mmap(str
 		retval = -EINTR;
 		goto fail_uprobe_end;
 	}
+#ifdef CONFIG_PER_VMA_LOCK
+	/* Disallow any page faults before calling flush_cache_dup_mm */
+	for_each_vma(old_vmi, mpnt)
+		vma_start_write(mpnt);
+	vma_iter_set(&old_vmi, 0);
+#endif
 	flush_cache_dup_mm(oldmm);
 	uprobe_dup_mmap(oldmm, mm);
 	/*



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

* [PATCH 6.4 8/8] fork: lock VMAs of the parent process when forking, again
  2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
                   ` (6 preceding siblings ...)
  2023-07-09 11:14 ` [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking Greg Kroah-Hartman
@ 2023-07-09 11:14 ` Greg Kroah-Hartman
  7 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 11:14 UTC (permalink / raw)
  To: stable
  Cc: Greg Kroah-Hartman, patches, David Hildenbrand, Jiri Slaby,
	Holger Hoffstätte, Jacob Young, Suren Baghdasaryan,
	Linus Torvalds

From: Suren Baghdasaryan <surenb@google.com>

commit fb49c455323ff8319a123dd312be9082c49a23a5 upstream.

When forking a child process, the parent write-protects anonymous pages
and COW-shares them with the child being forked using copy_present_pte().

We must not take any concurrent page faults on the source vma's as they
are being processed, as we expect both the vma and the pte's behind it
to be stable.  For example, the anon_vma_fork() expects the parents
vma->anon_vma to not change during the vma copy.

A concurrent page fault on a page newly marked read-only by the page
copy might trigger wp_page_copy() and a anon_vma_prepare(vma) on the
source vma, defeating the anon_vma_clone() that wasn't done because the
parent vma originally didn't have an anon_vma, but we now might end up
copying a pte entry for a page that has one.

Before the per-vma lock based changes, the mmap_lock guaranteed
exclusion with concurrent page faults.  But now we need to do a
vma_start_write() to make sure no concurrent faults happen on this vma
while it is being processed.

This fix can potentially regress some fork-heavy workloads.  Kernel
build time did not show noticeable regression on a 56-core machine while
a stress test mapping 10000 VMAs and forking 5000 times in a tight loop
shows ~5% regression.  If such fork time regression is unacceptable,
disabling CONFIG_PER_VMA_LOCK should restore its performance.  Further
optimizations are possible if this regression proves to be problematic.

Suggested-by: David Hildenbrand <david@redhat.com>
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Closes: https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/
Reported-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Closes: https://lore.kernel.org/all/b198d649-f4bf-b971-31d0-e8433ec2a34c@applied-asynchrony.com/
Reported-by: Jacob Young <jacobly.alt@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217624
Fixes: 0bff0aaea03e ("x86/mm: try VMA lock-based page fault handling first")
Cc: stable@vger.kernel.org
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 kernel/fork.c |    1 +
 1 file changed, 1 insertion(+)

--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -696,6 +696,7 @@ static __latent_entropy int dup_mmap(str
 	for_each_vma(old_vmi, mpnt) {
 		struct file *file;
 
+		vma_start_write(mpnt);
 		if (mpnt->vm_flags & VM_DONTCOPY) {
 			vm_stat_account(mm, mpnt->vm_flags, -vma_pages(mpnt));
 			continue;



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

* Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking
  2023-07-09 11:14 ` [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking Greg Kroah-Hartman
@ 2023-07-09 12:39   ` Thorsten Leemhuis
  2023-07-09 13:32     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 17+ messages in thread
From: Thorsten Leemhuis @ 2023-07-09 12:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman, stable
  Cc: patches, Suren Baghdasaryan, David Hildenbrand, Jiri Slaby,
	Holger Hoffstätte, Jacob Young, Liam R. Howlett,
	Andrew Morton

On 09.07.23 13:14, Greg Kroah-Hartman wrote:
> From: Suren Baghdasaryan <surenb@google.com>
> 
> commit 2b4f3b4987b56365b981f44a7e843efa5b6619b9 upstream.
> 
> Patch series "Avoid memory corruption caused by per-VMA locks", v4.
> 
> A memory corruption was reported in [1] with bisection pointing to the
> patch [2] enabling per-VMA locks for x86.  Based on the reproducer
> provided in [1] we suspect this is caused by the lack of VMA locking while
> forking a child process.
> [...]

Question from someone that is neither a C nor a git expert -- and thus
might say something totally stupid below (and thus maybe should not have
sent this mail at all).

But I have to wonder: is adding this patch to stable necessary given
patch 8/8?

FWIW, this change looks like this:

> ---
>  kernel/fork.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -662,6 +662,12 @@ static __latent_entropy int dup_mmap(str
>  		retval = -EINTR;
>  		goto fail_uprobe_end;
>  	}
> +#ifdef CONFIG_PER_VMA_LOCK
> +	/* Disallow any page faults before calling flush_cache_dup_mm */
> +	for_each_vma(old_vmi, mpnt)
> +		vma_start_write(mpnt);
> +	vma_iter_set(&old_vmi, 0);
> +#endif
>  	flush_cache_dup_mm(oldmm);
>  	uprobe_dup_mmap(oldmm, mm);
>  	/*

But when I look at kernel/fork.c in mainline I can't see this bit. I
also only see Linus' change (e.g. patch 8/8 in this series) when I look at
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/kernel/fork.c

What am I missing?

Ciao, Thorsten (who noticed this just by chance)

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

* Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking
  2023-07-09 12:39   ` Thorsten Leemhuis
@ 2023-07-09 13:32     ` Greg Kroah-Hartman
  2023-07-09 16:04       ` Suren Baghdasaryan
  0 siblings, 1 reply; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 13:32 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: stable, patches, Suren Baghdasaryan, David Hildenbrand,
	Jiri Slaby, Holger Hoffstätte, Jacob Young, Liam R. Howlett,
	Andrew Morton

On Sun, Jul 09, 2023 at 02:39:00PM +0200, Thorsten Leemhuis wrote:
> On 09.07.23 13:14, Greg Kroah-Hartman wrote:
> > From: Suren Baghdasaryan <surenb@google.com>
> > 
> > commit 2b4f3b4987b56365b981f44a7e843efa5b6619b9 upstream.
> > 
> > Patch series "Avoid memory corruption caused by per-VMA locks", v4.
> > 
> > A memory corruption was reported in [1] with bisection pointing to the
> > patch [2] enabling per-VMA locks for x86.  Based on the reproducer
> > provided in [1] we suspect this is caused by the lack of VMA locking while
> > forking a child process.
> > [...]
> 
> Question from someone that is neither a C nor a git expert -- and thus
> might say something totally stupid below (and thus maybe should not have
> sent this mail at all).
> 
> But I have to wonder: is adding this patch to stable necessary given
> patch 8/8?
> 
> FWIW, this change looks like this:
> 
> > ---
> >  kernel/fork.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -662,6 +662,12 @@ static __latent_entropy int dup_mmap(str
> >  		retval = -EINTR;
> >  		goto fail_uprobe_end;
> >  	}
> > +#ifdef CONFIG_PER_VMA_LOCK
> > +	/* Disallow any page faults before calling flush_cache_dup_mm */
> > +	for_each_vma(old_vmi, mpnt)
> > +		vma_start_write(mpnt);
> > +	vma_iter_set(&old_vmi, 0);
> > +#endif
> >  	flush_cache_dup_mm(oldmm);
> >  	uprobe_dup_mmap(oldmm, mm);
> >  	/*
> 
> But when I look at kernel/fork.c in mainline I can't see this bit. I
> also only see Linus' change (e.g. patch 8/8 in this series) when I look at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/kernel/fork.c

Look at 946c6b59c56d ("Merge tag 'mm-hotfixes-stable-2023-07-08-10-43'
of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")

Where Linus manually dropped those #ifdefs.

Hm, I'll leave them for now in 6.4.y as that is "safer", but if Suren
feels comfortable, I'll gladly take a patch from him to drop them in the
6.4.y tree as well.

thanks,

greg k-h

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

* Re: [PATCH 6.4 0/8] 6.4.3-rc1 review
@ 2023-07-09 14:02 Ronald Warsow
  0 siblings, 0 replies; 17+ messages in thread
From: Ronald Warsow @ 2023-07-09 14:02 UTC (permalink / raw)
  To: linux-kernel; +Cc: stable

Hi Greg

6.4.3-rc1

compiles, boots and runs here on x86_64
(Intel Rocket Lake, i5-11400)

Thanks

Tested-by: Ronald Warsow <rwarsow@gmx.de

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

* Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking
  2023-07-09 13:32     ` Greg Kroah-Hartman
@ 2023-07-09 16:04       ` Suren Baghdasaryan
  2023-07-09 16:09         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 17+ messages in thread
From: Suren Baghdasaryan @ 2023-07-09 16:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Thorsten Leemhuis, stable, patches, David Hildenbrand, Jiri Slaby,
	Holger Hoffstätte, Jacob Young, Liam R. Howlett,
	Andrew Morton

On Sun, Jul 9, 2023 at 6:32 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jul 09, 2023 at 02:39:00PM +0200, Thorsten Leemhuis wrote:
> > On 09.07.23 13:14, Greg Kroah-Hartman wrote:
> > > From: Suren Baghdasaryan <surenb@google.com>
> > >
> > > commit 2b4f3b4987b56365b981f44a7e843efa5b6619b9 upstream.
> > >
> > > Patch series "Avoid memory corruption caused by per-VMA locks", v4.
> > >
> > > A memory corruption was reported in [1] with bisection pointing to the
> > > patch [2] enabling per-VMA locks for x86.  Based on the reproducer
> > > provided in [1] we suspect this is caused by the lack of VMA locking while
> > > forking a child process.
> > > [...]
> >
> > Question from someone that is neither a C nor a git expert -- and thus
> > might say something totally stupid below (and thus maybe should not have
> > sent this mail at all).
> >
> > But I have to wonder: is adding this patch to stable necessary given
> > patch 8/8?
> >
> > FWIW, this change looks like this:
> >
> > > ---
> > >  kernel/fork.c |    6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > --- a/kernel/fork.c
> > > +++ b/kernel/fork.c
> > > @@ -662,6 +662,12 @@ static __latent_entropy int dup_mmap(str
> > >             retval = -EINTR;
> > >             goto fail_uprobe_end;
> > >     }
> > > +#ifdef CONFIG_PER_VMA_LOCK
> > > +   /* Disallow any page faults before calling flush_cache_dup_mm */
> > > +   for_each_vma(old_vmi, mpnt)
> > > +           vma_start_write(mpnt);
> > > +   vma_iter_set(&old_vmi, 0);
> > > +#endif
> > >     flush_cache_dup_mm(oldmm);
> > >     uprobe_dup_mmap(oldmm, mm);
> > >     /*
> >
> > But when I look at kernel/fork.c in mainline I can't see this bit. I
> > also only see Linus' change (e.g. patch 8/8 in this series) when I look at
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/kernel/fork.c
>
> Look at 946c6b59c56d ("Merge tag 'mm-hotfixes-stable-2023-07-08-10-43'
> of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")
>
> Where Linus manually dropped those #ifdefs.
>
> Hm, I'll leave them for now in 6.4.y as that is "safer", but if Suren
> feels comfortable, I'll gladly take a patch from him to drop them in the
> 6.4.y tree as well.

Hi Greg,
Give me a couple hours to get back to my computer. Linus took a
different version of this patch and changed the description quite a
bit. Once I'm home I can send you the patchset that was merged into
his tree. Also let me know if you want to disable CONFIG_PER_VMA_LOCK
in the stable branch (the patch called "[PATCH 6.4 1/8] mm: disable
CONFIG_PER_VMA_LOCK until its fixed" which Linus did not take AFAIKT).
Thanks,
Suren.

>
> thanks,
>
> greg k-h

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

* Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking
  2023-07-09 16:04       ` Suren Baghdasaryan
@ 2023-07-09 16:09         ` Greg Kroah-Hartman
  2023-07-09 19:53           ` Suren Baghdasaryan
  0 siblings, 1 reply; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 16:09 UTC (permalink / raw)
  To: Suren Baghdasaryan
  Cc: Thorsten Leemhuis, stable, patches, David Hildenbrand, Jiri Slaby,
	Holger Hoffstätte, Jacob Young, Liam R. Howlett,
	Andrew Morton

On Sun, Jul 09, 2023 at 09:04:09AM -0700, Suren Baghdasaryan wrote:
> On Sun, Jul 9, 2023 at 6:32 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Sun, Jul 09, 2023 at 02:39:00PM +0200, Thorsten Leemhuis wrote:
> > > On 09.07.23 13:14, Greg Kroah-Hartman wrote:
> > > > From: Suren Baghdasaryan <surenb@google.com>
> > > >
> > > > commit 2b4f3b4987b56365b981f44a7e843efa5b6619b9 upstream.
> > > >
> > > > Patch series "Avoid memory corruption caused by per-VMA locks", v4.
> > > >
> > > > A memory corruption was reported in [1] with bisection pointing to the
> > > > patch [2] enabling per-VMA locks for x86.  Based on the reproducer
> > > > provided in [1] we suspect this is caused by the lack of VMA locking while
> > > > forking a child process.
> > > > [...]
> > >
> > > Question from someone that is neither a C nor a git expert -- and thus
> > > might say something totally stupid below (and thus maybe should not have
> > > sent this mail at all).
> > >
> > > But I have to wonder: is adding this patch to stable necessary given
> > > patch 8/8?
> > >
> > > FWIW, this change looks like this:
> > >
> > > > ---
> > > >  kernel/fork.c |    6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > --- a/kernel/fork.c
> > > > +++ b/kernel/fork.c
> > > > @@ -662,6 +662,12 @@ static __latent_entropy int dup_mmap(str
> > > >             retval = -EINTR;
> > > >             goto fail_uprobe_end;
> > > >     }
> > > > +#ifdef CONFIG_PER_VMA_LOCK
> > > > +   /* Disallow any page faults before calling flush_cache_dup_mm */
> > > > +   for_each_vma(old_vmi, mpnt)
> > > > +           vma_start_write(mpnt);
> > > > +   vma_iter_set(&old_vmi, 0);
> > > > +#endif
> > > >     flush_cache_dup_mm(oldmm);
> > > >     uprobe_dup_mmap(oldmm, mm);
> > > >     /*
> > >
> > > But when I look at kernel/fork.c in mainline I can't see this bit. I
> > > also only see Linus' change (e.g. patch 8/8 in this series) when I look at
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/kernel/fork.c
> >
> > Look at 946c6b59c56d ("Merge tag 'mm-hotfixes-stable-2023-07-08-10-43'
> > of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")
> >
> > Where Linus manually dropped those #ifdefs.
> >
> > Hm, I'll leave them for now in 6.4.y as that is "safer", but if Suren
> > feels comfortable, I'll gladly take a patch from him to drop them in the
> > 6.4.y tree as well.
> 
> Hi Greg,
> Give me a couple hours to get back to my computer. Linus took a
> different version of this patch and changed the description quite a
> bit. Once I'm home I can send you the patchset that was merged into
> his tree. Also let me know if you want to disable CONFIG_PER_VMA_LOCK
> in the stable branch (the patch called "[PATCH 6.4 1/8] mm: disable
> CONFIG_PER_VMA_LOCK until its fixed" which Linus did not take AFAIKT).

No rush, you can do this on Monday.

I took the patches that Linus added to his tree already into the stable
6.4.y tree, and it's in the -rc release I pushed out a few hours ago.

So if you want to look at the -rc release, that would be great, the full
list of patches can be seen here:
	https://lore.kernel.org/r/20230709111345.297026264@linuxfoundation.org

thanks,

greg k-h

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

* Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking
  2023-07-09 16:09         ` Greg Kroah-Hartman
@ 2023-07-09 19:53           ` Suren Baghdasaryan
  2023-07-09 20:24             ` Suren Baghdasaryan
  0 siblings, 1 reply; 17+ messages in thread
From: Suren Baghdasaryan @ 2023-07-09 19:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Thorsten Leemhuis, stable, patches, David Hildenbrand, Jiri Slaby,
	Holger Hoffstätte, Jacob Young, Liam R. Howlett,
	Andrew Morton

On Sun, Jul 9, 2023 at 7:48 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jul 09, 2023 at 09:04:09AM -0700, Suren Baghdasaryan wrote:
> > On Sun, Jul 9, 2023 at 6:32 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Sun, Jul 09, 2023 at 02:39:00PM +0200, Thorsten Leemhuis wrote:
> > > > On 09.07.23 13:14, Greg Kroah-Hartman wrote:
> > > > > From: Suren Baghdasaryan <surenb@google.com>
> > > > >
> > > > > commit 2b4f3b4987b56365b981f44a7e843efa5b6619b9 upstream.
> > > > >
> > > > > Patch series "Avoid memory corruption caused by per-VMA locks", v4.
> > > > >
> > > > > A memory corruption was reported in [1] with bisection pointing to the
> > > > > patch [2] enabling per-VMA locks for x86.  Based on the reproducer
> > > > > provided in [1] we suspect this is caused by the lack of VMA locking while
> > > > > forking a child process.
> > > > > [...]
> > > >
> > > > Question from someone that is neither a C nor a git expert -- and thus
> > > > might say something totally stupid below (and thus maybe should not have
> > > > sent this mail at all).
> > > >
> > > > But I have to wonder: is adding this patch to stable necessary given
> > > > patch 8/8?
> > > >
> > > > FWIW, this change looks like this:
> > > >
> > > > > ---
> > > > >  kernel/fork.c |    6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > --- a/kernel/fork.c
> > > > > +++ b/kernel/fork.c
> > > > > @@ -662,6 +662,12 @@ static __latent_entropy int dup_mmap(str
> > > > >             retval = -EINTR;
> > > > >             goto fail_uprobe_end;
> > > > >     }
> > > > > +#ifdef CONFIG_PER_VMA_LOCK
> > > > > +   /* Disallow any page faults before calling flush_cache_dup_mm */
> > > > > +   for_each_vma(old_vmi, mpnt)
> > > > > +           vma_start_write(mpnt);
> > > > > +   vma_iter_set(&old_vmi, 0);
> > > > > +#endif
> > > > >     flush_cache_dup_mm(oldmm);
> > > > >     uprobe_dup_mmap(oldmm, mm);
> > > > >     /*
> > > >
> > > > But when I look at kernel/fork.c in mainline I can't see this bit. I
> > > > also only see Linus' change (e.g. patch 8/8 in this series) when I look at
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/kernel/fork.c
> > >
> > > Look at 946c6b59c56d ("Merge tag 'mm-hotfixes-stable-2023-07-08-10-43'
> > > of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")
> > >
> > > Where Linus manually dropped those #ifdefs.
> > >
> > > Hm, I'll leave them for now in 6.4.y as that is "safer", but if Suren
> > > feels comfortable, I'll gladly take a patch from him to drop them in the
> > > 6.4.y tree as well.
> >
> > Hi Greg,
> > Give me a couple hours to get back to my computer. Linus took a
> > different version of this patch and changed the description quite a
> > bit. Once I'm home I can send you the patchset that was merged into
> > his tree. Also let me know if you want to disable CONFIG_PER_VMA_LOCK
> > in the stable branch (the patch called "[PATCH 6.4 1/8] mm: disable
> > CONFIG_PER_VMA_LOCK until its fixed" which Linus did not take AFAIKT).
>
> No rush, you can do this on Monday.
>
> I took the patches that Linus added to his tree already into the stable
> 6.4.y tree, and it's in the -rc release I pushed out a few hours ago.

I just checked your stable master branch and it's perfectly in sync
with Linus' tree.

>
> So if you want to look at the -rc release, that would be great, the full
> list of patches can be seen here:
>         https://lore.kernel.org/r/20230709111345.297026264@linuxfoundation.org

Let me sync git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
to see what's happening there.
Thanks,
Suren.

>
> thanks,
>
> greg k-h

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

* Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking
  2023-07-09 19:53           ` Suren Baghdasaryan
@ 2023-07-09 20:24             ` Suren Baghdasaryan
  2023-07-09 20:40               ` Greg Kroah-Hartman
  0 siblings, 1 reply; 17+ messages in thread
From: Suren Baghdasaryan @ 2023-07-09 20:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Thorsten Leemhuis, stable, patches, David Hildenbrand, Jiri Slaby,
	Holger Hoffstätte, Jacob Young, Liam R. Howlett,
	Andrew Morton

On Sun, Jul 9, 2023 at 7:53 PM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Sun, Jul 9, 2023 at 7:48 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Sun, Jul 09, 2023 at 09:04:09AM -0700, Suren Baghdasaryan wrote:
> > > On Sun, Jul 9, 2023 at 6:32 AM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Sun, Jul 09, 2023 at 02:39:00PM +0200, Thorsten Leemhuis wrote:
> > > > > On 09.07.23 13:14, Greg Kroah-Hartman wrote:
> > > > > > From: Suren Baghdasaryan <surenb@google.com>
> > > > > >
> > > > > > commit 2b4f3b4987b56365b981f44a7e843efa5b6619b9 upstream.
> > > > > >
> > > > > > Patch series "Avoid memory corruption caused by per-VMA locks", v4.
> > > > > >
> > > > > > A memory corruption was reported in [1] with bisection pointing to the
> > > > > > patch [2] enabling per-VMA locks for x86.  Based on the reproducer
> > > > > > provided in [1] we suspect this is caused by the lack of VMA locking while
> > > > > > forking a child process.
> > > > > > [...]
> > > > >
> > > > > Question from someone that is neither a C nor a git expert -- and thus
> > > > > might say something totally stupid below (and thus maybe should not have
> > > > > sent this mail at all).
> > > > >
> > > > > But I have to wonder: is adding this patch to stable necessary given
> > > > > patch 8/8?
> > > > >
> > > > > FWIW, this change looks like this:
> > > > >
> > > > > > ---
> > > > > >  kernel/fork.c |    6 ++++++
> > > > > >  1 file changed, 6 insertions(+)
> > > > > >
> > > > > > --- a/kernel/fork.c
> > > > > > +++ b/kernel/fork.c
> > > > > > @@ -662,6 +662,12 @@ static __latent_entropy int dup_mmap(str
> > > > > >             retval = -EINTR;
> > > > > >             goto fail_uprobe_end;
> > > > > >     }
> > > > > > +#ifdef CONFIG_PER_VMA_LOCK
> > > > > > +   /* Disallow any page faults before calling flush_cache_dup_mm */
> > > > > > +   for_each_vma(old_vmi, mpnt)
> > > > > > +           vma_start_write(mpnt);
> > > > > > +   vma_iter_set(&old_vmi, 0);
> > > > > > +#endif
> > > > > >     flush_cache_dup_mm(oldmm);
> > > > > >     uprobe_dup_mmap(oldmm, mm);
> > > > > >     /*
> > > > >
> > > > > But when I look at kernel/fork.c in mainline I can't see this bit. I
> > > > > also only see Linus' change (e.g. patch 8/8 in this series) when I look at
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/kernel/fork.c
> > > >
> > > > Look at 946c6b59c56d ("Merge tag 'mm-hotfixes-stable-2023-07-08-10-43'
> > > > of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")
> > > >
> > > > Where Linus manually dropped those #ifdefs.
> > > >
> > > > Hm, I'll leave them for now in 6.4.y as that is "safer", but if Suren
> > > > feels comfortable, I'll gladly take a patch from him to drop them in the
> > > > 6.4.y tree as well.
> > >
> > > Hi Greg,
> > > Give me a couple hours to get back to my computer. Linus took a
> > > different version of this patch and changed the description quite a
> > > bit. Once I'm home I can send you the patchset that was merged into
> > > his tree. Also let me know if you want to disable CONFIG_PER_VMA_LOCK
> > > in the stable branch (the patch called "[PATCH 6.4 1/8] mm: disable
> > > CONFIG_PER_VMA_LOCK until its fixed" which Linus did not take AFAIKT).
> >
> > No rush, you can do this on Monday.
> >
> > I took the patches that Linus added to his tree already into the stable
> > 6.4.y tree, and it's in the -rc release I pushed out a few hours ago.
>
> I just checked your stable master branch and it's perfectly in sync
> with Linus' tree.
>
> >
> > So if you want to look at the -rc release, that would be great, the full
> > list of patches can be seen here:
> >         https://lore.kernel.org/r/20230709111345.297026264@linuxfoundation.org
>
> Let me sync git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> to see what's happening there.

Ok, I'm looking at the linux-6.4.y branch in
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git

This patch is not needed (obsolete):
32d458fa68fe ("fork: lock VMAs of the parent process when forking")

Patch fb49c455323f ("fork: lock VMAs of the parent process when
forking, again") can be renamed into "fork: lock VMAs of the parent
process when forking" as its original.

Patch 11eaf9aa0699 ("mm: disable CONFIG_PER_VMA_LOCK until its fixed")
was removed in Linus' tree, see comment in
946c6b59c56dc6e7d8364a8959cb36bf6d10bc37 saying: "The merge undoes the
disabling of the CONFIG_PER_VMA_LOCK feature, since it was all
hopefully fixed in mainline.". Unless you want to keep
CONFIG_PER_VMA_LOCK disabled in the stable tree, that patch should
also be dropped.

Everything else LGTM.
Thanks,
Suren.


> Thanks,
> Suren.
>
> >
> > thanks,
> >
> > greg k-h

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

* Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking
  2023-07-09 20:24             ` Suren Baghdasaryan
@ 2023-07-09 20:40               ` Greg Kroah-Hartman
  0 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2023-07-09 20:40 UTC (permalink / raw)
  To: Suren Baghdasaryan
  Cc: Thorsten Leemhuis, stable, patches, David Hildenbrand, Jiri Slaby,
	Holger Hoffstätte, Jacob Young, Liam R. Howlett,
	Andrew Morton

On Sun, Jul 09, 2023 at 08:24:29PM +0000, Suren Baghdasaryan wrote:
> On Sun, Jul 9, 2023 at 7:53 PM Suren Baghdasaryan <surenb@google.com> wrote:
> >
> > On Sun, Jul 9, 2023 at 7:48 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Sun, Jul 09, 2023 at 09:04:09AM -0700, Suren Baghdasaryan wrote:
> > > > On Sun, Jul 9, 2023 at 6:32 AM Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Sun, Jul 09, 2023 at 02:39:00PM +0200, Thorsten Leemhuis wrote:
> > > > > > On 09.07.23 13:14, Greg Kroah-Hartman wrote:
> > > > > > > From: Suren Baghdasaryan <surenb@google.com>
> > > > > > >
> > > > > > > commit 2b4f3b4987b56365b981f44a7e843efa5b6619b9 upstream.
> > > > > > >
> > > > > > > Patch series "Avoid memory corruption caused by per-VMA locks", v4.
> > > > > > >
> > > > > > > A memory corruption was reported in [1] with bisection pointing to the
> > > > > > > patch [2] enabling per-VMA locks for x86.  Based on the reproducer
> > > > > > > provided in [1] we suspect this is caused by the lack of VMA locking while
> > > > > > > forking a child process.
> > > > > > > [...]
> > > > > >
> > > > > > Question from someone that is neither a C nor a git expert -- and thus
> > > > > > might say something totally stupid below (and thus maybe should not have
> > > > > > sent this mail at all).
> > > > > >
> > > > > > But I have to wonder: is adding this patch to stable necessary given
> > > > > > patch 8/8?
> > > > > >
> > > > > > FWIW, this change looks like this:
> > > > > >
> > > > > > > ---
> > > > > > >  kernel/fork.c |    6 ++++++
> > > > > > >  1 file changed, 6 insertions(+)
> > > > > > >
> > > > > > > --- a/kernel/fork.c
> > > > > > > +++ b/kernel/fork.c
> > > > > > > @@ -662,6 +662,12 @@ static __latent_entropy int dup_mmap(str
> > > > > > >             retval = -EINTR;
> > > > > > >             goto fail_uprobe_end;
> > > > > > >     }
> > > > > > > +#ifdef CONFIG_PER_VMA_LOCK
> > > > > > > +   /* Disallow any page faults before calling flush_cache_dup_mm */
> > > > > > > +   for_each_vma(old_vmi, mpnt)
> > > > > > > +           vma_start_write(mpnt);
> > > > > > > +   vma_iter_set(&old_vmi, 0);
> > > > > > > +#endif
> > > > > > >     flush_cache_dup_mm(oldmm);
> > > > > > >     uprobe_dup_mmap(oldmm, mm);
> > > > > > >     /*
> > > > > >
> > > > > > But when I look at kernel/fork.c in mainline I can't see this bit. I
> > > > > > also only see Linus' change (e.g. patch 8/8 in this series) when I look at
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/kernel/fork.c
> > > > >
> > > > > Look at 946c6b59c56d ("Merge tag 'mm-hotfixes-stable-2023-07-08-10-43'
> > > > > of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm")
> > > > >
> > > > > Where Linus manually dropped those #ifdefs.
> > > > >
> > > > > Hm, I'll leave them for now in 6.4.y as that is "safer", but if Suren
> > > > > feels comfortable, I'll gladly take a patch from him to drop them in the
> > > > > 6.4.y tree as well.
> > > >
> > > > Hi Greg,
> > > > Give me a couple hours to get back to my computer. Linus took a
> > > > different version of this patch and changed the description quite a
> > > > bit. Once I'm home I can send you the patchset that was merged into
> > > > his tree. Also let me know if you want to disable CONFIG_PER_VMA_LOCK
> > > > in the stable branch (the patch called "[PATCH 6.4 1/8] mm: disable
> > > > CONFIG_PER_VMA_LOCK until its fixed" which Linus did not take AFAIKT).
> > >
> > > No rush, you can do this on Monday.
> > >
> > > I took the patches that Linus added to his tree already into the stable
> > > 6.4.y tree, and it's in the -rc release I pushed out a few hours ago.
> >
> > I just checked your stable master branch and it's perfectly in sync
> > with Linus' tree.
> >
> > >
> > > So if you want to look at the -rc release, that would be great, the full
> > > list of patches can be seen here:
> > >         https://lore.kernel.org/r/20230709111345.297026264@linuxfoundation.org
> >
> > Let me sync git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > to see what's happening there.
> 
> Ok, I'm looking at the linux-6.4.y branch in
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> 
> This patch is not needed (obsolete):
> 32d458fa68fe ("fork: lock VMAs of the parent process when forking")

Ok, I can drop that.

> Patch fb49c455323f ("fork: lock VMAs of the parent process when
> forking, again") can be renamed into "fork: lock VMAs of the parent
> process when forking" as its original.

Yes, I had to rename it, git doesn't like patches with identical names.

> Patch 11eaf9aa0699 ("mm: disable CONFIG_PER_VMA_LOCK until its fixed")
> was removed in Linus' tree, see comment in
> 946c6b59c56dc6e7d8364a8959cb36bf6d10bc37 saying: "The merge undoes the
> disabling of the CONFIG_PER_VMA_LOCK feature, since it was all
> hopefully fixed in mainline.". Unless you want to keep
> CONFIG_PER_VMA_LOCK disabled in the stable tree, that patch should
> also be dropped.

Ok, let me drop this too.

I'll push out a -rc2 with these changes, let me go work on it now...

thanks,

gre gk-h

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

end of thread, other threads:[~2023-07-09 20:40 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-09 11:14 [PATCH 6.4 0/8] 6.4.3-rc1 review Greg Kroah-Hartman
2023-07-09 11:14 ` [PATCH 6.4 1/8] mm: disable CONFIG_PER_VMA_LOCK until its fixed Greg Kroah-Hartman
2023-07-09 11:14 ` [PATCH 6.4 2/8] mm: lock a vma before stack expansion Greg Kroah-Hartman
2023-07-09 11:14 ` [PATCH 6.4 3/8] mm: lock newly mapped VMA which can be modified after it becomes visible Greg Kroah-Hartman
2023-07-09 11:14 ` [PATCH 6.4 4/8] mm: lock newly mapped VMA with corrected ordering Greg Kroah-Hartman
2023-07-09 11:14 ` [PATCH 6.4 5/8] mm: call arch_swap_restore() from do_swap_page() Greg Kroah-Hartman
2023-07-09 11:14 ` [PATCH 6.4 6/8] bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page Greg Kroah-Hartman
2023-07-09 11:14 ` [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking Greg Kroah-Hartman
2023-07-09 12:39   ` Thorsten Leemhuis
2023-07-09 13:32     ` Greg Kroah-Hartman
2023-07-09 16:04       ` Suren Baghdasaryan
2023-07-09 16:09         ` Greg Kroah-Hartman
2023-07-09 19:53           ` Suren Baghdasaryan
2023-07-09 20:24             ` Suren Baghdasaryan
2023-07-09 20:40               ` Greg Kroah-Hartman
2023-07-09 11:14 ` [PATCH 6.4 8/8] fork: lock VMAs of the parent process when forking, again Greg Kroah-Hartman
  -- strict thread matches above, loose matches on Subject: below --
2023-07-09 14:02 [PATCH 6.4 0/8] 6.4.3-rc1 review Ronald Warsow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).