[PATCH 6.4 0/8] 6.4.3-rc1 review

[PATCH 6.4 0/8] 6.4.3-rc1 review

[Greg Kroah-Hartman]

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(-)

[PATCH 6.4 1/8] mm: disable CONFIG_PER_VMA_LOCK until its fixed

[Greg Kroah-Hartman]

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.
 

[PATCH 6.4 2/8] mm: lock a vma before stack expansion

[Greg Kroah-Hartman]

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

[PATCH 6.4 3/8] mm: lock newly mapped VMA which can be modified after it becomes visible

[Greg Kroah-Hartman]

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) {

[PATCH 6.4 4/8] mm: lock newly mapped VMA with corrected ordering

[Greg Kroah-Hartman]

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) {

[PATCH 6.4 5/8] mm: call arch_swap_restore() from do_swap_page()

[Greg Kroah-Hartman]

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.

[PATCH 6.4 6/8] bootmem: remove the vmemmap pages from kmemleak in free_bootmem_page

[Greg Kroah-Hartman]

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

[PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking

[Greg Kroah-Hartman]

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);
 	/*

Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking

[Thorsten Leemhuis]

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)

Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking

[Greg Kroah-Hartman]

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

Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking

[Suren Baghdasaryan]

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

Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking

[Greg Kroah-Hartman]

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

Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking

[Suren Baghdasaryan]

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

Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking

[Suren Baghdasaryan]

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

Re: [PATCH 6.4 7/8] fork: lock VMAs of the parent process when forking

[Greg Kroah-Hartman]

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

[PATCH 6.4 8/8] fork: lock VMAs of the parent process when forking, again

[Greg Kroah-Hartman]

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;