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: avoid unsafe VMA hook invocation when error arises on mmap hook" has been added to the 5.10-stable tree
Date: Tue, 19 Nov 2024 15:24:37 +0100 [thread overview]
Message-ID: <2024111937-regress-pacifist-65e9@gregkh> (raw)
In-Reply-To: <3a4ff9ebcc085e0074711ae883fa5ea11e6d7a8e.1731670097.git.lorenzo.stoakes@oracle.com>
This is a note to let you know that I've just added the patch titled
mm: avoid unsafe VMA hook invocation when error arises on mmap hook
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-avoid-unsafe-vma-hook-invocation-when-error-arises-on-mmap-hook.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-93520-greg=kroah.com@vger.kernel.org Fri Nov 15 13:37:55 2024
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Fri, 15 Nov 2024 12:36:51 +0000
Subject: mm: avoid unsafe VMA hook invocation when error arises on mmap hook
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: <3a4ff9ebcc085e0074711ae883fa5ea11e6d7a8e.1731670097.git.lorenzo.stoakes@oracle.com>
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
[ Upstream commit 3dd6ed34ce1f2356a77fb88edafb5ec96784e3cf ]
Patch series "fix error handling in mmap_region() and refactor
(hotfixes)", v4.
mmap_region() is somewhat terrifying, with spaghetti-like control flow and
numerous means by which issues can arise and incomplete state, memory
leaks and other unpleasantness can occur.
A large amount of the complexity arises from trying to handle errors late
in the process of mapping a VMA, which forms the basis of recently
observed issues with resource leaks and observable inconsistent state.
This series goes to great lengths to simplify how mmap_region() works and
to avoid unwinding errors late on in the process of setting up the VMA for
the new mapping, and equally avoids such operations occurring while the
VMA is in an inconsistent state.
The patches in this series comprise the minimal changes required to
resolve existing issues in mmap_region() error handling, in order that
they can be hotfixed and backported. There is additionally a follow up
series which goes further, separated out from the v1 series and sent and
updated separately.
This patch (of 5):
After an attempted mmap() fails, we are no longer in a situation where we
can safely interact with VMA hooks. This is currently not enforced,
meaning that we need complicated handling to ensure we do not incorrectly
call these hooks.
We can avoid the whole issue by treating the VMA as suspect the moment
that the file->f_ops->mmap() function reports an error by replacing
whatever VMA operations were installed with a dummy empty set of VMA
operations.
We do so through a new helper function internal to mm - mmap_file() -
which is both more logically named than the existing call_mmap() function
and correctly isolates handling of the vm_op reassignment to mm.
All the existing invocations of call_mmap() outside of mm are ultimately
nested within the call_mmap() from mm, which we now replace.
It is therefore safe to leave call_mmap() in place as a convenience
function (and to avoid churn). The invokers are:
ovl_file_operations -> mmap -> ovl_mmap() -> backing_file_mmap()
coda_file_operations -> mmap -> coda_file_mmap()
shm_file_operations -> shm_mmap()
shm_file_operations_huge -> shm_mmap()
dma_buf_fops -> dma_buf_mmap_internal -> i915_dmabuf_ops
-> i915_gem_dmabuf_mmap()
None of these callers interact with vm_ops or mappings in a problematic
way on error, quickly exiting out.
Link: https://lkml.kernel.org/r/cover.1730224667.git.lorenzo.stoakes@oracle.com
Link: https://lkml.kernel.org/r/d41fd763496fd0048a962f3fd9407dc72dd4fd86.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: Liam R. Howlett <Liam.Howlett@oracle.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
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 | 12 ++++++++++++
mm/mmap.c | 4 ++--
mm/nommu.c | 4 ++--
mm/util.c | 18 ++++++++++++++++++
4 files changed, 34 insertions(+), 4 deletions(-)
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -34,6 +34,18 @@
void page_writeback_init(void);
+/*
+ * This is a file-backed mapping, and is about to be memory mapped - invoke its
+ * mmap hook and safely handle error conditions. On error, VMA hooks will be
+ * mutated.
+ *
+ * @file: File which backs the mapping.
+ * @vma: VMA which we are mapping.
+ *
+ * Returns: 0 if success, error otherwise.
+ */
+int mmap_file(struct file *file, 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
@@ -1808,7 +1808,7 @@ unsigned long mmap_region(struct file *f
* new file must not have been exposed to user-space, yet.
*/
vma->vm_file = get_file(file);
- error = call_mmap(file, vma);
+ error = mmap_file(file, vma);
if (error)
goto unmap_and_free_vma;
@@ -1823,7 +1823,7 @@ unsigned long mmap_region(struct file *f
addr = vma->vm_start;
- /* If vm_flags changed after call_mmap(), we should try merge vma again
+ /* If vm_flags changed after mmap_file(), we should try merge vma again
* as we may succeed this time.
*/
if (unlikely(vm_flags != vma->vm_flags && prev)) {
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -955,7 +955,7 @@ static int do_mmap_shared_file(struct vm
{
int ret;
- ret = call_mmap(vma->vm_file, vma);
+ ret = mmap_file(vma->vm_file, vma);
if (ret == 0) {
vma->vm_region->vm_top = vma->vm_region->vm_end;
return 0;
@@ -986,7 +986,7 @@ static int do_mmap_private(struct vm_are
* - VM_MAYSHARE will be set if it may attempt to share
*/
if (capabilities & NOMMU_MAP_DIRECT) {
- ret = call_mmap(vma->vm_file, vma);
+ ret = mmap_file(vma->vm_file, vma);
if (ret == 0) {
/* shouldn't return success if we're not sharing */
BUG_ON(!(vma->vm_flags & VM_MAYSHARE));
--- a/mm/util.c
+++ b/mm/util.c
@@ -1073,3 +1073,21 @@ int __weak memcmp_pages(struct page *pag
kunmap_atomic(addr1);
return ret;
}
+
+int mmap_file(struct file *file, struct vm_area_struct *vma)
+{
+ static const struct vm_operations_struct dummy_vm_ops = {};
+ int err = call_mmap(file, vma);
+
+ if (likely(!err))
+ return 0;
+
+ /*
+ * OK, we tried to call the file hook for mmap(), but an error
+ * arose. The mapping is in an inconsistent state and we most not invoke
+ * any further hooks on it.
+ */
+ vma->vm_ops = &dummy_vm_ops;
+
+ return err;
+}
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 ` gregkh [this message]
2024-11-15 12:36 ` [PATCH 5.10.y 2/4] mm: unconditionally close VMAs on error Lorenzo Stoakes
2024-11-19 14:24 ` Patch "mm: unconditionally close VMAs on error" has been added to the 5.10-stable tree gregkh
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=2024111937-regress-pacifist-65e9@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox