From: <gregkh@linuxfoundation.org>
To: James.Bottomley@HansenPartnership.com,Liam.Howlett@oracle.com,akpm@linux-foundation.org,andreas@gaisler.com,broonie@kernel.org,catalin.marinas@arm.com,davem@davemloft.net,deller@gmx.de,gregkh@linuxfoundation.org,jannh@google.com,linux-mm@kvack.org,lorenzo.stoakes@oracle.com,peterx@redhat.com,torvalds@linux-foundation.org,vbabka@suse.cz,will@kernel.org
Cc: <stable-commits@vger.kernel.org>
Subject: Patch "mm: unconditionally close VMAs on error" has been added to the 5.10-stable tree
Date: Tue, 19 Nov 2024 15:24:38 +0100 [thread overview]
Message-ID: <2024111938-work-appease-851a@gregkh> (raw)
In-Reply-To: <a0a4cde203f9b3b4fa42577e43ca2679ae63b8f1.1731670097.git.lorenzo.stoakes@oracle.com>
This is a note to let you know that I've just added the patch titled
mm: unconditionally close VMAs on error
to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
mm-unconditionally-close-vmas-on-error.patch
and it can be found in the queue-5.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From stable+bounces-93521-greg=kroah.com@vger.kernel.org Fri Nov 15 13:37:59 2024
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Fri, 15 Nov 2024 12:36:52 +0000
Subject: mm: unconditionally close VMAs on error
To: stable@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>, "Liam R . Howlett" <Liam.Howlett@oracle.com>, Vlastimil Babka <vbabka@suse.cz>, Jann Horn <jannh@google.com>, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Linus Torvalds <torvalds@linux-foundation.org>, Peter Xu <peterx@redhat.com>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Mark Brown <broonie@kernel.org>, "David S . Miller" <davem@davemloft.net>, Andreas Larsson <andreas@gaisler.com>, "James E . J . Bottomley" <James.Bottomley@HansenPartnership.com>, Helge Deller <deller@gmx.de>
Message-ID: <a0a4cde203f9b3b4fa42577e43ca2679ae63b8f1.1731670097.git.lorenzo.stoakes@oracle.com>
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
[ Upstream commit 4080ef1579b2413435413988d14ac8c68e4d42c8 ]
Incorrect invocation of VMA callbacks when the VMA is no longer in a
consistent state is bug prone and risky to perform.
With regards to the important vm_ops->close() callback We have gone to
great lengths to try to track whether or not we ought to close VMAs.
Rather than doing so and risking making a mistake somewhere, instead
unconditionally close and reset vma->vm_ops to an empty dummy operations
set with a NULL .close operator.
We introduce a new function to do so - vma_close() - and simplify existing
vms logic which tracked whether we needed to close or not.
This simplifies the logic, avoids incorrect double-calling of the .close()
callback and allows us to update error paths to simply call vma_close()
unconditionally - making VMA closure idempotent.
Link: https://lkml.kernel.org/r/28e89dda96f68c505cb6f8e9fc9b57c3e9f74b42.1730224667.git.lorenzo.stoakes@oracle.com
Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails")
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reported-by: Jann Horn <jannh@google.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Reviewed-by: Jann Horn <jannh@google.com>
Cc: Andreas Larsson <andreas@gaisler.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Helge Deller <deller@gmx.de>
Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
mm/internal.h | 7 +++++++
mm/mmap.c | 9 +++------
mm/nommu.c | 3 +--
mm/util.c | 15 +++++++++++++++
4 files changed, 26 insertions(+), 8 deletions(-)
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -46,6 +46,13 @@ void page_writeback_init(void);
*/
int mmap_file(struct file *file, struct vm_area_struct *vma);
+/*
+ * If the VMA has a close hook then close it, and since closing it might leave
+ * it in an inconsistent state which makes the use of any hooks suspect, clear
+ * them down by installing dummy empty hooks.
+ */
+void vma_close(struct vm_area_struct *vma);
+
vm_fault_t do_swap_page(struct vm_fault *vmf);
void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -176,8 +176,7 @@ static struct vm_area_struct *remove_vma
struct vm_area_struct *next = vma->vm_next;
might_sleep();
- if (vma->vm_ops && vma->vm_ops->close)
- vma->vm_ops->close(vma);
+ vma_close(vma);
if (vma->vm_file)
fput(vma->vm_file);
mpol_put(vma_policy(vma));
@@ -1901,8 +1900,7 @@ out:
return addr;
close_and_free_vma:
- if (vma->vm_ops && vma->vm_ops->close)
- vma->vm_ops->close(vma);
+ vma_close(vma);
unmap_and_free_vma:
vma->vm_file = NULL;
fput(file);
@@ -2788,8 +2786,7 @@ int __split_vma(struct mm_struct *mm, st
return 0;
/* Clean everything up if vma_adjust failed. */
- if (new->vm_ops && new->vm_ops->close)
- new->vm_ops->close(new);
+ vma_close(new);
if (new->vm_file)
fput(new->vm_file);
unlink_anon_vmas(new);
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -662,8 +662,7 @@ static void delete_vma_from_mm(struct vm
*/
static void delete_vma(struct mm_struct *mm, struct vm_area_struct *vma)
{
- if (vma->vm_ops && vma->vm_ops->close)
- vma->vm_ops->close(vma);
+ vma_close(vma);
if (vma->vm_file)
fput(vma->vm_file);
put_nommu_region(vma->vm_region);
--- a/mm/util.c
+++ b/mm/util.c
@@ -1091,3 +1091,18 @@ int mmap_file(struct file *file, struct
return err;
}
+
+void vma_close(struct vm_area_struct *vma)
+{
+ static const struct vm_operations_struct dummy_vm_ops = {};
+
+ if (vma->vm_ops && vma->vm_ops->close) {
+ vma->vm_ops->close(vma);
+
+ /*
+ * The mapping is in an inconsistent state, and no further hooks
+ * may be invoked upon it.
+ */
+ vma->vm_ops = &dummy_vm_ops;
+ }
+}
Patches currently in stable-queue which might be from lorenzo.stoakes@oracle.com are
queue-5.10/mm-resolve-faulty-mmap_region-error-path-behaviour.patch
queue-5.10/mm-refactor-arch_calc_vm_flag_bits-and-arm64-mte-handling.patch
queue-5.10/mm-unconditionally-close-vmas-on-error.patch
queue-5.10/mm-avoid-unsafe-vma-hook-invocation-when-error-arises-on-mmap-hook.patch
next prev parent reply other threads:[~2024-11-19 14:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-15 12:36 [PATCH 5.10.y 0/4] fix error handling in mmap_region() and refactor (hotfixes) Lorenzo Stoakes
2024-11-15 12:36 ` [PATCH 5.10.y 1/4] mm: avoid unsafe VMA hook invocation when error arises on mmap hook Lorenzo Stoakes
2024-11-19 14:24 ` Patch "mm: avoid unsafe VMA hook invocation when error arises on mmap hook" has been added to the 5.10-stable tree gregkh
2024-11-15 12:36 ` [PATCH 5.10.y 2/4] mm: unconditionally close VMAs on error Lorenzo Stoakes
2024-11-19 14:24 ` gregkh [this message]
2024-11-15 12:36 ` [PATCH 5.10.y 3/4] mm: refactor arch_calc_vm_flag_bits() and arm64 MTE handling Lorenzo Stoakes
2024-11-19 14:24 ` Patch "mm: refactor arch_calc_vm_flag_bits() and arm64 MTE handling" has been added to the 5.10-stable tree gregkh
2024-11-15 12:36 ` [PATCH 5.10.y 4/4] mm: resolve faulty mmap_region() error path behaviour Lorenzo Stoakes
2024-11-19 14:24 ` Patch "mm: resolve faulty mmap_region() error path behaviour" has been added to the 5.10-stable tree gregkh
2024-11-25 15:28 ` [PATCH 5.10.y 0/4] fix error handling in mmap_region() and refactor (hotfixes) Vlastimil Babka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2024111938-work-appease-851a@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=James.Bottomley@HansenPartnership.com \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=andreas@gaisler.com \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=davem@davemloft.net \
--cc=deller@gmx.de \
--cc=jannh@google.com \
--cc=linux-mm@kvack.org \
--cc=lorenzo.stoakes@oracle.com \
--cc=peterx@redhat.com \
--cc=stable-commits@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=vbabka@suse.cz \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.